[PATCH v1 12/13] intel_iommu_accel: Add pasid bits size check

Zhenzhong Duan posted 13 patches 1 month ago
Maintainers: Yi Liu <yi.l.liu@intel.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
[PATCH v1 12/13] intel_iommu_accel: Add pasid bits size check
Posted by Zhenzhong Duan 1 month ago
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.

Pasid bits size should also be no more than 20 bits according to PCI spec.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu_internal.h | 1 +
 hw/i386/intel_iommu.c          | 5 +++++
 hw/i386/intel_iommu_accel.c    | 8 ++++++++
 3 files changed, 14 insertions(+)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index ede4db6d2d..d6674861fd 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_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.c b/hw/i386/intel_iommu.c
index 3ea5b92b34..e99a9cf9c6 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -5559,6 +5559,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
         error_setg(errp, "Need to set scalable mode for PASID");
         return false;
     }
+    if (s->pasid > PCI_EXT_CAP_PASID_MAX_WIDTH) {
+        error_setg(errp, "PASID width %d, exceed Max PASID Width %d allowed "
+                   "in PCI spec", s->pasid, PCI_EXT_CAP_PASID_MAX_WIDTH);
+        return false;
+    }
 
     if (s->svm) {
         if (!x86_iommu->dt_supported) {
diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
index acb1b1e238..15412123d5 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_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) {
+        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");
-- 
2.47.3
Re: [PATCH v1 12/13] intel_iommu_accel: Add pasid bits size check
Posted by CLEMENT MATHIEU--DRIF 1 month ago
Hi Zhenzhong,

On Thu, 2026-03-05 at 22:44 -0500, 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.
> 
> Pasid bits size should also be no more than 20 bits according to PCI spec.
> 
> Signed-off-by: Zhenzhong Duan <[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)>  
> ---  
>  hw/i386/intel_iommu_internal.h | 1 +  
>  hw/i386/intel_iommu.c          | 5 +++++  
>  hw/i386/intel_iommu_accel.c    | 8 ++++++++  
>  3 files changed, 14 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h  
> index ede4db6d2d..d6674861fd 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_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.c b/hw/i386/intel_iommu.c  
> index 3ea5b92b34..e99a9cf9c6 100644  
> --- a/hw/i386/intel_iommu.c  
> +++ b/hw/i386/intel_iommu.c  
> @@ -5559,6 +5559,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)  
>          error_setg(errp, "Need to set scalable mode for PASID");  
>          return false;  
>      }  
> +    if (s->pasid > PCI_EXT_CAP_PASID_MAX_WIDTH) {  
> +        error_setg(errp, "PASID width %d, exceed Max PASID Width %d allowed "  
> +                   "in PCI spec", s->pasid, PCI_EXT_CAP_PASID_MAX_WIDTH);  
> +        return false;  
> +    }  

I think this one should go in patch 5/13 as it is not related to accel

>    
>      if (s->svm) {  
>          if (!x86_iommu->dt_supported) {  
> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c  
> index acb1b1e238..15412123d5 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_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) {  
> +        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");
RE: [PATCH v1 12/13] intel_iommu_accel: Add pasid bits size check
Posted by Duan, Zhenzhong 1 month ago
Hi Clement,

>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@bull.com>
>Subject: Re: [PATCH v1 12/13] intel_iommu_accel: Add pasid bits size check
>
>Hi Zhenzhong,
>
>On Thu, 2026-03-05 at 22:44 -0500, 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.
>>
>> Pasid bits size should also be no more than 20 bits according to PCI spec.
>>
>> Signed-off-by: Zhenzhong Duan
><[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)>
>> ---
>>  hw/i386/intel_iommu_internal.h | 1 +
>>  hw/i386/intel_iommu.c          | 5 +++++
>>  hw/i386/intel_iommu_accel.c    | 8 ++++++++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index ede4db6d2d..d6674861fd 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_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.c b/hw/i386/intel_iommu.c
>> index 3ea5b92b34..e99a9cf9c6 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -5559,6 +5559,11 @@ static bool vtd_decide_config(IntelIOMMUState *s,
>Error **errp)
>>          error_setg(errp, "Need to set scalable mode for PASID");
>>          return false;
>>      }
>> +    if (s->pasid > PCI_EXT_CAP_PASID_MAX_WIDTH) {
>> +        error_setg(errp, "PASID width %d, exceed Max PASID Width %d allowed "
>> +                   "in PCI spec", s->pasid, PCI_EXT_CAP_PASID_MAX_WIDTH);
>> +        return false;
>> +    }
>
>I think this one should go in patch 5/13 as it is not related to accel

Yes, make sense, will do.

Thanks
Zhenzhong