[PATCH v2 08/14] intel_iommu_accel: Handle PASID entry removal for pc_inv_dsc request

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 08/14] intel_iommu_accel: Handle PASID entry removal for pc_inv_dsc request
Posted by Zhenzhong Duan 1 week ago
When guest deletes PASID entries, QEMU will capture the pasid cache
invalidation request, walk through pasid_cache_list in each passthrough
device to find stale VTDAccelPASIDCacheEntry and delete them.

This happen before the PASID entry addition, because a new added entry
should never be removed.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu_accel.c | 75 +++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
index 32d8ab0ef9..c1285ce331 100644
--- a/hw/i386/intel_iommu_accel.c
+++ b/hw/i386/intel_iommu_accel.c
@@ -16,6 +16,28 @@
 #include "hw/pci/pci_bus.h"
 #include "trace.h"
 
+static inline int vtd_hiod_get_pe_from_pasid(VTDAccelPASIDCacheEntry *vtd_pce,
+                                             VTDPASIDEntry *pe)
+{
+    VTDHostIOMMUDevice *vtd_hiod = vtd_pce->vtd_hiod;
+    IntelIOMMUState *s = vtd_hiod->iommu_state;
+    uint32_t pasid = vtd_pce->pasid;
+    VTDContextEntry ce;
+    int ret;
+
+    if (!s->dmar_enabled || !s->root_scalable) {
+        return -VTD_FR_RTADDR_INV_TTM;
+    }
+
+    ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_hiod->bus),
+                                   vtd_hiod->devfn, &ce);
+    if (ret) {
+        return ret;
+    }
+
+    return vtd_ce_get_pasid_entry(s, &ce, pe, pasid);
+}
+
 bool vtd_check_hiod_accel(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hiod,
                           Error **errp)
 {
@@ -257,6 +279,52 @@ void vtd_flush_host_piotlb_all_locked(IntelIOMMUState *s, uint16_t domain_id,
                          vtd_flush_host_piotlb_locked, &piotlb_info);
 }
 
+static void vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry *vtd_pce,
+                                           VTDPASIDCacheInfo *pc_info)
+{
+    VTDPASIDEntry pe;
+    uint16_t did;
+
+    /*
+     * VTD_INV_DESC_PASIDC_G_DSI and VTD_INV_DESC_PASIDC_G_PASID_SI require
+     * DID check. If DID doesn't match the value in cache or memory, then
+     * it's not a pasid entry we want to invalidate.
+     */
+    switch (pc_info->type) {
+    case VTD_INV_DESC_PASIDC_G_PASID_SI:
+        if (pc_info->pasid != vtd_pce->pasid) {
+            return;
+        }
+        /* Fall through */
+    case VTD_INV_DESC_PASIDC_G_DSI:
+        did = VTD_SM_PASID_ENTRY_DID(&vtd_pce->pasid_entry);
+        if (pc_info->did != did) {
+            return;
+        }
+    }
+
+    if (vtd_hiod_get_pe_from_pasid(vtd_pce, &pe)) {
+        /*
+         * No valid pasid entry in guest memory. e.g. pasid entry was modified
+         * to be either all-zero or non-present. Either case means existing
+         * pasid cache should be invalidated.
+         */
+        QLIST_REMOVE(vtd_pce, next);
+        g_free(vtd_pce);
+    }
+}
+
+/* Delete invalid pasid cache entry from pasid_cache_list */
+static void vtd_pasid_cache_invalidate(VTDHostIOMMUDevice *vtd_hiod,
+                                       VTDPASIDCacheInfo *pc_info)
+{
+    VTDAccelPASIDCacheEntry *vtd_pce, *next;
+
+    QLIST_FOREACH_SAFE(vtd_pce, &vtd_hiod->pasid_cache_list, next, next) {
+        vtd_pasid_cache_invalidate_one(vtd_pce, pc_info);
+    }
+}
+
 static void vtd_accel_fill_pc(VTDHostIOMMUDevice *vtd_hiod, uint32_t pasid,
                               VTDPASIDEntry *pe)
 {
@@ -423,6 +491,13 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState *s, VTDPASIDCacheInfo *pc_info)
                                  TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
             continue;
         }
+
+        /*
+         * PASID entry removal is handled before addition intentionally,
+         * because it's unnecessary to iterate on an entry that will be
+         * removed.
+         */
+        vtd_pasid_cache_invalidate(vtd_hiod, pc_info);
         vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
     }
 }
-- 
2.47.3
Re: [PATCH v2 08/14] intel_iommu_accel: Handle PASID entry removal for pc_inv_dsc request
Posted by Yi Liu 6 days, 17 hours ago
On 3/26/26 17:11, Zhenzhong Duan wrote:
> When guest deletes PASID entries, QEMU will capture the pasid cache
> invalidation request, walk through pasid_cache_list in each passthrough
> device to find stale VTDAccelPASIDCacheEntry and delete them.

> This happen before the PASID entry addition, because a new added entry
> should never be removed.

