RE: [git pull] IOMMU Updates for Linux v6.4

From: Tian, Kevin
Date: Fri May 05 2023 - 22:58:50 EST


> From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Sent: Saturday, May 6, 2023 4:11 AM
>
> On Fri, May 5, 2023 at 12:57 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> >
> > At least with the current situation CONFIG_INTEL_ENQCMD might be an
> > appropriate name, split out from the IOMMU kconfig and put in arch
> > kconfig?
>
> That would at least somewhat clarify the use. I find IOMMU_SVA to be a
> particularly opaque name.
>
> Admittedly I probably find it opaque because I come at it as somebody
> much more familiar with the MM side, not from the IOMMU side (and the
> _other_ conditional fields there make sense from that standpoint), but
> even so I think it would be good to clarify.
>
> Of course, making clear it's architecture-specific would also be an
> argument for having an actual architecture-specific part of 'struct
> mm_struct' (the same way we have 'struct thread_info' in task_struct),
> but with only one single field I suspect that's just not worth it.
>
> And who knows - maybe other architectures will pick something like this up?

Looks ARM has ST64BV/ST64BV0 similar to ENQCMD according to [1].

Looking at ARM spec [2]:

Single-copy Atomic 64-byte EL0 Store with Return stores eight 64-bit
doublewords from consecutive registers, Xt to X(t+7), to a memory
location, with the bottom 32 bits taken from ACCDATA_EL1, and writes
the status result of the store to a register.

ACCDATA_EL1 sounds like the PASID_MSR used in ENQCMD, though it's
not explicitly stated so...

With that CONFIG_CPU_PASID might be more future-proof based on
the semantics of those ENQCMD-like instructions?

But...

>
> But yes, it might be good to really pin down what the rules are. Right
> now IOMMU_SVA has *no* real help-text (there's a comment saying "#
> Shared Virtual Addressing"), and is also enabled by ARM_SMMU_V3_SVA,
> which apparently doesn't actually want pasid support at all.

... ARM_SMMU_V3_SVA does require pasid support. Just no ENQCMD.

SVA allows the device to access CPU virtual addresses with pasid to
identify a specific CPU page table when the device is shared by many
processes. e.g. each work queue in the device can be associated with
a unique pasid pointing to the CPU page table of the owning process.

w/o ENQCMD mm->pasid is not strictly necessary. IOMMU driver can
store the pasid information in its own structure when a CPU page
table is associated to a device.

ENQCMD further allows a single work queue shared by many processes,
by storing pasid in a MSR and sending it in the wire so the work
queue can be stateless. This needs to store the pasid in mm to handle
#GP, fork(), etc.

But after mm->pasid was introduced in 52ad9bc64c74 ("mm: Add a
pasid member to struct mm_struct "), it then became the common
storage for SVA beyond ENQCMD and common sva helpers are built
around mm->pasid, e.g. iommu_sva_bind_device().

From this angle using IOMMU_SVA to guard mm->pasid still makes
sense to me. But we do need to add the missing help-text for it.

>
> I dunno. I don't think this is a huge deal, but it did cause some
> confusion during the merge.
>
> Linus

[1] https://stackoverflow.com/questions/70561491/temporality-of-st64b-and-movdir64b
[2] https://developer.arm.com/documentation/ddi0596/2020-12/Base-Instructions/ST64BV0--Single-copy-Atomic-64-byte-EL0-Store-with-Return-