qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Yi Liu <yi.l.liu@intel.com>
To: Zhenzhong Duan <zhenzhong.duan@intel.com>, <qemu-devel@nongnu.org>
Cc: <alex.williamson@redhat.com>, <clg@redhat.com>,
	<eric.auger@redhat.com>, <mst@redhat.com>, <jasowang@redhat.com>,
	<peterx@redhat.com>, <ddutile@redhat.com>, <jgg@nvidia.com>,
	<nicolinc@nvidia.com>, <skolothumtho@nvidia.com>,
	<joao.m.martins@oracle.com>, <clement.mathieu--drif@eviden.com>,
	<kevin.tian@intel.com>, <chao.p.peng@intel.com>
Subject: Re: [PATCH v6 12/22] intel_iommu: Handle PASID cache invalidation
Date: Sun, 12 Oct 2025 22:58:18 +0800	[thread overview]
Message-ID: <42881757-3e8d-437e-80e6-aa2d409523f6@intel.com> (raw)
In-Reply-To: <20250918085803.796942-13-zhenzhong.duan@intel.com>

On 2025/9/18 16:57, Zhenzhong Duan wrote:
> This adds PASID cache sync for RID_PASID, non-RID_PASID isn't supported.
> 
> 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 <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu_internal.h |  19 +++-
>   include/hw/i386/intel_iommu.h  |   6 ++
>   hw/i386/intel_iommu.c          | 157 ++++++++++++++++++++++++++++++---
>   hw/i386/trace-events           |   3 +
>   4 files changed, 173 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 9cdc8d5dbb..d400bcee21 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;
> @@ -552,6 +562,13 @@ 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 struct VTDPASIDCacheInfo {
> +    uint8_t type;
> +    uint16_t did;
> +    uint32_t pasid;
> +    bool reset;
> +} VTDPASIDCacheInfo;
> +
>   /* PASID Table Related Definitions */
>   #define VTD_PASID_DIR_BASE_ADDR_MASK  (~0xfffULL)
>   #define VTD_PASID_TABLE_BASE_ADDR_MASK (~0xfffULL)
> @@ -573,7 +590,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_FSPM          3ULL
>   #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 3351892da0..ff01e5c82d 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 d37d47115a..24061f6dc6 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1614,7 +1614,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);
> @@ -3074,6 +3074,144 @@ 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_pasid_entry(s, &ce, pe, vtd_as->pasid);
> +}
> +
> +/*
> + * For each IOMMUFD backed device, update or invalidate pasid cache based on
> + * the value in memory.
> + */
> +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;
> +
> +    /* Ignore emulated device or legacy VFIO backed device */
> +    if (!vtd_find_hiod_iommufd(vtd_as)) {
> +        return;
> +    }
> +
> +    /* non-RID_PASID isn't supported yet */
> +    assert(vtd_as->pasid == PCI_NO_PASID);
> +
> +    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.

I think comparing DID applies to the case in which pc_entry->valid is
true. If pc_entry->valid is false, this means no cached pc_entry yet. If
pe in guest memory is valid, the pc_entry should be updated/set hence
the bind_pasid operation (added in later patch) would be conducted.

> +     */
> +    switch (pc_info->type) {
> +    case VTD_INV_DESC_PASIDC_G_PASID_SI:
> +    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;
> +        }
> +        return;
> +    }
> +
> +    pc_entry->pasid_entry = pe;
> +    pc_entry->valid = true;
> +}
> +
> +static void vtd_pasid_cache_sync(IntelIOMMUState *s, VTDPASIDCacheInfo *pc_info)
> +{
> +    if (!s->fsts || !s->root_scalable || !s->dmar_enabled) {
> +        return;
> +    }
> +
> +    vtd_iommu_lock(s);
> +    g_hash_table_foreach(s->vtd_address_spaces, vtd_pasid_cache_sync_locked,
> +                         pc_info);
> +    vtd_iommu_unlock(s);
> +}
> +
> +static bool vtd_process_pasid_desc(IntelIOMMUState *s,
> +                                   VTDInvDesc *inv_desc)
> +{
> +    uint16_t did;
> +    uint32_t pasid;
> +    VTDPASIDCacheInfo pc_info = {};
> +    uint64_t mask[4] = {VTD_INV_DESC_PASIDC_RSVD_VAL0, VTD_INV_DESC_ALL_ONE,
> +                        VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
> +
> +    if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, true,
> +                                     __func__, "pasid cache inv")) {
> +        return false;
> +    }
> +
> +    did = VTD_INV_DESC_PASIDC_DID(inv_desc);
> +    pasid = VTD_INV_DESC_PASIDC_PASID(inv_desc);
> +    pc_info.type = VTD_INV_DESC_PASIDC_G(inv_desc);
> +
> +    switch (pc_info.type) {
> +    case VTD_INV_DESC_PASIDC_G_DSI:
> +        trace_vtd_inv_desc_pasid_cache_dsi(did);
> +        pc_info.did = did;
> +        break;
> +
> +    case VTD_INV_DESC_PASIDC_G_PASID_SI:
> +        /* PASID selective implies a DID selective */
> +        trace_vtd_inv_desc_pasid_cache_psi(did, pasid);
> +        /* Currently non-RID_PASID invalidation requests are ignored */

I'm a bit doubting if this is safe given the ATS path (for emulated
device) is merged. ATS path supports non-RID_PASID if emulated device
has PASID cap. The lucky thing is that the ATS path does not have
pasid level cache. So skipping invalidation for non-RID_PASID is not
harmful so far. Just a note to other reviewers although I didn't see a
problem here.

