Re: [PATCH v5 14/15] intel_iommu_accel: Add pasid bits size check

[Yi Liu <yi.l.liu@intel.com>]

On 5/9/26 12:08, Zhenzhong Duan wrote:
> If pasid bits size is bigger than host side, host could fail to emulate
> all bindings in guest. Add a check to fail device plug early.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Tested-by: Xudong Hao <xudong.hao@intel.com>
> Reviewed-by: Clement Mathieu--Drif <clement.mathieu--drif@bull.com>
> ---
>   hw/i386/intel_iommu_internal.h | 1 +
>   hw/i386/intel_iommu_accel.c    | 8 ++++++++
>   2 files changed, 9 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 2c716c5297..519af3fa90 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -196,6 +196,7 @@
>   #define VTD_ECAP_SRS                (1ULL << 31)
>   #define VTD_ECAP_NWFS               (1ULL << 33)
>   #define VTD_ECAP_SET_PSS(x, v)      ((x)->ecap = deposit64((x)->ecap, 35, 5, v))
> +#define VTD_ECAP_GET_PSS(ecap)      extract64(ecap, 35, 5)
>   #define VTD_ECAP_PASID              (1ULL << 40)
>   #define VTD_ECAP_PDS                (1ULL << 42)
>   #define VTD_ECAP_SMTS               (1ULL << 43)
> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
> index 4ddf66262c..a0dd6b0ee0 100644
> --- a/hw/i386/intel_iommu_accel.c
> +++ b/hw/i386/intel_iommu_accel.c
> @@ -44,6 +44,7 @@ bool vtd_check_hiod_accel(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hiod,
>       HostIOMMUDevice *hiod = vtd_hiod->hiod;
>       struct HostIOMMUDeviceCaps *caps = &hiod->caps;
>       struct iommu_hw_info_vtd *vtd = &caps->vendor_caps.vtd;
> +    uint8_t hpasid = VTD_ECAP_GET_PSS(vtd->ecap_reg) + 1;
>       PCIBus *bus = vtd_hiod->bus;
>       PCIDevice *pdev = bus->devices[vtd_hiod->devfn];
>   
> @@ -64,6 +65,13 @@ bool vtd_check_hiod_accel(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hiod,
>           return false;
>       }
>   
> +    /* Only do the check when host device support PASIDs */
> +    if (caps->max_pasid_log2 && s->pasid > hpasid) {

the second comparison looks strange. hpasid is derived from ecap_reg, 
while ecap_reg is from s->pasid... is there any place that changes
the pss filed of ecap_reg afterward? I think this check should be
against caps->max_pasid_log2 as this is the value from hardware. right?

> +        error_setg(errp, "PASID bits size %d > host IOMMU PASID bits size %d",
> +                   s->pasid, hpasid);
> +        return false;
> +    }
> +
>       if (pci_device_get_iommu_bus_devfn(pdev, &bus, NULL, NULL)) {
>           error_setg(errp, "Host device downstream to a PCI bridge is "
>                      "unsupported when x-flts=on");