Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

[Yi Liu]

On 5/19/26 15:20, Duan, Zhenzhong wrote:

-----Original Message-----
From: Liu, Yi L <yi.l....@intel.com>
Subject: Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use
IOMMU_NO_PASID internally

On 5/9/26 12:08, Zhenzhong Duan wrote:

The PCI subsystem uses PCI_NO_PASID for requests-without-PASID, but VT-d
uses IOMMU_NO_PASID internally. This leads to conversion and checking code

s/VT-d uses IOMMU_NO_PASID internally/VT-d emulation uses
IOMMU_NO_PASID
internally (ecap.RPS==0)/

Will do.

between PCI_NO_PASID and IOMMU_NO_PASID throughout the

implementation.

Refactor to use IOMMU PASID consistently within Intel IOMMU by storing
IOMMU PASID value in vtd_as->pasid. After this change, PCI_NO_PASID is
only used at three boundary points:

a typo or the third boundary is missed?

Section 2 contains two points:

1. Convert when notifying UNMAP events via memory_region_notify_iommu()
2. Convert when returning IOMMUTLBEntry in vtd_iommu_translate()

got it.

1. PCI_NO_PASID -> IOMMU_NO_PASID: Convert PCI PASID to IOMMU PASID in
     vtd_find_add_as() and cache in vtd_as->pasid.
2. IOMMU_NO_PASID -> PCI_NO_PASID: Convert when notifying UNMAP events
     via memory_region_notify_iommu() and returning IOMMUTLBEntry in
     vtd_iommu_translate().

This eliminates conversion/checks in PASID table lookups, simplifies
invalidation logic with consistent PASID values, and improves code
readability. The PCI subsystem interface remains unchanged to maintain
compatibility with other IOMMU implementations that may not use PASID 0
for requests-without-PASID.

Suggested-by: Clement Mathieu--Drif <clement.mathieu--d...@bull.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
   include/system/memory.h     |   2 +-
   hw/i386/intel_iommu.c       | 164 +++++++++++++++++-------------------
   hw/i386/intel_iommu_accel.c |   2 +-
   3 files changed, 80 insertions(+), 88 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 1417132f6d..1edb38b07d 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -150,7 +150,7 @@ struct IOMMUTLBEntry {
       hwaddr           translated_addr;
       hwaddr           addr_mask;  /* 0xfff = 4k translation */
       IOMMUAccessFlags perm;
-    uint32_t         pasid;
+    uint32_t         pasid; /* PCI pasid */
   };

   /*
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5e5dcdc274..b50c556c40 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -938,12 +938,8 @@ static int

vtd_get_pe_from_pasid_table(IntelIOMMUState *s,

   int vtd_ce_get_pasid_entry(IntelIOMMUState *s, VTDContextEntry *ce,
                              VTDPASIDEntry *pe, uint32_t pasid)
   {
-    dma_addr_t pasid_dir_base;
+    dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);

-    if (pasid == PCI_NO_PASID) {
-        pasid = IOMMU_NO_PASID;
-    }
-    pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
       return vtd_get_pe_from_pasid_table(s, pasid_dir_base, pasid, pe);
   }

@@ -953,15 +949,10 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,
                                   uint32_t pasid)
   {
       int ret;
-    dma_addr_t pasid_dir_base;
+    dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
       VTDPASIDDirEntry pdire;
       VTDPASIDEntry pe;

-    if (pasid == PCI_NO_PASID) {
-        pasid = IOMMU_NO_PASID;
-    }
-    pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
-
       /*
        * No present bit check since fpd is meaningful even
        * if the present bit is clear.
@@ -1750,7 +1741,7 @@ static bool

vtd_switch_address_space(VTDAddressSpace *as)

            *
            * Need to disable ir for as with PASID.
            */
-        if (as->pasid != PCI_NO_PASID) {
+        if (as->pasid != IOMMU_NO_PASID) {
               memory_region_set_enabled(&as->iommu_ir, false);
           } else {
               memory_region_set_enabled(&as->iommu_ir, true);
@@ -1780,7 +1771,7 @@ static bool

vtd_switch_address_space(VTDAddressSpace *as)

        * We enable per as memory region (iommu_ir_fault) for catching
        * the translation for interrupt range through PASID + PT.
        */
-    if (pt && as->pasid != PCI_NO_PASID) {
+    if (pt && as->pasid != IOMMU_NO_PASID) {
           memory_region_set_enabled(&as->iommu_ir_fault, true);
       } else {
           memory_region_set_enabled(&as->iommu_ir_fault, false);
@@ -1892,7 +1883,7 @@ static VTDAddressSpace

*vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,

   VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
   {
-    return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
+    return vtd_get_as_by_sid_and_pasid(s, sid, IOMMU_NO_PASID);
   }

   static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)
@@ -2121,10 +2112,6 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,

       vtd_iommu_lock(s);

-    if (pasid == PCI_NO_PASID && s->root_scalable) {
-        pasid = IOMMU_NO_PASID;
-    }
-
       /* Try to fetch pte from IOTLB */
       iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
       if (iotlb_entry) {
@@ -2235,7 +2222,7 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,

       if (ret_fr) {
           if (!vtd_is_recoverable_fault(-ret_fr, iommu_idx)) {
               vtd_report_fault(s, -ret_fr, is_fpd_set, source_id,
-                            addr, is_write, pasid != PCI_NO_PASID, pasid);
+                            addr, is_write, s->root_scalable, pasid);

a typo here? s->root_scalable should be "pasid != IOMMU_NO_PASID"?

This is intentional.
pasid = vtd_as->pasid = iommu_pasid, so it never equals PCI_NO_PASID now.

not quite get. It can still possibly be IOMMU_NO_PASID, right? If not,
this looks to have introduced functional change, which should not be
expected by this patch.