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....@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@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
