Hi
>-----Original Message-----
>From: Liu, Yi L <[email protected]>
>Subject: Re: [PATCH v5 14/15] intel_iommu_accel: Add pasid bits size check
>
>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 <[email protected]>
>> Tested-by: Xudong Hao <[email protected]>
>> Reviewed-by: Clement Mathieu--Drif <[email protected]>
>> ---
>> 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?
ecap_reg is from ioctl(IOMMU_GET_HW_INFO), it's not ecap register in vIOMMU.
My understanding is that guest kernel will pick the min(caps->max_pasid_log2,
s->pasid), so checking the two doesn't make sense?
Thanks
Zhenzhong
>
>> + 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");
