>-----Original Message----- >From: Liu, Yi L <[email protected]> >Subject: Re: [PATCH v5 11/21] intel_iommu: Handle PASID entry removal and >update > >On 2025/8/22 14:40, Zhenzhong Duan wrote: >> This 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. > >Have you seen any extra code needed based on this series to support non >rid_pasid PASIDs? If no, may just relax the scope of this series. >otherwise, you may need to tweak the patch a little bit. e.g. factor >out setting x-flts and x-pasid-mode at the same time.
There are quite a few code are common for both non-rid_pasid and rid_pasid. So in this series, there are some infrastructure code that looks like it's for non-rid_pasid. But to support non-rid_pasid, we need pasid_attach/detach() which is not implemented in this series. Even if x-flts and x-pasid-mode both on, pasid isn't enabled since VFIO device doesn't expose pasid capability to guest, so guest never use non-rid_pasid with this VFIO device. > >> >> VTDAddressSpace of PCI_NO_PASID is allocated when device is plugged and >> never freed. For other pasid, VTDAddressSpace instance is >created/destroyed >> per the guest pasid entry set up/destroy. > >> When guest removes or updates a PASID entry, QEMU will capture the guest >pasid >> selective pasid cache invalidation, removes VTDAddressSpace or update >cached >> PASID entry. >> >> vIOMMU emulator could figure out the reason by fetching latest guest pasid >entry >> and compare it with cached PASID entry. >> >> Signed-off-by: Yi Liu <[email protected]> >> Signed-off-by: Yi Sun <[email protected]> >> Signed-off-by: Zhenzhong Duan <[email protected]> >> --- >> hw/i386/intel_iommu_internal.h | 27 ++++- >> include/hw/i386/intel_iommu.h | 6 + >> hw/i386/intel_iommu.c | 196 >+++++++++++++++++++++++++++++++-- >> hw/i386/trace-events | 3 + >> 4 files changed, 220 insertions(+), 12 deletions(-) >> >> diff --git a/hw/i386/intel_iommu_internal.h >b/hw/i386/intel_iommu_internal.h >> index f7510861d1..b9b76dd996 100644 >> --- a/hw/i386/intel_iommu_internal.h >> +++ b/hw/i386/intel_iommu_internal.h >> @@ -316,6 +316,7 @@ 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 */ >> /* PASID directory entry access failure */ >> VTD_FR_PASID_DIR_ACCESS_ERR = 0x50, >> /* The Present(P) field of pasid directory entry is 0 */ >> @@ -493,6 +494,15 @@ typedef union VTDInvDesc VTDInvDesc; >> #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 >> + >> /* Information about page-selective IOTLB invalidate */ >> struct VTDIOTLBPageInvInfo { >> uint16_t domain_id; >> @@ -553,6 +563,21 @@ typedef struct VTDRootEntry VTDRootEntry; >> #define VTD_SM_CONTEXT_ENTRY_RSVD_VAL0(aw) (0x1e0ULL | >~VTD_HAW_MASK(aw)) >> #define VTD_SM_CONTEXT_ENTRY_RSVD_VAL1 >0xffffffffffe00000ULL >> >> +typedef enum VTDPCInvType { >> + /* VTD spec defined PASID cache invalidation type */ >> + VTD_PASID_CACHE_DOMSI = VTD_INV_DESC_PASIDC_G_DSI, >> + VTD_PASID_CACHE_PASIDSI = VTD_INV_DESC_PASIDC_G_PASID_SI, >> + VTD_PASID_CACHE_GLOBAL_INV = >VTD_INV_DESC_PASIDC_G_GLOBAL, >> +} VTDPCInvType; >> + >> +typedef struct VTDPASIDCacheInfo { >> + VTDPCInvType type; >> + uint16_t did; >> + uint32_t pasid; >> + PCIBus *bus; >> + uint16_t devfn; >> +} VTDPASIDCacheInfo; >> + >> /* PASID Table Related Definitions */ >> #define VTD_PASID_DIR_BASE_ADDR_MASK (~0xfffULL) >> #define VTD_PASID_TABLE_BASE_ADDR_MASK (~0xfffULL) >> @@ -574,7 +599,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) >> >> #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 50f9b27a45..0e3826f6f0 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 1801f1cdf6..a2ee6d684e 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -1675,7 +1675,7 @@ static uint16_t >vtd_get_domain_id(IntelIOMMUState *s, >> >> if (s->root_scalable) { >> vtd_ce_get_pasid_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); >> @@ -3112,6 +3112,183 @@ static bool >vtd_process_piotlb_desc(IntelIOMMUState *s, >> return true; >> } >> >> +static inline int vtd_dev_get_pe_from_pasid(VTDAddressSpace *vtd_as, >> + uint32_t pasid, >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_pasid_entry(s, &ce, pe, pasid); >> +} >> + >> +static bool vtd_pasid_entry_compare(VTDPASIDEntry *p1, VTDPASIDEntry >*p2) >> +{ >> + return !memcmp(p1, p2, sizeof(*p1)); >> +} >> + >> +/* >> + * This function is a loop function which return value determines if >> + * vtd_as including cached pasid entry is removed. >> + * >> + * For PCI_NO_PASID, when corresponding cached pasid entry is cleared, >> + * it returns false so that vtd_as is reserved as it's owned by PCI >> + * sub-system. For other pasid, it returns true so vtd_as is removed. > >also, this helper will always return true if this series does not >support non-rid_pasid PASID. Do you mean return false? I don't think it will return true. For non-rid_pasid, it may return false. Thanks Zhenzhong
