>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v5 09/21] intel_iommu: Fail passthrough device under PCI
>bridge if x-flts=on
>
>On 2025/8/22 14:40, Zhenzhong Duan wrote:
>> Currently we don't support nested translation for passthrough device with
>> emulated device under same PCI bridge, because they require different
>address
>> space when x-flts=on.
>>
>> In theory, we do support if devices under same PCI bridge are all
>passthrough
>> devices. But emulated device can be hotplugged under same bridge. To
>simplify,
>> just forbid passthrough device under PCI bridge no matter if there is, or will
>> be emulated devices under same bridge. This is acceptable 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>
>> Reviewed-by: Eric Auger <eric.auger@redhat.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 da355bda79..6edd91d94e 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -4341,9 +4341,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;
>>
>> @@ -4370,6 +4371,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);
>
>pci_find_device() finds bus pointer with bus_num, this can be avoided
>as you already have bus pointer. Perhaps this may be done by wrapping
>bus->devices[devfn] to a helper. Especially, pci_bus_num() may not have
>the correct bus number at this point.
Indeed, will do, just pdev=bus->devices[devfn] should work.
Thanks
Zhenzhong
>
>>
>> /* Remaining checks are all stage-1 translation specific */
>> if (!object_dynamic_cast(OBJECT(hiod),
>TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>> @@ -4392,6 +4395,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");
>> + return false;
>> + }
>> #endif
>>
>> error_setg(errp, "host IOMMU is incompatible with stage-1
>translation");
>> @@ -4425,7 +4434,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;
>
>other parts looks good to me.
>
>Reviewed-by: Yi Liu <yi.l.liu@intel.com>