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').
>
> This callback here originally did something similar but no longer does.
I don't get why it does not do something similar anymore?
> 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.
Thanks
Eric
>
> Thanks,
> Shameer
