On 12/11/25 7:49 AM, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Subject: Re: [PATCH v8 09/23] intel_iommu_accel: Check for compatibility
>> with IOMMUFD backed device when x-flts=on
>>
>> Hi Zhenzhong,
>> On 11/17/25 10:37 AM, Zhenzhong Duan wrote:
>>> When vIOMMU is configured x-flts=on in scalable mode, first stage page
>> table
>>> is passed to host to construct nested page table for passthrough devices.
>>>
>>> We need to check compatibility of some critical IOMMU capabilities
>> between
>>> vIOMMU and host IOMMU to ensure guest first stage page table could be
>> used by
>>> host.
>>>
>>> For instance, vIOMMU supports first stage 1GB large page mapping, but
>> host does
>>> not, then this IOMMUFD backed device should fail.
>>>
>>> Even of the checks pass, for now we willingly reject the association because
>>> all the bits are not there yet, it will be relaxed in the end of this
>>> series.
>>>
>>> Note vIOMMU has exposed IOMMU_HWPT_ALLOC_NEST_PARENT flag to
>> force VFIO core to
>>> create nesting parent HWPT, if host doesn't support nested translation, the
>>> creation will fail. So no need to check nested capability here.
>>>
>>> Signed-off-by: Zhenzhong Duan <[email protected]>
>>> ---
>>> MAINTAINERS | 1 +
>>> hw/i386/intel_iommu_accel.h | 28 +++++++++++++++++++++++++
>>> hw/i386/intel_iommu.c | 5 ++---
>>> hw/i386/intel_iommu_accel.c | 42
>> +++++++++++++++++++++++++++++++++++++
>>> hw/i386/Kconfig | 5 +++++
>>> hw/i386/meson.build | 1 +
>>> 6 files changed, 79 insertions(+), 3 deletions(-)
>>> create mode 100644 hw/i386/intel_iommu_accel.h
>>> create mode 100644 hw/i386/intel_iommu_accel.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index f4a30c126b..bc1d2b6261 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -3929,6 +3929,7 @@ R: Clément Mathieu--Drif
>> <[email protected]>
>>> S: Supported
>>> F: hw/i386/intel_iommu.c
>>> F: hw/i386/intel_iommu_internal.h
>>> +F: hw/i386/intel_iommu_accel.*
>>> F: include/hw/i386/intel_iommu.h
>>> F: tests/functional/x86_64/test_intel_iommu.py
>>> F: tests/qtest/intel-iommu-test.c
>>> diff --git a/hw/i386/intel_iommu_accel.h b/hw/i386/intel_iommu_accel.h
>>> new file mode 100644
>>> index 0000000000..c5274e342c
>>> --- /dev/null
>>> +++ b/hw/i386/intel_iommu_accel.h
>>> @@ -0,0 +1,28 @@
>>> +/*
>>> + * Intel IOMMU acceleration with nested translation
>>> + *
>>> + * Copyright (C) 2025 Intel Corporation.
>>> + *
>>> + * Authors: Zhenzhong Duan <[email protected]>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#ifndef HW_I386_INTEL_IOMMU_ACCEL_H
>>> +#define HW_I386_INTEL_IOMMU_ACCEL_H
>>> +#include CONFIG_DEVICES
> Here to address Cédric's suggestion.
>
>>> +
>>> +#ifdef CONFIG_VTD_ACCEL
>>> +bool vtd_check_hiod_accel(IntelIOMMUState *s, HostIOMMUDevice
>> *hiod,
>>> + Error **errp);
>>> +#else
>>> +static inline bool vtd_check_hiod_accel(IntelIOMMUState *s,
>>> + HostIOMMUDevice
>> *hiod,
>>> + Error **errp)
>>> +{
>>> + error_setg(errp,
>>> + "host IOMMU is incompatible with guest first stage
>> translation");
>> I would rather change the error msg to
>>
>> host IOMMU cannot be checked!
>> + append a hint through error_append_hint,
>> CONFIG_VTD_ACCEL is not enabled or smthg alike
> Will do.
>
>>> + return false;
>>> +}
>>> +#endif
>>> +#endif
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 3095d78321..d3c8a75878 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -26,6 +26,7 @@
>>> #include "hw/sysbus.h"
>>> #include "hw/iommu.h"
>>> #include "intel_iommu_internal.h"
>>> +#include "intel_iommu_accel.h"
>>> #include "hw/pci/pci.h"
>>> #include "hw/pci/pci_bus.h"
>>> #include "hw/qdev-properties.h"
>>> @@ -4596,9 +4597,7 @@ static bool vtd_check_hiod(IntelIOMMUState *s,
>> HostIOMMUDevice *hiod,
>>> return true;
>>> }
>>>
>>> - error_setg(errp,
>>> - "host device is uncompatible with first stage
>> translation");
>>> - return false;
>>> + return vtd_check_hiod_accel(s, hiod, errp);
>>> }
>>>
>>> static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
>> devfn,
>>> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
>>> new file mode 100644
>>> index 0000000000..6846c6ec4d
>>> --- /dev/null
>>> +++ b/hw/i386/intel_iommu_accel.c
>>> @@ -0,0 +1,42 @@
>>> +/*
>>> + * Intel IOMMU acceleration with nested translation
>>> + *
>>> + * Copyright (C) 2025 Intel Corporation.
>>> + *
>>> + * Authors: Zhenzhong Duan <[email protected]>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "system/iommufd.h"
>>> +#include "intel_iommu_internal.h"
>>> +#include "intel_iommu_accel.h"
>>> +
>>> +bool vtd_check_hiod_accel(IntelIOMMUState *s, HostIOMMUDevice
>> *hiod,
>>> + Error **errp)
>>> +{
>>> + struct HostIOMMUDeviceCaps *caps = &hiod->caps;
>>> + struct iommu_hw_info_vtd *vtd = &caps->vendor_caps.vtd;
>>> +
>>> + if (!object_dynamic_cast(OBJECT(hiod),
>> TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>>> + error_setg(errp, "Need IOMMUFD backend when x-flts=on");
>>> + return false;
>>> + }
>>> +
>>> + if (caps->type != IOMMU_HW_INFO_TYPE_INTEL_VTD) {
>>> + error_setg(errp, "Incompatible host platform IOMMU type %d",
>>> + caps->type);
>>> + return false;
>>> + }
>>> +
>>> + if (s->fs1gp && !(vtd->cap_reg & VTD_CAP_FS1GP)) {
>>> + error_setg(errp,
>>> + "First stage 1GB large page is unsupported by host
>> IOMMU");
>>> + return false;
>>> + }
>>> +
>>> + error_setg(errp,
>>> + "host IOMMU is incompatible with guest first stage
>> translation");
>>> + return false;
>>> +}
>>> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
>>> index 6a0ab54bea..12473acaa7 100644
>>> --- a/hw/i386/Kconfig
>>> +++ b/hw/i386/Kconfig
>>> @@ -150,8 +150,13 @@ config X86_IOMMU
>>>
>>> config VTD
>>> bool
>>> + imply VTD_ACCEL
>>> select X86_IOMMU
>>>
>>> +config VTD_ACCEL
>>> + bool
>>> + depends on VTD && IOMMUFD
>>> +
>>> config AMD_IOMMU
>>> bool
>>> select X86_IOMMU
>>> diff --git a/hw/i386/meson.build b/hw/i386/meson.build
>>> index 436b3ce52d..63ae57baa5 100644
>>> --- a/hw/i386/meson.build
>>> +++ b/hw/i386/meson.build
>>> @@ -21,6 +21,7 @@ i386_ss.add(when: 'CONFIG_Q35', if_true:
>> files('pc_q35.c'))
>>> i386_ss.add(when: 'CONFIG_VMMOUSE', if_true: files('vmmouse.c'))
>>> i386_ss.add(when: 'CONFIG_VMPORT', if_true: files('vmport.c'))
>>> i386_ss.add(when: 'CONFIG_VTD', if_true: files('intel_iommu.c'))
>>> +i386_ss.add(when: 'CONFIG_VTD_ACCEL', if_true:
>> files('intel_iommu_accel.c'))
>>> i386_ss.add(when: 'CONFIG_SGX', if_true: files('sgx-epc.c','sgx.c'),
>>> if_false: files('sgx-stub.c'))
>>>
>> wrt comments made by Cédric in
>> https://lore.kernel.org/all/IA3PR11MB9136B13C0C48EF293D3B599D92FAA@
>> IA3PR11MB9136.namprd11.prod.outlook.com/
>> I see you kept the original approach. I have no strong opinion on that.
>> I let Cédric's comment if he strongly disagrees.
> Guess you mean adding '#include CONFIG_DEVICES'?
> I added it in hw/i386/intel_iommu_accel.h, see above. There is reference to
> CONFIG_VTD_ACCEL in intel_iommu_accel.h, I thought it's better to add it
> there instead of intel_iommu_accel.c
No I rather meant Cédric's comment on extending HostIOMMUDeviceClass
instead of using iommufd directly Eric
>
> Thanks
> Zhenzhong
>
>> With my comment taken into account feel free to grab my
>>
>> Reviewed-by: Eric Auger <[email protected]>
>>
>> Thanks
>>
>> Eric