drop above line.

> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu_accel.c | 75 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
> index 32d8ab0ef9..c1285ce331 100644
> --- a/hw/i386/intel_iommu_accel.c
> +++ b/hw/i386/intel_iommu_accel.c
> @@ -16,6 +16,28 @@
>   #include "hw/pci/pci_bus.h"
>   #include "trace.h"
>   
> +static inline int vtd_hiod_get_pe_from_pasid(VTDAccelPASIDCacheEntry *vtd_pce,
> +                                             VTDPASIDEntry *pe)
> +{
> +    VTDHostIOMMUDevice *vtd_hiod = vtd_pce->vtd_hiod;
> +    IntelIOMMUState *s = vtd_hiod->iommu_state;
> +    uint32_t pasid = vtd_pce->pasid;
> +    VTDContextEntry ce;
> +    int ret;
> +
> +    if (!s->dmar_enabled || !s->root_scalable) {
> +        return -VTD_FR_RTADDR_INV_TTM;
> +    }
> +
> +    ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_hiod->bus),
> +                                   vtd_hiod->devfn, &ce);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    return vtd_ce_get_pasid_entry(s, &ce, pe, pasid);
> +}
> +
>   bool vtd_check_hiod_accel(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hiod,
>                             Error **errp)
>   {
> @@ -257,6 +279,52 @@ void vtd_flush_host_piotlb_all_locked(IntelIOMMUState *s, uint16_t domain_id,
>                            vtd_flush_host_piotlb_locked, &piotlb_info);
>   }
>   
> +static void vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry *vtd_pce,
> +                                           VTDPASIDCacheInfo *pc_info)
> +{
> +    VTDPASIDEntry pe;
> +    uint16_t did;
> +
> +    /*
> +     * VTD_INV_DESC_PASIDC_G_DSI and VTD_INV_DESC_PASIDC_G_PASID_SI require
> +     * DID check. If DID doesn't match the value in cache or memory, then
> +     * it's not a pasid entry we want to invalidate.
> +     */
> +    switch (pc_info->type) {
> +    case VTD_INV_DESC_PASIDC_G_PASID_SI:
> +        if (pc_info->pasid != vtd_pce->pasid) {
> +            return;
> +        }
> +        /* Fall through */
> +    case VTD_INV_DESC_PASIDC_G_DSI:
> +        did = VTD_SM_PASID_ENTRY_DID(&vtd_pce->pasid_entry);
> +        if (pc_info->did != did) {
> +            return;
> +        }
> +    }
> +
> +    if (vtd_hiod_get_pe_from_pasid(vtd_pce, &pe)) {
> +        /*
> +         * No valid pasid entry in guest memory. e.g. pasid entry was modified
> +         * to be either all-zero or non-present. Either case means existing
> +         * pasid cache should be invalidated.
> +         */
> +        QLIST_REMOVE(vtd_pce, next);
> +        g_free(vtd_pce);

could you wrap above two lines into a helper near to the
vtd_accel_fill_pc()? Although no other callers, just more readable. :)

> +    }
> +}
> +
> +/* Delete invalid pasid cache entry from pasid_cache_list */

above comment is not quite necessary.

> +static void vtd_pasid_cache_invalidate(VTDHostIOMMUDevice *vtd_hiod,
> +                                       VTDPASIDCacheInfo *pc_info)
> +{
> +    VTDAccelPASIDCacheEntry *vtd_pce, *next;
> +
> +    QLIST_FOREACH_SAFE(vtd_pce, &vtd_hiod->pasid_cache_list, next, next) {
> +        vtd_pasid_cache_invalidate_one(vtd_pce, pc_info);
> +    }
> +}
> +
>   static void vtd_accel_fill_pc(VTDHostIOMMUDevice *vtd_hiod, uint32_t pasid,
>                                 VTDPASIDEntry *pe)
>   {
> @@ -423,6 +491,13 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState *s, VTDPASIDCacheInfo *pc_info)
>                                    TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>               continue;
>           }
> +
> +        /*
> +         * PASID entry removal is handled before addition intentionally,
> +         * because it's unnecessary to iterate on an entry that will be
> +         * removed.
> +         */
> +        vtd_pasid_cache_invalidate(vtd_hiod, pc_info);

s/vtd_pasid_cache_invalidate/vtd_accel_pasid_cache_invalidate/

>           vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
>       }
>   }
RE: [PATCH v2 08/14] intel_iommu_accel: Handle PASID entry removal for pc_inv_dsc request
Posted by Duan, Zhenzhong 3 days, 15 hours ago

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 08/14] intel_iommu_accel: Handle PASID entry removal for
>pc_inv_dsc request
>
>On 3/26/26 17:11, Zhenzhong Duan wrote:
>> When guest deletes PASID entries, QEMU will capture the pasid cache
>> invalidation request, walk through pasid_cache_list in each passthrough
>> device to find stale VTDAccelPASIDCacheEntry and delete them.
>
>> This happen before the PASID entry addition, because a new added entry
>> should never be removed.
>
>drop above line.

