RE: [PATCH v3 10/20] intel_iommu: Handle PASID entry removing and updating
>-Original Message-
>From: Eric Auger
>Subject: Re: [PATCH v3 10/20] intel_iommu: Handle PASID entry removing and
>updating
...
>> +static bool vtd_pasid_entry_compare(VTDPASIDEntry *p1, VTDPASIDEntry
>*p2)
>> +{
>> +return !memcmp(p1, p2, sizeof(*p1));
>> +}
>> +
>> +/*
>> + * This function is used to update or clear cached pasid entry in vtd_as.
>> + * vtd_as is released when corresponding cached pasid entry is cleared,
>> + * except for PCI_NO_PASID which vtd_as is owen by PCI sub-system.
>I would document when it is supposed to return true (indicates that the
>cached pasid entry needs to be removed).
Will do.
>> + */
>> +static gboolean vtd_flush_pasid_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;
>> +uint32_t pasid;
>> +int ret;
>> +
>> +if (!pc_entry->valid) {
>> +return false;
>> +}
>> +did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>> +
>> +if (vtd_as_to_iommu_pasid_locked(vtd_as, &pasid)) {
>> +goto remove;
>> +}
>> +
>> +switch (pc_info->type) {
>> +case VTD_PASID_CACHE_PASIDSI:
>> +if (pc_info->pasid != pasid) {
>> +return false;
>> +}
>> +/* fall through */
>> +case VTD_PASID_CACHE_DOMSI:
>> +if (pc_info->did != did) {
>> +return false;
>> +}
>> +/* fall through */
>> +case VTD_PASID_CACHE_GLOBAL_INV:
>> +break;
>> +default:
>> +error_setg(&error_fatal, "invalid pc_info->type for flush");
>> +}
>> +
>> +/*
>> + * pasid cache invalidation may indicate a present pasid
>> + * entry to present pasid entry modification. To cover such
>> + * case, vIOMMU emulator needs to fetch latest guest pasid
>> + * entry and compares with cached pasid entry, then update
>> + * pasid cache.
>> + */
>> +ret = vtd_dev_get_pe_from_pasid(vtd_as, pasid, &pe);
>> +if (ret) {
>> +/*
>> + * 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 removed.
>> + */
>> +goto remove;
>> +}
>> +
>> +/* No need to update if cached pasid entry is latest */
>that comment sounds really weird to me. In case the cache entry does not
>match the one in guest mem, you update it below - at least that's what I
>understand ;-) - However you want to return false because you don't want
>g_hash_table_foreach_remove() to remove the entry.
You understand totally right😊, will rephase to:
/* Update cached pasid entry if it's stale compared to what's in guest memory */
>> +if (!vtd_pasid_entry_compare(&pe, &pc_entry->pasid_entry)) {
>> +pc_entry->pasid_entry = pe;
>> +}
>> +return false;
>> +
>> +remove:
>> +pc_entry->valid = false;
>> +
>> +/*
>> + * Don't remove address space of PCI_NO_PASID which is created for
>PCI
>> + * sub-system.
>> + */
>> +if (vtd_as->pasid == PCI_NO_PASID) {
>> +return false;
>> +}
>> +return true;
>> +}
>> +
>> +/* Update the pasid cache in vIOMMU */
>> +static void vtd_pasid_cache_sync(IntelIOMMUState *s,
>VTDPASIDCacheInfo *pc_info)
>> +{
>> +if (!s->flts || !s->root_scalable || !s->dmar_enabled) {
>> +return;
>> +}
>> +
>> +/*
>> + * Regards to a pasid cache invalidation, e.g. a PSI.
>Regarded PASID cache invalidation?
>> + * It could be either cases of below:
>> + * a) a present pasid entry moved to non-present
>a present cache pasid entry needs to be removed
>> + * b) a present pasid entry to be a present entry
>above sounds a bit weird. A present cached pasid entry needs to be updated
>> + * c) a non-present pasid entry moved to present
>a present cached pasid entry needs to be created. As this is not handled
>in this patch I would move this to next one.
>Besides since there is another comment below I am not even sure this is
Re: [PATCH v3 10/20] intel_iommu: Handle PASID entry removing and updating
On 7/8/25 1:05 PM, 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.
>
> 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 modifies a PASID entry, QEMU will capture the guest pasid selective
> pasid cache invalidation, allocate or remove a VTDAddressSpace instance per
> the
> invalidation reasons:
>
> a) a present pasid entry moved to non-present
> b) a present pasid entry to be a present entry
> c) a non-present pasid entry moved to present
>
> This patch handles a) and b), following patch will handle c).
>
> 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
> Signed-off-by: Yi Sun
> Signed-off-by: Zhenzhong Duan
> ---
> hw/i386/intel_iommu.c | 194 +++--
> hw/i386/intel_iommu_internal.h | 27 -
> hw/i386/trace-events | 3 +
> include/hw/i386/intel_iommu.h | 6 +
> 4 files changed, 218 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 38e7f7b7be..5bda439de6 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);
> @@ -3103,6 +3103,181 @@ 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 used to update or clear cached pasid entry in vtd_as.
> + * vtd_as is released when corresponding cached pasid entry is cleared,
> + * except for PCI_NO_PASID which vtd_as is owen by PCI sub-system.
I would document when it is supposed to return true (indicates that the
cached pasid entry needs to be removed).
> + */
> +static gboolean vtd_flush_pasid_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;
> +uint32_t pasid;
> +int ret;
> +
> +if (!pc_entry->valid) {
> +return false;
> +}
> +did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
> +
> +if (vtd_as_to_iommu_pasid_locked(vtd_as, &pasid)) {
> +goto remove;
> +}
> +
> +switch (pc_info->type) {
> +case VTD_PASID_CACHE_PASIDSI:
> +if (pc_info->pasid != pasid) {
> +return false;
> +}
> +/* fall through */
> +case VTD_PASID_CACHE_DOMSI:
> +if (pc_info->did != did) {
> +return false;
> +}
> +/* fall through */
> +case VTD_PASID_CACHE_GLOBAL_INV:
> +break;
> +default:
> +error_setg(&error_fatal, "invalid pc_info->type for flush");
> +}
> +
> +/*
> + * pasid cache invalidation may indicate a present pasid
> + * entry to present pasid entry modification. To cover such
> + * case, vIOMMU emulator needs to fetch latest guest pasid
> + * entry and compares with cached pasid entry, then update
> + * pasid cache.
> + */
> +ret = vtd_dev_get_pe_from_pasid(vtd_as, pasid, &pe);
> +if (ret) {
> +/*
> + * 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 removed.
> + */
> +goto remove;
> +}
> +
> +/* No need to update if cached pasid entry is latest */
that comment sounds really weird to me. In case the cache entry does not
match the one in guest mem, you update it below - at least that's what I
understand ;-) - However you want to retur
[PATCH v3 10/20] intel_iommu: Handle PASID entry removing and updating
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.
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 modifies a PASID entry, QEMU will capture the guest pasid selective
pasid cache invalidation, allocate or remove a VTDAddressSpace instance per the
invalidation reasons:
a) a present pasid entry moved to non-present
b) a present pasid entry to be a present entry
c) a non-present pasid entry moved to present
This patch handles a) and b), following patch will handle c).
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
Signed-off-by: Yi Sun
Signed-off-by: Zhenzhong Duan
---
hw/i386/intel_iommu.c | 194 +++--
hw/i386/intel_iommu_internal.h | 27 -
hw/i386/trace-events | 3 +
include/hw/i386/intel_iommu.h | 6 +
4 files changed, 218 insertions(+), 12 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 38e7f7b7be..5bda439de6 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);
@@ -3103,6 +3103,181 @@ 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 used to update or clear cached pasid entry in vtd_as.
+ * vtd_as is released when corresponding cached pasid entry is cleared,
+ * except for PCI_NO_PASID which vtd_as is owen by PCI sub-system.
+ */
+static gboolean vtd_flush_pasid_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;
+uint32_t pasid;
+int ret;
+
+if (!pc_entry->valid) {
+return false;
+}
+did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
+
+if (vtd_as_to_iommu_pasid_locked(vtd_as, &pasid)) {
+goto remove;
+}
+
+switch (pc_info->type) {
+case VTD_PASID_CACHE_PASIDSI:
+if (pc_info->pasid != pasid) {
+return false;
+}
+/* fall through */
+case VTD_PASID_CACHE_DOMSI:
+if (pc_info->did != did) {
+return false;
+}
+/* fall through */
+case VTD_PASID_CACHE_GLOBAL_INV:
+break;
+default:
+error_setg(&error_fatal, "invalid pc_info->type for flush");
+}
+
+/*
+ * pasid cache invalidation may indicate a present pasid
+ * entry to present pasid entry modification. To cover such
+ * case, vIOMMU emulator needs to fetch latest guest pasid
+ * entry and compares with cached pasid entry, then update
+ * pasid cache.
+ */
+ret = vtd_dev_get_pe_from_pasid(vtd_as, pasid, &pe);
+if (ret) {
+/*
+ * 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 removed.
+ */
+goto remove;
+}
+
+/* No need to update if cached pasid entry is latest */
+if (!vtd_pasid_entry_compare(&pe, &pc_entry->pasid_entry)) {
+pc_entry->pasid_entry = pe;
+}
+return false;
+
+remove:
+pc_entry->valid = false;
+
+/*
+ * Don't remove address space of PCI_NO_PASID which is created for PCI
+ * sub-system.
+ */
+if (vtd_as->pasid == PCI_NO_PASID) {
+return false;
+}
+return true;
+}
+
+/* Update the pasid cache in vIOMMU */
+static void vtd_pasid_cache_sync(IntelIOMMUState *s, VTDPASIDCacheInfo
*pc_info)
+{
+if (!s->flts || !s->root_scalable || !s->dmar_enabled) {
+return;
+}
+
+/*
+ * Regards t
