[PATCH v2 09/14] intel_iommu_accel: Bypass PASID entry addition for just deleted entry

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 09/14] intel_iommu_accel: Bypass PASID entry addition for just deleted entry
Posted by Zhenzhong Duan 1 week ago
For VTD_INV_DESC_PASIDC_G_PASID_SI typed pc_inv_dsc invalidation, if an
pasid entry is just removed, it can never be a new entry to add. So
calling vtd_replay_pasid_bind_for_dev() is unnecessary.

Introduce a new field accel_pce_deleted in VTDPASIDCacheInfo to mark
this case and to do the bypassing.

Suggested-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu_internal.h |  1 +
 hw/i386/intel_iommu_accel.c    | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index d5f212ded9..f3cb6cff1c 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -631,6 +631,7 @@ typedef struct VTDPASIDCacheInfo {
     uint8_t type;
     uint16_t did;
     uint32_t pasid;
+    bool accel_pce_deleted;
 } VTDPASIDCacheInfo;
 
 typedef struct VTDPIOTLBInvInfo {
diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
index c1285ce331..1e27c0feb8 100644
--- a/hw/i386/intel_iommu_accel.c
+++ b/hw/i386/intel_iommu_accel.c
@@ -311,6 +311,10 @@ static void vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry *vtd_pce,
          */
         QLIST_REMOVE(vtd_pce, next);
         g_free(vtd_pce);
+
+        if (pc_info->type == VTD_INV_DESC_PASIDC_G_PASID_SI) {
+            pc_info->accel_pce_deleted = true;
+        }
     }
 }
 
@@ -498,7 +502,12 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState *s, VTDPASIDCacheInfo *pc_info)
          * removed.
          */
         vtd_pasid_cache_invalidate(vtd_hiod, pc_info);
-        vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
+
+        if (pc_info->accel_pce_deleted) {
+            pc_info->accel_pce_deleted = false;
+        } else {
+            vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
+        }
     }
 }
 
-- 
2.47.3
Re: [PATCH v2 09/14] intel_iommu_accel: Bypass PASID entry addition for just deleted entry
Posted by CLEMENT MATHIEU--DRIF 2 days, 15 hours ago
On Thu, 2026-03-26 at 05:11 -0400, Zhenzhong Duan wrote:
> For VTD_INV_DESC_PASIDC_G_PASID_SI typed pc_inv_dsc invalidation, if an  
> pasid entry is just removed, it can never be a new entry to add. So  
> calling vtd_replay_pasid_bind_for_dev() is unnecessary.
> 
> Introduce a new field accel_pce_deleted in VTDPASIDCacheInfo to mark  
> this case and to do the bypassing.
> 
> Suggested-by: Yi Liu <[yi.l.liu@intel.com](mailto:yi.l.liu@intel.com)>  
> Signed-off-by: Zhenzhong Duan <[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)>  
> ---  
>  hw/i386/intel_iommu_internal.h |  1 +  
>  hw/i386/intel_iommu_accel.c    | 11 ++++++++++-  
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h  
> index d5f212ded9..f3cb6cff1c 100644  
> --- a/hw/i386/intel_iommu_internal.h  
> +++ b/hw/i386/intel_iommu_internal.h  
> @@ -631,6 +631,7 @@ typedef struct VTDPASIDCacheInfo {  
>      uint8_t type;  
>      uint16_t did;  
>      uint32_t pasid;  
> +    bool accel_pce_deleted;  
>  } VTDPASIDCacheInfo;  
>    
>  typedef struct VTDPIOTLBInvInfo {  
> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c  
> index c1285ce331..1e27c0feb8 100644  
> --- a/hw/i386/intel_iommu_accel.c  
> +++ b/hw/i386/intel_iommu_accel.c  
> @@ -311,6 +311,10 @@ static void vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry *vtd_pce,  
>           */  
>          QLIST_REMOVE(vtd_pce, next);  
>          g_free(vtd_pce);  
> +  
> +        if (pc_info->type == VTD_INV_DESC_PASIDC_G_PASID_SI) {  
> +            pc_info->accel_pce_deleted = true;  
> +        }  
>      }  
>  }  
>    
> @@ -498,7 +502,12 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState *s, VTDPASIDCacheInfo *pc_info)  
>           * removed.  
>           */  
>          vtd_pasid_cache_invalidate(vtd_hiod, pc_info);  
> -        vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);  
> +  
> +        if (pc_info->accel_pce_deleted) {  
> +            pc_info->accel_pce_deleted = false;  

Are we doing this to make sure that nobody can see the side effect from outside the function?

> +        } else {  
> +            vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);  
> +        }  
>      }  
>  }  
>  
RE: [PATCH v2 09/14] intel_iommu_accel: Bypass PASID entry addition for just deleted entry
Posted by Duan, Zhenzhong 2 days, 15 hours ago

