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
> +
> +#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

> +    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/ia3pr11mb9136b13c0c48ef293d3b599d92...@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.

With my comment taken into account feel free to grab my

Reviewed-by: Eric Auger <[email protected]>

Thanks

Eric


Reply via email to