RE: [PATCH v1 08/13] intel_iommu: Handle PASID entry removal for pc_inv_dsc request

["Duan, Zhenzhong" <zhenzhong.duan@intel.com>]

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v1 08/13] intel_iommu: Handle PASID entry removal for
>pc_inv_dsc request
>
>On 3/6/26 11:44, 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.
>
>As the comment to the last patch, I think this patch should handle both
>update and removal of PASID entry.
>
>> This happen before the PASID entry addition, because a new added entry
>> should never be removed.
>
>This is confusing. TBH. this order is not quite important as the current
>implementation, we don't have multiple pasid entires share same cache
>entry, so a given pasid entry, we either add new cache entry or
>update/delete cache entry. So I would just drop this line.

OK, will remove it.

In fact, the order is for performance reason. Imaging there are lots of pasid entires
in pasid_cache_list, we remove stale entries first, then in addition code when we
loop on pasid_cache_list to see if a new pasid entry already exists, we can iterate
less entries.

Thanks
Zhenzhong

>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu_accel.c | 70
>+++++++++++++++++++++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
>> index 0acf3ae77f..5a956af916 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->pe);
>> +        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);
>
>call vtd_accel_update_pc() and vtd_accel_delete_pc() accordingly.
>
>> +    }
>> +}
>> +
>> +/* 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_find_add_pc(VTDHostIOMMUDevice *vtd_hiod, uint32_t pasid,
>>                               VTDPASIDEntry *pe)
>>   {
>> @@ -423,6 +491,8 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState *s,
>VTDPASIDCacheInfo *pc_info)
>>                                    TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>>               continue;
>>           }
>> +
>> +        vtd_pasid_cache_invalidate(vtd_hiod, pc_info);
>>           vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
>>       }
>>   }