> +        if (pasid != RID_PASID) {
> +            return true;
> +        }
> +        pc_info.did = did;
> +        pc_info.pasid = pasid;
> +        break;
> +

Regards,
Yi Liu


  reply	other threads:[~2025-10-12 14:52 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18  8:57 [PATCH v6 00/22] intel_iommu: Enable first stage translation for passthrough device Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 01/22] intel_iommu: Rename vtd_ce_get_rid2pasid_entry to vtd_ce_get_pasid_entry Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 02/22] intel_iommu: Delete RPS capability related supporting code Zhenzhong Duan
2025-09-30 13:49   ` Eric Auger
2025-10-09 10:10     ` Duan, Zhenzhong
2025-10-12 12:30       ` Yi Liu
2025-09-18  8:57 ` [PATCH v6 03/22] intel_iommu: Update terminology to match VTD spec Zhenzhong Duan
2025-09-30  7:45   ` Eric Auger
2025-10-12 12:30   ` Yi Liu
2025-10-13  6:20     ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 04/22] hw/pci: Export pci_device_get_iommu_bus_devfn() and return bool Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 05/22] hw/pci: Introduce pci_device_get_viommu_flags() Zhenzhong Duan
2025-09-23 18:47   ` Nicolin Chen
2025-09-24  7:05     ` Duan, Zhenzhong
2025-09-24  8:21       ` Nicolin Chen
2025-09-26  2:54         ` Duan, Zhenzhong
2025-09-30 13:55           ` Eric Auger
2025-10-12 12:26   ` Yi Liu
2025-10-13  6:24     ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 06/22] intel_iommu: Implement get_viommu_flags() callback Zhenzhong Duan
2025-10-12 12:28   ` Yi Liu
2025-10-13  6:26     ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 07/22] intel_iommu: Introduce a new structure VTDHostIOMMUDevice Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 08/22] vfio/iommufd: Force creating nesting parent HWPT Zhenzhong Duan
2025-09-30 14:19   ` Eric Auger
2025-10-12 12:33   ` Yi Liu
2025-09-18  8:57 ` [PATCH v6 09/22] intel_iommu: Stick to system MR for IOMMUFD backed host device when x-fls=on Zhenzhong Duan
2025-09-30 15:04   ` Eric Auger
2025-10-09 10:10     ` Duan, Zhenzhong
2025-10-12 12:51   ` Yi Liu
2025-09-18  8:57 ` [PATCH v6 10/22] intel_iommu: Check for compatibility with IOMMUFD backed device when x-flts=on Zhenzhong Duan
2025-10-12 12:55   ` Yi Liu
2025-10-13  6:48     ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 11/22] intel_iommu: Fail passthrough device under PCI bridge if x-flts=on Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 12/22] intel_iommu: Handle PASID cache invalidation Zhenzhong Duan
2025-10-12 14:58   ` Yi Liu [this message]
2025-10-13  7:37     ` Duan, Zhenzhong
2025-10-13 12:53       ` Yi Liu
2025-09-18  8:57 ` [PATCH v6 13/22] intel_iommu: Reset pasid cache when system level reset Zhenzhong Duan
2025-10-13 10:25   ` Yi Liu
2025-10-14  5:56     ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 14/22] intel_iommu: Add some macros and inline functions Zhenzhong Duan
2025-10-13 10:25   ` Yi Liu
2025-09-18  8:57 ` [PATCH v6 15/22] intel_iommu: Bind/unbind guest page table to host Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 16/22] intel_iommu: Propagate PASID-based iotlb invalidation " Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 17/22] intel_iommu: Replay all pasid bindings when either SRTP or TE bit is changed Zhenzhong Duan
2025-09-18  8:57 ` [PATCH v6 18/22] iommufd: Introduce a helper function to extract vendor capabilities Zhenzhong Duan
2025-09-23 19:45   ` Nicolin Chen
2025-09-24  8:05     ` Duan, Zhenzhong
2025-09-24  8:27       ` Nicolin Chen
2025-09-26  2:54         ` Duan, Zhenzhong
2025-09-18  8:57 ` [PATCH v6 19/22] vfio: Add a new element bypass_ro in VFIOContainerBase Zhenzhong Duan
2025-09-26 12:25   ` Cédric Le Goater
2025-09-18  8:57 ` [PATCH v6 20/22] Workaround for ERRATA_772415_SPR17 Zhenzhong Duan
2025-09-18  8:58 ` [PATCH v6 21/22] intel_iommu: Enable host device when x-flts=on in scalable mode Zhenzhong Duan
2025-09-18  8:58 ` [PATCH v6 22/22] docs/devel: Add IOMMUFD nesting documentation Zhenzhong Duan
2025-09-18 10:00   ` Cédric Le Goater
2025-09-19  2:17     ` Duan, Zhenzhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42881757-3e8d-437e-80e6-aa2d409523f6@intel.com \
    --to=yi.l.liu@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=chao.p.peng@intel.com \
    --cc=clement.mathieu--drif@eviden.com \
    --cc=clg@redhat.com \
    --cc=ddutile@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jgg@nvidia.com \
    --cc=joao.m.martins@oracle.com \
    --cc=kevin.tian@intel.com \
    --cc=mst@redhat.com \
    --cc=nicolinc@nvidia.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=skolothumtho@nvidia.com \
    --cc=zhenzhong.duan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).