Re: [PATCH v2 08/19] intel_iommu: Fail passthrough device under PCI bridge if x-flts=on

[Eric Auger <eric.auger@redhat.com>]

Hi Zhenzhong,

On 6/20/25 9:18 AM, Zhenzhong Duan wrote:
> Currently we don't support nested translation for passthrough device
> with emulated device under same PCI bridge.

I don't understand the above. Do you want to forbid a configuration
where both a passthrough'ed device and an emulated device would be
downstream to a PCI bridge and protected by nested IOMMU?

If I am not wrong in the path you don't check coexistence of emulated
and passthrough'ed device but simply check whether the host assigned
device is downstream to a PCI bridge. So if I am not wrong this is not
really aligned to the commit msg description.
>
> Reason is for emulated devices, AS should switch to iommu MR, while for
> passthrough devices, it needs the AS stick with the system MR hence be
> able to keep the VFIO container IOAS as a GPA IOAS. To support this, let
> AS switch to iommu MR and have a separate GPA IOAS is needed, but that
> brings a new memory listener which duplicates with VFIO memory listener.
I have difficulties to parse the the above sentence
>
> For trade off, we choose to not support this special scenario because
> PCIE bridge is more popular than PCI bridge now.
>
> Suggested-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/i386/intel_iommu.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 1c79efc1cb..9d4adc9458 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -4330,9 +4330,10 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>      return vtd_dev_as;
>  }
>  
> -static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
> +static bool vtd_check_hiod(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hiod,
>                             Error **errp)
>  {
> +    HostIOMMUDevice *hiod = vtd_hiod->hiod;
>      HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
>      int ret;
>  
> @@ -4359,6 +4360,8 @@ static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
>  #ifdef CONFIG_IOMMUFD
>      struct HostIOMMUDeviceCaps *caps = &hiod->caps;
>      struct iommu_hw_info_vtd *vtd = &caps->vendor_caps.vtd;
> +    PCIBus *bus = vtd_hiod->bus;
> +    PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), vtd_hiod->devfn);
>  
>      /* Remaining checks are all stage-1 translation specific */
>      if (!object_dynamic_cast(OBJECT(hiod), TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
> @@ -4381,6 +4384,12 @@ static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
>          error_setg(errp, "Stage-1 1GB huge page is unsupported by host IOMMU");
>          return false;
>      }
> +
> +    if (pci_device_get_iommu_bus_devfn(pdev, &bus, NULL, NULL)) {
> +        error_setg(errp, "Host device under PCI bridge is unsupported "
> +                   "when x-flts=on");
so now the compatibility also comes from the device and not only from
the host IOMMU caps. (refering to my previous comment about
s/device/iommu in error msg)
> +        return false;
> +    }
>  #endif
>  
>      error_setg(errp, "host device is uncompatible with stage-1 translation");
> @@ -4414,7 +4423,7 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>      vtd_hiod->iommu_state = s;
>      vtd_hiod->hiod = hiod;
>  
> -    if (!vtd_check_hiod(s, hiod, errp)) {
> +    if (!vtd_check_hiod(s, vtd_hiod, errp)) {
>          g_free(vtd_hiod);
>          vtd_iommu_unlock(s);
>          return false;
Thanks

Eric