> -----Original Message----- > From: Cédric Le Goater <[email protected]> > Sent: 12 March 2026 08:39 > To: Shameer Kolothum Thodi <[email protected]>; Markus > Armbruster <[email protected]> > Cc: Nathan Chen <[email protected]>; [email protected]; qemu- > [email protected]; Yi Liu <[email protected]>; Eric Auger > <[email protected]>; Zhenzhong Duan <[email protected]>; > Peter Maydell <[email protected]>; Shannon Zhao > <[email protected]>; Michael S . Tsirkin <[email protected]>; Igor > Mammedov <[email protected]>; Ani Sinha <[email protected]>; > Paolo Bonzini <[email protected]>; Daniel P.Berrangé > <[email protected]>; Alex Williamson <[email protected]>; Eric Blake > <[email protected]> > Subject: Re: [RFC PATCH 1/8] hw/arm/smmuv3-accel: Add helper for resolving > auto parameters > > External email: Use caution opening links or attachments > > > On 3/12/26 09:33, Shameer Kolothum Thodi wrote: > > > > > >> -----Original Message----- > >> From: Markus Armbruster <[email protected]> > >> Sent: 12 March 2026 08:21 > >> To: Shameer Kolothum Thodi <[email protected]> > >> Cc: Nathan Chen <[email protected]>; [email protected]; > qemu- > >> [email protected]; Yi Liu <[email protected]>; Eric Auger > >> <[email protected]>; Zhenzhong Duan > <[email protected]>; > >> Peter Maydell <[email protected]>; Shannon Zhao > >> <[email protected]>; Michael S . Tsirkin <[email protected]>; > >> Igor Mammedov <[email protected]>; Ani Sinha > <[email protected]>; > >> Paolo Bonzini <[email protected]>; Daniel P.Berrangé > >> <[email protected]>; Alex Williamson <[email protected]>; Cédric Le > >> Goater <[email protected]>; Eric Blake <[email protected]> > >> Subject: Re: [RFC PATCH 1/8] hw/arm/smmuv3-accel: Add helper for > >> resolving auto parameters > >> > >> External email: Use caution opening links or attachments > >> > >> > >> Shameer Kolothum Thodi <[email protected]> writes: > >> > >>>> -----Original Message----- > >>>> From: Markus Armbruster <[email protected]> > >> > >> [...] > >> > >>>> Nathan Chen <[email protected]> writes: > >>>> > >>>>> From: Nathan Chen <[email protected]> > >>>>> > >>>>> 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 values, while avoiding triggering > >>>>> auto-resolved values for hot-plugged devices. > >>>>> > >>>>> Auto mode requires at least one cold-plugged device to retrieve > >>>>> and finalise these properties, and we fail boot if that is not the > >>>>> case. > >>>>> > >>>>> Subsequent patches will make use of this helper to set the values > >>>>> when we convert the values to OnOffAuto. New auto_mode and > >>>>> auto_finalised bool members are added to SMMUv3AccelState. > >>>>> smmuv3_accel_init() will set auto_mode to true when 'auto' is > >>>>> detected for the accel SMMUv3 properties. > >>>>> smmuv3_accel_auto_finalise() will set auto_finalised to true after > >>>>> all 'auto' properties are resolved, and subsequent calls to this > >>>>> function will return early if auto_finalised is set to true. > >>>>> > >>>>> Suggested-by: Shameer Kolothum <[email protected]> > >>>>> Signed-off-by: Nathan Chen <[email protected]> > >>>>> --- > >>>>> hw/arm/smmuv3-accel.c | 38 > >> +++++++++++++++++++++++++++++++++-- > >>>> --- > >>>>> hw/arm/smmuv3-accel.h | 2 ++ > >>>>> 2 files changed, 35 insertions(+), 5 deletions(-) > >>>>> > >>>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c index > >>>>> 17306cd04b..617629bacd 100644 > >>>>> --- a/hw/arm/smmuv3-accel.c > >>>>> +++ b/hw/arm/smmuv3-accel.c > >>>>> @@ -35,11 +35,34 @@ static int smmuv3_oas_bits(uint32_t oas) > >>>>> return map[oas]; > >>>>> } > >>>>> > >>>>> +static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice > >> *pdev, > >>>>> + struct iommu_hw_info_arm_smmuv3 > >>>>> *info) { > >>>>> + SMMUv3AccelState *accel = s->s_accel; > >>>>> + > >>>>> + /* Return if no auto for any or finalised already */ > >>>>> + if (!accel->auto_mode || accel->auto_finalised) { > >>>>> + return; > >>>>> + } > >>>>> + > >>>>> + /* We can't update if device is hotplugged */ > >>>>> + if (DEVICE(pdev)->hotplugged) { > >>>>> + warn_report("arm-smmuv3: 'auto' feature property > >>>>> + detected, but host > >>>> " > >>>>> + "value cannot be applied for hot-plugged device; > >>>>> using " > >>>>> + "existing value"); > >>>> > >>>> Why is this warning useful? > >>>> > >>>> Does @auto's meaning depend on whether the device is cold- or > >>>> hot-plugged? > >>> > >>> If SMMUv3 accel=on and properties are set to "auto", we require at > >>> least one cold-plugged vfio-pci device to retrieve the associated > >>> host > >>> SMMUv3 information and finalise the QEMU SMMUv3 properties. > >>> > >>> In this series, "auto_mode" is set if any accel property is > >>> specified as "auto". The properties are then finalised using the > >>> first cold-plugged device and "auto_finalised" is set. > >>> > >>> Since we later fail the guest boot(see below) in the reset phase if > >>> auto_mode is set but auto_finalised is still false, the above > >>> hotplug case should not really happen. In that case the VM would not > boot. > >> > >> Sounds complicated. > >> > >>> So likely we can get rid of the hotplug check above. Need to do a > >>> bit more testing to confirm we cover all corner cases. > >>> > >>>> > >>>>> + return; > >>>>> + } > >>>>> + > >>>>> + accel->auto_finalised = true; } > >> > >> [...] > >> > >>>>> @@ -867,8 +891,12 @@ bool > >> smmuv3_accel_attach_gbpa_hwpt(SMMUv3State > >>>>> *s, Error **errp) > >>>>> > >>>>> void smmuv3_accel_reset(SMMUv3State *s) { > >>>>> - /* Attach a HWPT based on GBPA reset value */ > >>>>> - smmuv3_accel_attach_gbpa_hwpt(s, NULL); > >>>>> + if (s->s_accel && s->s_accel->auto_mode && > >>>>> + !s->s_accel->auto_finalised) > >>>> { > >>>>> + error_report("AUTO mode specified but properties not > >>>>> finalised."); > >>>>> + exit(1); > >>>> > >>>> How can we get here? > >>> > >>> This is reached from the SMMUv3 reset handler when auto mode is > >>> specified but no cold-plugged device has been attached to finalise > >>> the accelerated SMMUv3 properties. > >>> > >>> If no such device is present, "auto_finalised" remains false and we > >>> hit this path during reset. > >>> > >>> Cédric mentioned that we should not exit during the reset phase. If > >>> that is the case, we likely need another way to handle the scenario > >>> where auto mode is specified but no cold-plugged device is present. > >> > >> What exactly does the "auto" feature buy us? Is it worth the > >> complexity? > > > > The requirement came from the KubeVirt folks. It is easier for them if > > QEMU can probe the platform and set these values, instead of > > specifying them for each platform. > > > > See: > > https://lore.kernel.org/qemu-devel/d9192ae6-a94f-479a-b19f- > 734cc52a6be > > [email protected]/ > > > > Without "auto", the management layer needs to know the values > > supported on each platform. > > > > eg: on a GB200, it will look like, > > > > -device > > arm-smmuv3,primary-bus=pcie.1,id=smmuv3.0,accel=on,ats=on,ril=off,ssid > > size=20,oas=48 > > > > With "auto" it can simply be: > > > > -device arm-smmuv3,primary-bus=pcie.1,id=smmuv3.0,accel=on > > > > (provided a vfio-pci device is cold-plugged). > > how about, > > -device arm-smmuv3,primary- > bus=pcie.1,id=smmuv3.0,accel=on,iommufd=iommufd0 > > and smmuv3_accel_init() would check that the conditions are correct. >
We need both iommufd and idev for iommufd_backend_get_device_info() as kernel uses that to find the host SMMUv3 attached to that device. /** * struct iommu_hw_info - ioctl(IOMMU_GET_HW_INFO) * @size: sizeof(struct iommu_hw_info) * @flags: Must be 0 * @dev_id: The device bound to the iommufd ... Thanks, Shameer