OK.

>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu_accel.c | 75
>+++++++++++++++++++++++++++++++++++++
>>   1 file changed, 75 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
>> index 32d8ab0ef9..c1285ce331 100644
>> --- a/hw/i386/intel_iommu_accel.c
>> +++ b/hw/i386/intel_iommu_accel.c
>> @@ -16,6 +16,28 @@
>>   #include "hw/pci/pci_bus.h"
>>   #include "trace.h"
>>
>> +static inline int vtd_hiod_get_pe_from_pasid(VTDAccelPASIDCacheEntry
>*vtd_pce,
>> +                                             VTDPASIDEntry *pe)
>> +{
>> +    VTDHostIOMMUDevice *vtd_hiod = vtd_pce->vtd_hiod;
>> +    IntelIOMMUState *s = vtd_hiod->iommu_state;
>> +    uint32_t pasid = vtd_pce->pasid;
>> +    VTDContextEntry ce;
>> +    int ret;
>> +
>> +    if (!s->dmar_enabled || !s->root_scalable) {
>> +        return -VTD_FR_RTADDR_INV_TTM;
>> +    }
>> +
>> +    ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_hiod->bus),
>> +                                   vtd_hiod->devfn, &ce);
>> +    if (ret) {
>> +        return ret;
>> +    }
>> +
>> +    return vtd_ce_get_pasid_entry(s, &ce, pe, pasid);
>> +}
>> +
>>   bool vtd_check_hiod_accel(IntelIOMMUState *s, VTDHostIOMMUDevice
>*vtd_hiod,
>>                             Error **errp)
>>   {
>> @@ -257,6 +279,52 @@ void
>vtd_flush_host_piotlb_all_locked(IntelIOMMUState *s, uint16_t domain_id,
>>                            vtd_flush_host_piotlb_locked, &piotlb_info);
>>   }
>>
>> +static void vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry
>*vtd_pce,
>> +                                           VTDPASIDCacheInfo *pc_info)
>> +{
>> +    VTDPASIDEntry pe;
>> +    uint16_t did;
>> +
>> +    /*
>> +     * VTD_INV_DESC_PASIDC_G_DSI and VTD_INV_DESC_PASIDC_G_PASID_SI
>require
>> +     * DID check. If DID doesn't match the value in cache or memory, then
>> +     * it's not a pasid entry we want to invalidate.
>> +     */
>> +    switch (pc_info->type) {
>> +    case VTD_INV_DESC_PASIDC_G_PASID_SI:
>> +        if (pc_info->pasid != vtd_pce->pasid) {
>> +            return;
>> +        }
>> +        /* Fall through */
>> +    case VTD_INV_DESC_PASIDC_G_DSI:
>> +        did = VTD_SM_PASID_ENTRY_DID(&vtd_pce->pasid_entry);
>> +        if (pc_info->did != did) {
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (vtd_hiod_get_pe_from_pasid(vtd_pce, &pe)) {
>> +        /*
>> +         * No valid pasid entry in guest memory. e.g. pasid entry was modified
>> +         * to be either all-zero or non-present. Either case means existing
>> +         * pasid cache should be invalidated.
>> +         */
>> +        QLIST_REMOVE(vtd_pce, next);
>> +        g_free(vtd_pce);
>
>could you wrap above two lines into a helper near to the
>vtd_accel_fill_pc()? Although no other callers, just more readable. :)

Sure, will do.

>
>> +    }
>> +}
>> +
>> +/* Delete invalid pasid cache entry from pasid_cache_list */
>
>above comment is not quite necessary.

Will drop.

>
>> +static void vtd_pasid_cache_invalidate(VTDHostIOMMUDevice *vtd_hiod,
>> +                                       VTDPASIDCacheInfo *pc_info)
>> +{
>> +    VTDAccelPASIDCacheEntry *vtd_pce, *next;
>> +
>> +    QLIST_FOREACH_SAFE(vtd_pce, &vtd_hiod->pasid_cache_list, next, next) {
>> +        vtd_pasid_cache_invalidate_one(vtd_pce, pc_info);
>> +    }
>> +}
>> +
>>   static void vtd_accel_fill_pc(VTDHostIOMMUDevice *vtd_hiod, uint32_t pasid,
>>                                 VTDPASIDEntry *pe)
>>   {
>> @@ -423,6 +491,13 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState
>*s, VTDPASIDCacheInfo *pc_info)
>>                                    TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>>               continue;
>>           }
>> +
>> +        /*
>> +         * PASID entry removal is handled before addition intentionally,
>> +         * because it's unnecessary to iterate on an entry that will be
>> +         * removed.
>> +         */
>> +        vtd_pasid_cache_invalidate(vtd_hiod, pc_info);
>
>s/vtd_pasid_cache_invalidate/vtd_accel_pasid_cache_invalidate/

OK.

Thanks
Zhenzhong