> -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 01 October 2025 17:39 > To: Shameer Kolothum <[email protected]>; Jonathan Cameron > <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; Jason Gunthorpe <[email protected]>; Nicolin Chen > <[email protected]>; [email protected]; [email protected]; Nathan > Chen <[email protected]>; Matt Ochs <[email protected]>; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH v4 06/27] hw/arm/smmuv3-accel: Restrict accelerated > SMMUv3 to vfio-pci endpoints with iommufd > > External email: Use caution opening links or attachments > > > On 9/30/25 10:03 AM, Shameer Kolothum wrote: > > > >> -----Original Message----- > >> From: Jonathan Cameron <[email protected]> > >> Sent: 29 September 2025 17:09 > >> To: Shameer Kolothum <[email protected]> > >> Cc: [email protected]; [email protected]; > >> [email protected]; [email protected]; Jason Gunthorpe > >> <[email protected]>; Nicolin Chen <[email protected]>; > [email protected]; > >> [email protected]; Nathan Chen <[email protected]>; Matt Ochs > >> <[email protected]>; [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected] > >> Subject: Re: [PATCH v4 06/27] hw/arm/smmuv3-accel: Restrict accelerated > >> SMMUv3 to vfio-pci endpoints with iommufd > >> > >> External email: Use caution opening links or attachments > >> > >> > >> On Mon, 29 Sep 2025 14:36:22 +0100 > >> Shameer Kolothum <[email protected]> wrote: > >> > >>> Accelerated SMMUv3 is only useful when the device can take advantage > of > >>> the host's SMMUv3 in nested mode. To keep things simple and correct, we > >>> only allow this feature for vfio-pci endpoint devices that use the iommufd > >>> backend. We also allow non-endpoint emulated devices like PCI bridges > and > >>> root ports, so that users can plug in these vfio-pci devices. We can only > >>> enforce this if devices are cold plugged. For hotplug cases, give > >>> appropriate > >>> warnings. > >>> > >>> Another reason for this limit is to avoid problems with IOTLB > >>> invalidations. Some commands (e.g., CMD_TLBI_NH_ASID) lack an > >> associated > >>> SID, making it difficult to trace the originating device. If we allowed > >>> emulated endpoint devices, QEMU would have to invalidate both its own > >>> software IOTLB and the host's hardware IOTLB, which could slow things > >>> down. > >>> > >>> Since vfio-pci devices in nested mode rely on the host SMMUv3's nested > >>> translation (S1+S2), their get_address_space() callback must return the > >>> system address space so that VFIO core can setup correct S2 mappings > >>> for guest RAM. > >>> > >>> So in short: > >>> - vfio-pci devices(with iommufd as backend) return the system address > >>> space. > >>> - bridges and root ports return the IOMMU address space. > >>> > >>> Signed-off-by: Shameer Kolothum <[email protected]> > >> One question that really applies to earlier patch and an even more trivial > >> comment on a comment than the earlier ones ;) > >> > >> Reviewed-by: Jonathan Cameron <[email protected]> > >> > >>> --- > >>> hw/arm/smmuv3-accel.c | 68 > ++++++++++++++++++++++++++++- > >>> hw/pci-bridge/pci_expander_bridge.c | 1 - > >>> include/hw/pci/pci_bridge.h | 1 + > >>> 3 files changed, 68 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c > >>> index 79f1713be6..44410cfb2a 100644 > >>> --- a/hw/arm/smmuv3-accel.c > >>> +++ b/hw/arm/smmuv3-accel.c > >>> static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void > >> *opaque, > >> > >> I should have noticed this in previous patch... > >> What does add stand for here? This name is not particularly clear to me. > > Good question ๐. > > > > I believe the name comes from the smmu-common.c implementation of > > get_address_space: > > > > static const PCIIOMMUOps smmu_ops = { > > .get_address_space = smmu_find_add_as, > > }; > > Looking at it again, that version allocates a new MR and creates a > > new address space per sdev, so perhaps "add" referred to the address > > space creation. > this stems from the original terminology used in intel-iommu.c > (vtd_find_add_as) > > the smmu-common code looks for a registered device corresponding to @bus > and @devfn (this is the 'find'). If it exists it returns it, otherwise > it allocates a bus and SMMUDevice object according to what exists and > initializes the AddressSpace (this is the 'add').
Agree. > > > > > This callback here originally did something similar but no longer does. > I don't get why it does not do something similar anymore? Ok. It does all the above related to the "find" and "add" described above. But for vfio-pci dev with IOMMUFD backend, it now returns the global &address_space_memory. Previously we were creating a separate address space pointing to system memory for each such devices. We could argue that in general what the function does is "get" the appropriate address space for the device and can just call it simply, get_dev_address_space() . > > So, I think itโs better to just rename it to smmuv3_accel_get_as() > Well I would prefer we keep the original terminology to match other > viommu code. Except of course if I misunderstood the existing code. Ok. I will keep the same then with some comment to explain that "find" and "add" part. Thanks, Shameer
