Hi Clement,
I see your pains. How about below cleanup?
It introduces a converting function vtd_pasid_to_pci_pasid(),
then the check always is on PCI pasid with vtd_as->pasid
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -98,6 +98,14 @@ static void vtd_pasid_cache_reset_locked(IntelIOMMUState *s)
}
}
+static uint32_t vtd_pasid_to_pci_pasid(uint32_t pasid)
+{
+ if (pasid == PASID_0) {
+ pasid = PCI_NO_PASID;
+ }
+ return pasid;
+}
+
static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
uint64_t wmask, uint64_t w1cmask)
{
@@ -2506,9 +2514,10 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState
*s, uint16_t domain_id)
}
/*
- * There is no pasid field in iotlb invalidation descriptor, so PCI_NO_PASID
+ * There is no pasid field in iotlb invalidation descriptor, so PASID_0
* is passed as parameter. Piotlb invalidation supports pasid, pasid in its
- * descriptor is passed which should not be PCI_NO_PASID.
+ * descriptor is passed. In both cases, pasid is converted to PCI pasid
+ * before checking with vtd_as->pasid.
*/
static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
uint16_t domain_id, hwaddr addr,
@@ -2519,51 +2528,45 @@ static void
vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
int ret;
hwaddr size = (1 << am) * VTD_PAGE_SIZE;
+ pasid = vtd_pasid_to_pci_pasid(pasid);
+
QLIST_FOREACH(vtd_as, &(s->vtd_as_with_notifiers), next) {
ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
vtd_as->devfn, &ce);
- if (!ret && domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
+ if (ret || vtd_as->pasid != pasid ||
+ domain_id != vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
+ continue;
+ }
+
+ if (vtd_as_has_map_notifier(vtd_as)) {
/*
- * In legacy mode, vtd_as->pasid == pasid is always true.
- * In scalable mode, for vtd address space backing a PCI
- * device without pasid, needs to compare pasid with
- * PASID_0 of this device.
+ * When first stage translation is off, as long as we have MAP
+ * notifications registered in any of our IOMMU notifiers,
+ * we need to sync the shadow page table. Otherwise VFIO
+ * device attaches to nested page table instead of shadow
+ * page table, so no need to sync.
*/
- if (!(vtd_as->pasid == pasid ||
- (vtd_as->pasid == PCI_NO_PASID && pasid == PASID_0))) {
- continue;
- }
-
- if (vtd_as_has_map_notifier(vtd_as)) {
- /*
- * When first stage translation is off, as long as we have MAP
- * notifications registered in any of our IOMMU notifiers,
- * we need to sync the shadow page table. Otherwise VFIO
- * device attaches to nested page table instead of shadow
- * page table, so no need to sync.
- */
- if (!s->fsts || !s->root_scalable) {
- vtd_sync_shadow_page_table_range(vtd_as, &ce, addr, size);
- }
- } else {
- /*
- * For UNMAP-only notifiers, we don't need to walk the
- * page tables. We just deliver the PSI down to
- * invalidate caches.
- */
- const IOMMUTLBEvent event = {
- .type = IOMMU_NOTIFIER_UNMAP,
- .entry = {
- .target_as = &address_space_memory,
- .iova = addr,
- .translated_addr = 0,
- .addr_mask = size - 1,
- .perm = IOMMU_NONE,
- .pasid = vtd_as->pasid,
- },
- };
- memory_region_notify_iommu(&vtd_as->iommu, 0, event);
+ if (!s->fsts || !s->root_scalable) {
+ vtd_sync_shadow_page_table_range(vtd_as, &ce, addr, size);
}
+ } else {
+ /*
+ * For UNMAP-only notifiers, we don't need to walk the
+ * page tables. We just deliver the PSI down to
+ * invalidate caches.
+ */
+ const IOMMUTLBEvent event = {
+ .type = IOMMU_NOTIFIER_UNMAP,
+ .entry = {
+ .target_as = &address_space_memory,
+ .iova = addr,
+ .translated_addr = 0,
+ .addr_mask = size - 1,
+ .perm = IOMMU_NONE,
+ .pasid = vtd_as->pasid,
+ },
+ };
+ memory_region_notify_iommu(&vtd_as->iommu, 0, event);
}
}
}
@@ -2582,7 +2585,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, PCI_NO_PASID);
+ vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am, PASID_0);
}
/* Flush IOTLB
@@ -3020,12 +3023,17 @@ static gboolean vtd_hash_remove_by_pasid(gpointer key,
gpointer value,
(entry->pasid == info->pasid));
}
+/*
+ * Piotlb invalidation supports pasid, pasid in its descriptor is passed and
+ * is converted to PCI pasid before checking with vtd_as->pasid.
+ */
static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
uint16_t domain_id, uint32_t pasid)
{
VTDIOTLBPageInvInfo info;
VTDAddressSpace *vtd_as;
VTDContextEntry ce;
+ int ret;
info.domain_id = domain_id;
info.pasid = pasid;
@@ -3037,18 +3045,18 @@ static void vtd_piotlb_pasid_invalidate(IntelIOMMUState
*s,
false);
vtd_iommu_unlock(s);
+ pasid = vtd_pasid_to_pci_pasid(pasid);
+
QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
- if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
- vtd_as->devfn, &ce) &&
- domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
- if ((vtd_as->pasid != PCI_NO_PASID || pasid != PASID_0) &&
- vtd_as->pasid != pasid) {
- continue;
- }
+ ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
+ vtd_as->devfn, &ce);
+ if (ret || vtd_as->pasid != pasid ||
+ domain_id != vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
+ continue;
+ }
- if (!s->fsts || !vtd_as_has_map_notifier(vtd_as)) {
- vtd_address_space_sync(vtd_as);
- }
+ if (!s->fsts || !vtd_as_has_map_notifier(vtd_as)) {
+ vtd_address_space_sync(vtd_as);
}
}
}
Thanks
Zhenzhong
>-----Original Message-----
>From: Clément MATHIEU--DRIF <[email protected]>
>Subject: Re: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete
>PASID_0
>
>Hi Zhenzhong, sorry for the delay, a bit busy these days o.O
>
>Thanks for sharing the rationale.
>
>I see the purpose but I'm still a bit reluctant to the idea of mixing worlds
>too much.
>I think might be error prone and/or difficult to understand for newcomers.
>
>For instance, the following statement has a lot of implicit complexity:
>
>```c
> if (!(vtd_as->pasid == pasid ||
> (vtd_as->pasid == PCI_NO_PASID && pasid == IOMMU_NO_PASID)))
> {
> continue;
>```
>
>Couldn't we find a design that makes the code a bit more homogeneous?
>
>Thanks
>
>
>On Mon, 2026-04-13 at 07:44 +0000, Duan, Zhenzhong wrote:
>> Oh, sorry, you asked PCI_NO_PASID and I read it as IOMMU_NO_PASID.
>>
>> My understanding is:
>> PCI_NO_PASID(-1) is a definition in PCI sub-system, IOMMU_NO_PASID is used
>in
>> vIOMMU. We should not cross using them in different sub-systems.
>> So need to translate PCI_NO_PASID to IOMMU_NO_PASID in cross sub-system
>> communication.
>>
>> Thanks
>> Zhenzhong
>>
>>
>> > -----Original Message-----
>> > From: Duan, Zhenzhong
>> > Subject: RE: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and
>delete
>> > PASID_0
>> >
>> > Hi Clement,
>> >
>> > In kernel we defined it as:
>> >
>> > #define IOMMU_NO_PASID (0U) /* Reserved for DMA w/o PASID */
>> >
>> > In intel_pasid_get_entry(), IOMMU_NO_PASID is used to index the pasid table
>> > directly.
>> >
>> > In qemu, PASID_0 is used for DMA w/o PASID, so we think it's duplicated
>> > with
>> > IOMMU_NO_PASID.
>> >
>> > Thanks
>> > Zhenzhong
>> >
>> >
>> > > -----Original Message-----
>> > > From: Clément MATHIEU--DRIF <[clement.mathieu--
>[email protected]](mailto:[email protected])>
>> > > Subject: Re: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and
>delete
>> > > PASID_0
>> > >
>> > > Hi Zhenzhong,
>> > >
>> > > Could you just clarify how this interracts with PCI_NO_PASID?
>> > >
>> > > No sure I understand properly :/
>> > >
>> > > On Thu, 2026-04-02 at 23:55 -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])](mailto:[[email protected]](mailto:
>[email protected]))>
>> > > > Signed-off-by: Zhenzhong Duan
>> > >
>> > >
><[[[email protected]](mailto:[email protected])](mailto:[zhen
>[email protected]](mailto:[email protected]))>
>> > >
>> > > > Tested-by: Xudong Hao
>> > >
>> > >
><[[[email protected]](mailto:[email protected])](mailto:[xudong.hao@i
>ntel.com](mailto:[email protected]))>
>> > >
>> > > > ---
>> > > > hw/i386/intel_iommu_internal.h | 1 -
>> > > > hw/i386/intel_iommu.c | 18 +++++++++---------
>> > > > hw/i386/intel_iommu_accel.c | 2 +-
>> > > > 3 files changed, 10 insertions(+), 11 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 aa67c89e73..3b8d3a96d2 100644
>> > > > --- a/hw/i386/intel_iommu.c
>> > > > +++ b/hw/i386/intel_iommu.c
>> > > > @@ -941,7 +941,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);
>> > > > @@ -958,7 +958,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);
>> > > >
>> > > > @@ -1501,9 +1501,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) */
>> > > > @@ -1564,7 +1564,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.
>> > > > */
>> > > > @@ -2122,7 +2122,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 */
>> > > > @@ -2508,10 +2508,10 @@ static void
>> > >
>> > > vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
>> > >
>> > > > * In legacy mode, vtd_as->pasid == pasid is always true.
>> > > > * In scalable mode, for vtd address space backing a PCI
>> > > > * device without pasid, needs to compare pasid with
>> > > > - * PASID_0 of this device.
>> > > > + * IOMMU_NO_PASID of this device.
>> > > > */
>> > > > if (!(vtd_as->pasid == pasid ||
>> > > > - (vtd_as->pasid == PCI_NO_PASID && pasid ==
>> > > > PASID_0))) {
>> > > > + (vtd_as->pasid == PCI_NO_PASID && pasid ==
>> > >
>> >
>> > IOMMU_NO_PASID)))
>> >
>> > > {
>> > >
>> > > > continue;
>> > > > }
>> > > >
>> > > > @@ -3022,7 +3022,7 @@ static void
>> > >
>> > > vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
>> > >
>> > > > if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
>> > > > vtd_as->devfn, &ce) &&
>> > > > domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
>> > > > - if ((vtd_as->pasid != PCI_NO_PASID || pasid != PASID_0) &&
>> > > > + if ((vtd_as->pasid != PCI_NO_PASID || pasid !=
>IOMMU_NO_PASID) &&
>> > > > vtd_as->pasid != pasid) {
>> > > > continue;
>> > > > }
>> > > > 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
>*/
>> > >
>> >
>>
