>-----Original Message-----
>From: Liu, Yi L <[email protected]>
>Subject: Re: [PATCH 1/3] intel_iommu: Handle PASID cache invalidation
>
>On 2025/10/15 18:20, Zhenzhong Duan wrote:
>> Adds an new entry VTDPASIDCacheEntry in VTDAddressSpace to cache the
>pasid
>> entry and track PASID usage and future PASID tagged DMA address
>translation
>> support in vIOMMU.
>>
>> When guest triggers pasid cache invalidation, QEMU will capture it and
>> update or invalidate pasid cache.
>>
>> vIOMMU emulator could figure out the reason by fetching latest guest pasid
>> entry in memory and compare it with cached PASID entry if it's valid.
>>
>> Signed-off-by: Yi Liu <[email protected]>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> ---
>> hw/i386/intel_iommu_internal.h | 19 ++++-
>> include/hw/i386/intel_iommu.h | 6 ++
>> hw/i386/intel_iommu.c | 150
>++++++++++++++++++++++++++++++---
>> hw/i386/trace-events | 3 +
>> 4 files changed, 165 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 0f6a1237e4..80193ff28b 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -316,6 +316,8 @@ typedef enum VTDFaultReason {
>> * request while disabled */
>> VTD_FR_IR_SID_ERR = 0x26, /* Invalid Source-ID */
>>
>> + VTD_FR_RTADDR_INV_TTM = 0x31, /* Invalid TTM in RTADDR */
>> +
>> VTD_FR_SM_PRE_ABS = 0x47, /* SCT.8 : PRE bit in a present SM
>CE is 0 */
>>
>> /* PASID directory entry access failure */
>> @@ -517,6 +519,15 @@ typedef union VTDPRDesc VTDPRDesc;
>> #define VTD_INV_DESC_PIOTLB_RSVD_VAL0
>0xfff000000000f1c0ULL
>> #define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
>>
>> +/* PASID-cache Invalidate Descriptor (pc_inv_dsc) fields */
>> +#define VTD_INV_DESC_PASIDC_G(x) extract64((x)->val[0], 4, 2)
>> +#define VTD_INV_DESC_PASIDC_G_DSI 0
>> +#define VTD_INV_DESC_PASIDC_G_PASID_SI 1
>> +#define VTD_INV_DESC_PASIDC_G_GLOBAL 3
>> +#define VTD_INV_DESC_PASIDC_DID(x) extract64((x)->val[0], 16,
>16)
>> +#define VTD_INV_DESC_PASIDC_PASID(x) extract64((x)->val[0], 32,
>20)
>> +#define VTD_INV_DESC_PASIDC_RSVD_VAL0 0xfff000000000f1c0ULL
>> +
>> /* Page Request Descriptor */
>> /* For the low 64-bit of 128-bit */
>> #define VTD_PRD_TYPE (1ULL)
>> @@ -603,6 +614,12 @@ typedef struct VTDRootEntry VTDRootEntry;
>> #define VTD_SM_CONTEXT_ENTRY_RSVD_VAL1
>0xffffffffffe00000ULL
>> #define VTD_SM_CONTEXT_ENTRY_PRE 0x10ULL
>>
>> +typedef struct VTDPASIDCacheInfo {
>> + uint8_t type;
>> + uint16_t did;
>> + uint32_t pasid;
>> +} VTDPASIDCacheInfo;
>> +
>> /* PASID Table Related Definitions */
>> #define VTD_PASID_DIR_BASE_ADDR_MASK (~0xfffULL)
>> #define VTD_PASID_TABLE_BASE_ADDR_MASK (~0xfffULL)
>> @@ -624,7 +641,7 @@ typedef struct VTDRootEntry VTDRootEntry;
>> #define VTD_SM_PASID_ENTRY_PT (4ULL << 6)
>>
>> #define VTD_SM_PASID_ENTRY_AW 7ULL /* Adjusted
>guest-address-width */
>> -#define VTD_SM_PASID_ENTRY_DID(val) ((val) &
>VTD_DOMAIN_ID_MASK)
>> +#define VTD_SM_PASID_ENTRY_DID(x) extract64((x)->val[1], 0, 16)
>
>I think this can be done in a separate patch.
OK, nesting series has a patch handling this kind of cleanup, will move it
there.
>
>>
>> #define VTD_SM_PASID_ENTRY_FLPM 3ULL
>> #define VTD_SM_PASID_ENTRY_FLPTPTR (~0xfffULL)
>> diff --git a/include/hw/i386/intel_iommu.h
>b/include/hw/i386/intel_iommu.h
>> index 47730ac3c7..6e68734b3c 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -95,6 +95,11 @@ struct VTDPASIDEntry {
>> uint64_t val[8];
>> };
>>
>> +typedef struct VTDPASIDCacheEntry {
>> + struct VTDPASIDEntry pasid_entry;
>> + bool valid;
>> +} VTDPASIDCacheEntry;
>> +
>> struct VTDAddressSpace {
>> PCIBus *bus;
>> uint8_t devfn;
>> @@ -107,6 +112,7 @@ struct VTDAddressSpace {
>> MemoryRegion iommu_ir_fault; /* Interrupt region for catching
>fault */
>> IntelIOMMUState *iommu_state;
>> VTDContextCacheEntry context_cache_entry;
>> + VTDPASIDCacheEntry pasid_cache_entry;
>> QLIST_ENTRY(VTDAddressSpace) next;
>> /* Superset of notifier flags that this address space has */
>> IOMMUNotifierFlag notifier_flags;
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 6a168d5107..66f45f89cb 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1607,7 +1607,7 @@ static uint16_t
>vtd_get_domain_id(IntelIOMMUState *s,
>>
>> if (s->root_scalable) {
>> vtd_ce_get_rid2pasid_entry(s, ce, &pe, pasid);
>> - return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>> + return VTD_SM_PASID_ENTRY_DID(&pe);
>> }
>>
>> return VTD_CONTEXT_ENTRY_DID(ce->hi);
>> @@ -3051,6 +3051,135 @@ static bool
>vtd_process_piotlb_desc(IntelIOMMUState *s,
>> return true;
>> }
>>
>> +static inline int vtd_dev_get_pe_from_pasid(VTDAddressSpace *vtd_as,
>> + VTDPASIDEntry *pe)
>> +{
>> + IntelIOMMUState *s = vtd_as->iommu_state;
>> + VTDContextEntry ce;
>> + int ret;
>> +
>> + if (!s->root_scalable) {
>> + return -VTD_FR_RTADDR_INV_TTM;
>> + }
>> +
>> + ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
>vtd_as->devfn,
>> + &ce);
>> + if (ret) {
>> + return ret;
>> + }
>> +
>> + return vtd_ce_get_rid2pasid_entry(s, &ce, pe, vtd_as->pasid);
>> +}
>> +
>> +/*
>> + * Update or invalidate pasid cache based on the value in memory.
>
>s/the value in memory./the pasid entry in guest memory.
OK
>
>> + */
>> +static void vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>> + gpointer user_data)
>> +{
>> + VTDPASIDCacheInfo *pc_info = user_data;
>> + VTDAddressSpace *vtd_as = value;
>> + VTDPASIDCacheEntry *pc_entry = &vtd_as->pasid_cache_entry;
>> + VTDPASIDEntry pe;
>> + uint16_t did;
>> +
>> + if (vtd_dev_get_pe_from_pasid(vtd_as, &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.
>> + */
>> + pc_entry->valid = false;
>> + return;
>> + }
>> +
>> + /*
>> + * 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_as->pasid) {
>> + return;
>> + }
>> + /* Fall through */
>> + case VTD_INV_DESC_PASIDC_G_DSI:
>> + if (pc_entry->valid) {
>> + did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>> + if (pc_info->did == did) {
>> + break;
>> + }
>> + }
>> + did = VTD_SM_PASID_ENTRY_DID(&pe);
>> + if (pc_info->did == did) {
>> + break;
>> + }
>
>hmmm. how about below?
>
> /*
> * For newly set pasid entry, iommu driver is supposed to
> * invalidate pasid cache with the did configed in pasid entry
> * when caching-mode is reported. Oherwise qemu vIOMMU just
>skip
> * it.
> */
> if pc_entry->valid) {
> did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
> } else {
> did = VTD_SM_PASID_ENTRY_DID(&pe);
> }
>
> if (pc_info->did != did) {
> return;
> }
Yes, looks cleaner, will do.
Thanks
Zhenzhong
