>-----Original Message-----
>From: Liu, Yi L <[email protected]>
>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 <[email protected]>
>> Signed-off-by: Yi Liu <[email protected]>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> Tested-by: Xudong Hao <[email protected]>
>> ---
>> 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.
Thanks
Zhenzhong
>
>> + 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);
>> }
>> }
