1. Patchew
  2. QEMU
  3. [PATCH v2 00/14] intel_iommu: Enable PASID support for passthrough device
  4. View patch

[PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check

Zhenzhong Duan posted 14 patches 1 week 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@bull.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
[PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check
Posted by Zhenzhong Duan 1 week 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_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 f3cb6cff1c..d11064b527 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_accel.c b/hw/i386/intel_iommu_accel.c
index 2fd26690b9..e73695ff83 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 v2 13/14] intel_iommu_accel: Add pasid bits size check
Posted by Yi Liu 6 days, 17 hours ago 
On 3/26/26 17:11, 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.

this has been enforced in the prior patch. right? hence just drop it. Or
you may be merge them into one patch.

> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f3cb6cff1c..d11064b527 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)

perhaps VTD_ECAP_GET_PSS. I'm also wondering if we should have a set
of macros to get other caps as well. Like "s->ecap & VTD_ECAP_SMTS"
may be VTD_EAP_GET_SMTS(s->ecap, VTD_ECAP_SMTS)".

>   #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 2fd26690b9..e73695ff83 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 v2 13/14] intel_iommu_accel: Add pasid bits size check
Posted by Duan, Zhenzhong 3 days, 13 hours ago 

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check
>
>On 3/26/26 17:11, 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.
>
>this has been enforced in the prior patch. right? hence just drop it. Or
>you may be merge them into one patch.

Yes, will delete it.

>
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f3cb6cff1c..d11064b527 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)
>
>perhaps VTD_ECAP_GET_PSS. I'm also wondering if we should have a set
>of macros to get other caps as well. Like "s->ecap & VTD_ECAP_SMTS"
>may be VTD_EAP_GET_SMTS(s->ecap, VTD_ECAP_SMTS)".

Currently VTD_ECAP_PSS is an implicit VTD_ECAP_GET_PSS like other definitions.

I'm not sure if we should follow arm's style to use VTD_ECAP_GET_PSS or vtd's style to use VTD_ECAP_PSS.

Thanks
Zhenzhong

>
>>   #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 2fd26690b9..e73695ff83 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 v2 13/14] intel_iommu_accel: Add pasid bits size check
Posted by Yi Liu 3 days, 11 hours ago 
On 3/30/26 17:06, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Subject: Re: [PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check
>>
>> On 3/26/26 17:11, 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.
>>
>> this has been enforced in the prior patch. right? hence just drop it. Or
>> you may be merge them into one patch.
> 
> Yes, will delete it.
> 
>>
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f3cb6cff1c..d11064b527 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)
>>
>> perhaps VTD_ECAP_GET_PSS. I'm also wondering if we should have a set
>> of macros to get other caps as well. Like "s->ecap & VTD_ECAP_SMTS"
>> may be VTD_EAP_GET_SMTS(s->ecap, VTD_ECAP_SMTS)".
> 
> Currently VTD_ECAP_PSS is an implicit VTD_ECAP_GET_PSS like other definitions.
> 
> I'm not sure if we should follow arm's style to use VTD_ECAP_GET_PSS or vtd's style to use VTD_ECAP_PSS.

Hmmm. The existing list is already a mixture.. Some covers both SET and
GET depending on how caller uses it, while some only covers SET (like
VTD_ECAP_MHMV and VTD_ECAP_IRO). Then adding the PSS macros here is 
fine. You may use a explicit GET/SET for PSS if you don't have a good
idea. :)

