Hi Clement, Good finding, it's not good that PCI_NO_PASID(-1) is set as VTD_FRCD_PV when pasid == PCI_NO_PASID.
It looks error prone and confusing as we still have both PCI pasid and iommu pasid in intel_iommu code, I am trying to set vtd_as->pasid to iommu pasid to avoid those check and conversion. See link https://github.com/yiliu1765/qemu/commit/9f71129bd215cdb5c416e3c7abb7c282fdb7c006 Thanks Zhenzhong >-----Original Message----- >From: Clément MATHIEU--DRIF <[email protected]> >Subject: Re: [PATCH v4 08/15] intel_iommu: Use IOMMU_NO_PASID and delete >PASID_0 > >Hi Zhenzhong, > >Have you checked these kind of calls? > >``` > if (pasid == PCI_NO_PASID && s->root_scalable) { > pasid = IOMMU_NO_PASID; > } > > /* ....... */ > > 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); > } > goto error; > } >``` > >cmd > >On Thu, 2026-04-30 at 03:13 -0400, Zhenzhong Duan wrote: >> In previous patch we introduced a global macro IOMMU_NO_PASID(0) for >> Requests-without-PASID, this makes the local macro PASID_0 redundant. >> Delete it and use IOMMU_NO_PASID instead. >> >> Suggested-by: Yi Liu <[[email protected]](mailto:[email protected])> >> Signed-off-by: Zhenzhong Duan ><[[email protected]](mailto:[email protected])> >> Tested-by: Xudong Hao ><[[email protected]](mailto:[email protected])> >> --- >> hw/i386/intel_iommu_internal.h | 1 - >> hw/i386/intel_iommu.c | 21 +++++++++++---------- >> hw/i386/intel_iommu_accel.c | 2 +- >> 3 files changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h >> index c7e107fe87..0141316f83 100644 >> --- a/hw/i386/intel_iommu_internal.h >> +++ b/hw/i386/intel_iommu_internal.h >> @@ -615,7 +615,6 @@ typedef struct VTDRootEntry VTDRootEntry; >> #define VTD_CTX_ENTRY_LEGACY_SIZE 16 >> #define VTD_CTX_ENTRY_SCALABLE_SIZE 32 >> >> -#define PASID_0 0 >> #define VTD_SM_CONTEXT_ENTRY_RSVD_VAL0(aw) (0x1e0ULL | >~VTD_HAW_MASK(aw)) >> #define VTD_SM_CONTEXT_ENTRY_RSVD_VAL1 0xffffffffffe00000ULL >> #define VTD_SM_CONTEXT_ENTRY_PRE 0x10ULL >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 74642d8123..4d1b262d56 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -100,7 +100,8 @@ static void >vtd_pasid_cache_reset_locked(IntelIOMMUState *s) >> * >> * Requests-without-PASID: >> * - PCI subsystem: Uses PCI_NO_PASID (-1) to indicate no PASID present >> - * - VT-d IOMMU: Uses PASID_0 (0) to index the PASID table for >> translation >> + * - VT-d IOMMU: Uses IOMMU_NO_PASID (0) to index the PASID table for >> + * translation >> * >> * Requests-with-PASID: >> * - Both subsystems use identical PASID values (1-0xFFFFF) >> @@ -114,7 +115,7 @@ static void >vtd_pasid_cache_reset_locked(IntelIOMMUState *s) >> */ >> static uint32_t vtd_pasid_to_pci_pasid(uint32_t pasid) >> { >> - if (pasid == PASID_0) { >> + if (pasid == IOMMU_NO_PASID) { >> pasid = PCI_NO_PASID; >> } >> return pasid; >> @@ -969,7 +970,7 @@ int vtd_ce_get_pasid_entry(IntelIOMMUState *s, >VTDContextEntry *ce, >> dma_addr_t pasid_dir_base; >> >> if (pasid == PCI_NO_PASID) { >> - pasid = PASID_0; >> + 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); >> @@ -986,7 +987,7 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s, >> VTDPASIDEntry pe; >> >> if (pasid == PCI_NO_PASID) { >> - pasid = PASID_0; >> + pasid = IOMMU_NO_PASID; >> } >> pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce); >> >> @@ -1529,9 +1530,9 @@ static int vtd_ce_pasid_0_check(IntelIOMMUState *s, >VTDContextEntry *ce) >> >> /* >> * Make sure in Scalable Mode, a present context entry >> - * has valid pasid entry setting at PASID_0. >> + * has valid pasid entry setting at IOMMU_NO_PASID. >> */ >> - return vtd_ce_get_pasid_entry(s, ce, &pe, PASID_0); >> + return vtd_ce_get_pasid_entry(s, ce, &pe, IOMMU_NO_PASID); >> } >> >> /* Map a device to its corresponding domain (context-entry) */ >> @@ -1592,7 +1593,7 @@ int vtd_dev_to_context_entry(IntelIOMMUState *s, >uint8_t bus_num, >> } >> } else { >> /* >> - * Check if the programming of pasid setting of PASID_0 >> + * Check if the programming of pasid setting of IOMMU_NO_PASID >> * is valid, and thus avoids to check pasid entry fetching >> * result in future helper function calling. >> */ >> @@ -2150,7 +2151,7 @@ static bool >vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus, >> vtd_iommu_lock(s); >> >> if (pasid == PCI_NO_PASID && s->root_scalable) { >> - pasid = PASID_0; >> + pasid = IOMMU_NO_PASID; >> } >> >> /* Try to fetch pte from IOTLB */ >> @@ -2515,7 +2516,7 @@ static void >vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id) >> } >> >> /* >> - * There is no pasid field in iotlb invalidation descriptor, so PASID_0 >> + * There is no pasid field in iotlb invalidation descriptor, so >> IOMMU_NO_PASID >> * is passed as parameter. Piotlb invalidation supports pasid, pasid in its >> * descriptor is passed. In both cases, pasid is converted to PCI pasid >> * before checking with vtd_as->pasid. >> @@ -2586,7 +2587,7 @@ static void >vtd_iotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id, >> vtd_iommu_lock(s); >> g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page, &info); >> vtd_iommu_unlock(s); >> - vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am, PASID_0); >> + vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am, >IOMMU_NO_PASID); >> } >> >> /* Flush IOTLB >> diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c >> index bd1236c070..8940d240a1 100644 >> --- a/hw/i386/intel_iommu_accel.c >> +++ b/hw/i386/intel_iommu_accel.c >> @@ -217,7 +217,7 @@ static void vtd_flush_host_piotlb_locked(gpointer key, >gpointer value, >> >> did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry); >> >> - if (piotlb_info->domain_id == did && piotlb_info->pasid == PASID_0) { >> + if (piotlb_info->domain_id == did && piotlb_info->pasid == >IOMMU_NO_PASID) { >> HostIOMMUDeviceIOMMUFD *hiodi = >> HOST_IOMMU_DEVICE_IOMMUFD(vtd_hiod->hiod); >> uint32_t entry_num = 1; /* Only implement one request for >> simplicity */