>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@bull.com>
>Subject: Re: [PATCH v2 09/14] intel_iommu_accel: Bypass PASID entry addition for
>just deleted entry
>
>
>On Thu, 2026-03-26 at 05:11 -0400, Zhenzhong Duan wrote:
>> For VTD_INV_DESC_PASIDC_G_PASID_SI typed pc_inv_dsc invalidation, if an
>> pasid entry is just removed, it can never be a new entry to add. So
>> calling vtd_replay_pasid_bind_for_dev() is unnecessary.
>>
>> Introduce a new field accel_pce_deleted in VTDPASIDCacheInfo to mark
>> this case and to do the bypassing.
>>
>> Suggested-by: Yi Liu <[yi.l.liu@intel.com](mailto:yi.l.liu@intel.com)>
>> Signed-off-by: Zhenzhong Duan
><[zhenzhong.duan@intel.com](mailto:zhenzhong.duan@intel.com)>
>> ---
>>  hw/i386/intel_iommu_internal.h |  1 +
>>  hw/i386/intel_iommu_accel.c    | 11 ++++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index d5f212ded9..f3cb6cff1c 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -631,6 +631,7 @@ typedef struct VTDPASIDCacheInfo {
>>      uint8_t type;
>>      uint16_t did;
>>      uint32_t pasid;
>> +    bool accel_pce_deleted;
>>  } VTDPASIDCacheInfo;
>>
>>  typedef struct VTDPIOTLBInvInfo {
>> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
>> index c1285ce331..1e27c0feb8 100644
>> --- a/hw/i386/intel_iommu_accel.c
>> +++ b/hw/i386/intel_iommu_accel.c
>> @@ -311,6 +311,10 @@ static void
>vtd_pasid_cache_invalidate_one(VTDAccelPASIDCacheEntry *vtd_pce,
>>           */
>>          QLIST_REMOVE(vtd_pce, next);
>>          g_free(vtd_pce);
>> +
>> +        if (pc_info->type == VTD_INV_DESC_PASIDC_G_PASID_SI) {
>> +            pc_info->accel_pce_deleted = true;
>> +        }
>>      }
>>  }
>>
>> @@ -498,7 +502,12 @@ void vtd_pasid_cache_sync_accel(IntelIOMMUState
>*s, VTDPASIDCacheInfo *pc_info)
>>           * removed.
>>           */
>>          vtd_pasid_cache_invalidate(vtd_hiod, pc_info);
>> -        vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
>> +
>> +        if (pc_info->accel_pce_deleted) {
>> +            pc_info->accel_pce_deleted = false;
>
>Are we doing this to make sure that nobody can see the side effect from outside
>the function?

In each iteration of the while loop, we want to ensure pc_info->accel_pce_deleted is initially false, then vtd_accel_pasid_cache_invalidate() will make decision to set it or not. Then we check it to determine if we need to call vtd_accel_replay_pasid_bind_for_dev().

Thanks
Zhenzhong

>
>> +        } else {
>> +            vtd_replay_pasid_bind_for_dev(vtd_hiod, start, end, pc_info);
>> +        }
>>      }
>>  }
>>