Re: [PATCH v3 1/7] hw/arm/smmuv3-accel: Add helper for resolving auto parameters

[Nathan Chen <nathanc@nvidia.com>]

On 5/14/2026 6:41 AM, Shameer Kolothum Thodi wrote:
> 
>> -----Original Message-----
>> From: Nathan Chen<nathanc@nvidia.com>
>> Sent: 12 May 2026 20:35
>> To:qemu-arm@nongnu.org;qemu-devel@nongnu.org
>> Cc: Eric Auger<eric.auger@redhat.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Philippe Mathieu-Daudé<philmd@linaro.org>;
>> Zhao Liu<zhao1.liu@intel.com>; Matt Ochs<mochs@nvidia.com>; Nicolin
>> Chen<nicolinc@nvidia.com>; Shameer Kolothum Thodi
>> <skolothumtho@nvidia.com>; Nathan Chen<nathanc@nvidia.com>
>> Subject: [PATCH v3 1/7] hw/arm/smmuv3-accel: Add helper for resolving auto
>> parameters
>>
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Introduce smmuv3_accel_auto_finalise() to resolve properties that are
>> set to 'auto' for accelerated SMMUv3. This helper function allows
>> properties such as ats, ril, ssidsize, and oas support to be resolved
>> from host IOMMU capabilities via IOMMU_GET_HW_INFO.
>>
>> The later commits in this series set the auto_mode flag to true when
>> an accel SMMUv3 property value is explicitly set to 'auto', or if the
>> property value is not set and defaults to auto mode.
>>
>> Setting these property values to 'auto' requires at least one
>> cold-plugged device to retrieve and finalise these properties. If the
>> auto_mode flag is true, register a machine_init_done notifier to
>> verify this requirement and fail boot if it is not met.
>>
>> Hot-plugged devices into an accel SMMUv3-associated bus will re-use
>> the resolved host values from the initial cold-plug.
>>
>> Subsequent patches will make use of this helper to resolve 'auto' to
>> what is reported by host IOMMU capabilities.
>>
>> Suggested-by: Shameer Kolothum<skolothumtho@nvidia.com>
>> Reviewed-by: Eric Auger<eric.auger@redhat.com>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>>   hw/arm/smmuv3-accel.c   | 44
>> +++++++++++++++++++++++++++++++++++++++++
>>   hw/arm/smmuv3-accel.h   |  2 ++
>>   include/hw/arm/smmuv3.h |  2 ++
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 862be814a0..82c0800f4d 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -18,6 +18,7 @@
>>
>>   #include "smmuv3-internal.h"
>>   #include "smmuv3-accel.h"
>> +#include "system/system.h"
>>
>>   /*
>>    * The root region aliases the global system memory, and shared_as_sysmem
>> @@ -35,11 +36,32 @@ static int smmuv3_oas_bits(uint32_t oas)
>>       return map[oas];
>>   }
>>
>> +static void smmuv3_accel_auto_finalise(SMMUv3State *s,
>> +                                       struct iommu_hw_info_arm_smmuv3 *info) {
> Nit: Move the opening brace to its own line.
> 
>> +    SMMUv3AccelState *accel = s->s_accel;
>> +
>> +    /*
>> +     * Return if 'auto' was not set for any accel SMMUv3 property, or
>> +     * if property values were already resolved from a previous call
>> +     * to this function (e.g. if this function was called again after
>> +     * VM boot during device hot plug). We do not accept new property
>> +     * values in this case where auto_finalised == true, and we re-use
>> +     * the values determined from the initial cold plug.
>> +     */
>> +    if (!accel->auto_mode || accel->auto_finalised) {
>> +        return;
>> +    }
>> +
>> +    accel->auto_finalised = true;
>> +}
>> +
>>   static bool
>>   smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>>                                    struct iommu_hw_info_arm_smmuv3 *info,
>>                                    Error **errp)
>>   {
>> +    smmuv3_accel_auto_finalise(s, info);
>> +
>>       /* QEMU SMMUv3 supports both linear and 2-level stream tables */
>>       if (FIELD_EX32(info->idr[0], IDR0, STLEVEL) !=
>>                   FIELD_EX32(s->idr[0], IDR0, STLEVEL)) {
>> @@ -918,6 +940,22 @@ static void smmuv3_accel_as_init(SMMUv3State *s)
>>       address_space_init(shared_as_sysmem, &root, "smmuv3-accel-as-
>> sysmem");
>>   }
>>
>> +static void smmuv3_machine_done(Notifier *notifier, void *data)
> smmuv3_accel_machine_done() better.
> 
>> +{
>> +    SMMUv3State *s = container_of(notifier, SMMUv3State, machine_done);
>> +    SMMUv3AccelState *accel = s->s_accel;
>> +
>> +    if (!s->accel) {
>> +        return;
> Not sure we need this as we only register this for accel cases.
> 
>> +    }
>> +
>> +    if (accel->auto_mode && !accel->auto_finalised) {
>> +        error_report("arm-smmuv3 accel=on with 'auto' properties requires "
>> +                     "at least one cold-plugged VFIO device");
>> +        exit(1);
>> +    }
>> +}
>> +
>>   bool smmuv3_accel_init(SMMUv3State *s, Error **errp)
>>   {
>>       SMMUState *bs = ARM_SMMU(s);
>> @@ -925,5 +963,11 @@ bool smmuv3_accel_init(SMMUv3State *s, Error
>> **errp)
>>       s->s_accel = g_new0(SMMUv3AccelState, 1);
>>       bs->iommu_ops = &smmuv3_accel_ops;
>>       smmuv3_accel_as_init(s);
>> +
>> +    if (s->s_accel->auto_mode) {
>> +        s->machine_done.notify = smmuv3_machine_done;
>> +        qemu_add_machine_init_done_notifier(&s->machine_done);
>> +    }
>> +
>>       return true;
>>   }
>> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
>> index 407940616c..87fecb5c68 100644
>> --- a/hw/arm/smmuv3-accel.h
>> +++ b/hw/arm/smmuv3-accel.h
>> @@ -25,6 +25,8 @@ typedef struct SMMUv3AccelState {
>>       uint32_t bypass_hwpt_id;
>>       uint32_t abort_hwpt_id;
>>       QLIST_HEAD(, SMMUv3AccelDevice) device_list;
>> +    bool auto_mode;
>> +    bool auto_finalised;
>>   } SMMUv3AccelState;
>>
>>   typedef struct SMMUS1Hwpt {
>> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
>> index 82f18eb090..fe0493c1aa 100644
>> --- a/include/hw/arm/smmuv3.h
>> +++ b/include/hw/arm/smmuv3.h
>> @@ -74,6 +74,8 @@ struct SMMUv3State {
>>       OnOffAuto ats;
>>       OasMode oas;
>>       SsidSizeMode ssidsize;
>> +
>> +    Notifier machine_done;
> Better to #include "qemu/notify.h" in this header.
> It may be compiling through some indirect #include now.
Ok, thanks for the feedback - I will update these in the next refresh.

Thanks,
Nathan