Re: [PATCH v5 10/15] intel_iommu_accel: Handle PASID entry removal for pc_inv_dsc request

[Yi Liu]

On 5/22/26 11:07, Duan, Zhenzhong wrote:

-----Original Message-----
From: Liu, Yi L <yi.l....@intel.com>
Subject: Re: [PATCH v5 10/15] intel_iommu_accel: Handle PASID entry removal for
pc_inv_dsc request

On 5/9/26 12:08, 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.

Co-developed-by: Yi Liu <yi.l....@intel.com>
Signed-off-by: Yi Liu <yi.l....@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Tested-by: Xudong Hao <xudong....@intel.com>
---
   hw/i386/intel_iommu_accel.c | 80

+++++++++++++++++++++++++++++++++++++

   1 file changed, 80 insertions(+)

diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
index a66d63b4c8..82bfbdf484 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 int vtd_hiod_get_pe_from_pasid(VTDAccelPASIDCacheEntry *vtd_pce,

this helper is tricky. vtd_pce already has a pe cached, then why need
to do this?

It's used on pc_inv_dsc handler path, vtd_pce caches old pe, we need to get 
refreshed one in memory.

I think it's better to have a helper receives s, devfn, and
pasid instead of accepting vtd_pce. btw. I think the intel_iommu.c
should have such a helper. is it?

In intel_iommu.c, there is a helper for vtd_as:

static inline int vtd_dev_get_pe_from_pasid(VTDAddressSpace *vtd_as,
                                             VTDPASIDEntry *pe)

so in intel_iommu_accel.c, vtd_hiod_get_pe_from_pasid() is added for vtd_pce.

got it. VTDAddressSpace also cached a pe, with a closer read, it's
tricky as well. Could you refactor a bit? e.g. avoid using the
VTDAddressSpace as an input.

+                                      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)
   {
@@ -280,6 +302,57 @@ static void vtd_accel_fill_pc(VTDHostIOMMUDevice

*vtd_hiod, uint32_t pasid,

       QLIST_INSERT_HEAD(&vtd_hiod->pasid_cache_list, vtd_pce, next);
   }

+static void vtd_accel_delete_pc(VTDAccelPASIDCacheEntry *vtd_pce)
+{
+    QLIST_REMOVE(vtd_pce, next);
+    g_free(vtd_pce);
+}
+
+static void
+vtd_accel_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.
+         */
+        vtd_accel_delete_pc(vtd_pce);
+    }
+}
+
+static void vtd_accel_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_accel_pasid_cache_invalidate_one(vtd_pce, pc_info);
+    }
+}
+
   /*
    * This function walks over PASID range within [start, end) in a single
    * PASID table for entries matching @info type/did, then create
@@ -411,6 +484,13 @@ void vtd_accel_pasid_cache_sync(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.

how about below? :)

/*
  * The replay path inevitably needs to iterate through existing
  * PASID cache entries. Since cached PASID entries that are marked
  * for removal don't need to be iterated, we intentionally handle
  * removals before additions to optimize the replay process.
  */

+         */
+        vtd_accel_pasid_cache_invalidate(vtd_hiod, pc_info);
           vtd_accel_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
       }
   }