/* ECAP_REG */
/* (offset >> 4) << 8 */
#define VTD_ECAP_IRO                (DMAR_IOTLB_REG_OFFSET << 4)
#define VTD_ECAP_QI                 (1ULL << 1)
#define VTD_ECAP_DT                 (1ULL << 2)
/* Interrupt Remapping support */
#define VTD_ECAP_IR                 (1ULL << 3)
#define VTD_ECAP_EIM                (1ULL << 4)
#define VTD_ECAP_PT                 (1ULL << 6)
#define VTD_ECAP_SC                 (1ULL << 7)
#define VTD_ECAP_PRS                (1ULL << 29)
#define VTD_ECAP_MHMV               (15ULL << 20)
#define VTD_ECAP_SRS                (1ULL << 31)
#define VTD_ECAP_NWFS               (1ULL << 33)
#define VTD_ECAP_PSS                (7ULL << 35) /* limit: 
MemTxAttrs::pid */
#define VTD_ECAP_PASID              (1ULL << 40)
#define VTD_ECAP_PDS                (1ULL << 42)
#define VTD_ECAP_SMTS               (1ULL << 43)
#define VTD_ECAP_SSTS               (1ULL << 46)
#define VTD_ECAP_FSTS               (1ULL << 47)

> 
>>
>>>    #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 2fd26690b9..e73695ff83 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 v2 13/14] intel_iommu_accel: Add pasid bits size check
Posted by Duan, Zhenzhong 2 days, 17 hours ago 

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check
>
>On 3/30/26 17:06, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>> Subject: Re: [PATCH v2 13/14] intel_iommu_accel: Add pasid bits size check
>>>
>>> On 3/26/26 17:11, 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.
>>>
>>> this has been enforced in the prior patch. right? hence just drop it. Or
>>> you may be merge them into one patch.
>>
>> Yes, will delete it.
>>
>>>
>>>>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.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 f3cb6cff1c..d11064b527 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)
>>>
>>> perhaps VTD_ECAP_GET_PSS. I'm also wondering if we should have a set
>>> of macros to get other caps as well. Like "s->ecap & VTD_ECAP_SMTS"
>>> may be VTD_EAP_GET_SMTS(s->ecap, VTD_ECAP_SMTS)".
>>
>> Currently VTD_ECAP_PSS is an implicit VTD_ECAP_GET_PSS like other
>definitions.
>>
>> I'm not sure if we should follow arm's style to use VTD_ECAP_GET_PSS or vtd's
>style to use VTD_ECAP_PSS.
>
>Hmmm. The existing list is already a mixture.. Some covers both SET and
>GET depending on how caller uses it, while some only covers SET (like
>VTD_ECAP_MHMV and VTD_ECAP_IRO). Then adding the PSS macros here is
>fine. You may use a explicit GET/SET for PSS if you don't have a good
>idea. :)

OK, will use GET/SET pair for PSS.

Thanks
Zhenzhong

>
>/* ECAP_REG */
>/* (offset >> 4) << 8 */
>#define VTD_ECAP_IRO                (DMAR_IOTLB_REG_OFFSET << 4)
>#define VTD_ECAP_QI                 (1ULL << 1)
>#define VTD_ECAP_DT                 (1ULL << 2)
>/* Interrupt Remapping support */
>#define VTD_ECAP_IR                 (1ULL << 3)
>#define VTD_ECAP_EIM                (1ULL << 4)
>#define VTD_ECAP_PT                 (1ULL << 6)
>#define VTD_ECAP_SC                 (1ULL << 7)
>#define VTD_ECAP_PRS                (1ULL << 29)
>#define VTD_ECAP_MHMV               (15ULL << 20)
>#define VTD_ECAP_SRS                (1ULL << 31)
>#define VTD_ECAP_NWFS               (1ULL << 33)
>#define VTD_ECAP_PSS                (7ULL << 35) /* limit:
>MemTxAttrs::pid */
>#define VTD_ECAP_PASID              (1ULL << 40)
>#define VTD_ECAP_PDS                (1ULL << 42)
>#define VTD_ECAP_SMTS               (1ULL << 43)
>#define VTD_ECAP_SSTS               (1ULL << 46)
>#define VTD_ECAP_FSTS               (1ULL << 47)
>
>>
>>>
>>>>    #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 2fd26690b9..e73695ff83 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");
>>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.