On 10/2/25 11:38 AM, Shameer Kolothum wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 01 October 2025 18:37
>> To: Shameer Kolothum <[email protected]>; qemu-
>> [email protected]; [email protected]
>> Cc: [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];
>> [email protected]
>> Subject: Re: [PATCH v4 07/27] hw/arm/smmuv3: Implement
>> get_viommu_cap() callback
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 9/29/25 3:36 PM, Shameer Kolothum wrote:
>>> For accelerated SMMUv3, we need nested parent domain creation. Add the
>>> callback support so that VFIO can create a nested parent.
>>>
>>> In the accelerated SMMUv3 case, the host SMMUv3 is configured in nested
>>> mode (S1 + S2), and the guest owns the Stage-1 page table. Therefore, we
>>> expose only Stage-1 to the guest to ensure it uses the correct page-table
>>> format.
>>>
>>> Reviewed-by: Nicolin Chen <[email protected]>
>>> Signed-off-by: Shameer Kolothum
>> <[email protected]>
>>> Signed-off-by: Shameer Kolothum <[email protected]>
>> Wonder if you shall keep both. I don't know the usage though but worth
>> to check.
> Hmm.. I don't know either for sure. What I followed here(I will double check)
> is all the patches I had previously(v3) I kept all the S-by tags. That seems
> to be
> a right thing to do and IIRC I have seen that previously as well.
>
>>> ---
>>> hw/arm/smmuv3-accel.c | 13 +++++++++++++
>>> hw/arm/virt.c | 13 +++++++++++++
>>> 2 files changed, 26 insertions(+)
>>>
>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>> index 44410cfb2a..6b0e512d86 100644
>>> --- a/hw/arm/smmuv3-accel.c
>>> +++ b/hw/arm/smmuv3-accel.c
>>> @@ -10,6 +10,7 @@
>>> #include "qemu/error-report.h"
>>>
>>> #include "hw/arm/smmuv3.h"
>>> +#include "hw/iommu.h"
>>> #include "hw/pci/pci_bridge.h"
>>> #include "hw/pci-host/gpex.h"
>>> #include "hw/vfio/pci.h"
>>> @@ -106,8 +107,20 @@ static AddressSpace
>> *smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
>>> }
>>> }
>>>
>>> +static uint64_t smmuv3_accel_get_viommu_flags(void *opaque)
>>> +{
>>> + /*
>>> + * We return VIOMMU_FLAG_WANT_NESTING_PARENT to inform VFIO
>> core to create a
>>> + * nesting parent which is required for accelerated SMMUv3 support.
>>> + * The real HW nested support should be reported from host SMMUv3
>> and if
>>> + * it doesn't, the nesting parent allocation will fail anyway in VFIO
>>> core.
>>> + */
>>> + return VIOMMU_FLAG_WANT_NESTING_PARENT;
>>> +}
>>> +
>>> static const PCIIOMMUOps smmuv3_accel_ops = {
>>> .get_address_space = smmuv3_accel_find_add_as,
>>> + .get_viommu_flags = smmuv3_accel_get_viommu_flags,
>>> };
>>>
>>> void smmuv3_accel_init(SMMUv3State *s)
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 02209fadcf..b533b0556e 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -3073,6 +3073,19 @@ static void
>> virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>>> return;
>>> }
>>>
>>> + if (object_property_get_bool(OBJECT(dev), "accel",
>>> &error_abort)) {
>> This looks unrelated to the get_viommu_flags() addition and to me this
>> shall be put in a separate patch of squashed in the patch that exposes
>> the accel prop Thanks Eric
> But my thought process was, without this we can't say the vIOMMU will support
> the nesting parent. But then the flag seems to be indicating that vIOMMU
> "want"
> nesting parent. So I guess we can move it for later.
Yes that's my understanding too
Eric
>
> Thanks,
> Shameer
