>-----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
>
>Oh, yep, thanks for the effort.
>Do you mind if we go this route for the next iteration?
Sure, I'll add it as a prerequisite patch in next respin, thanks for suggesting.
>
>Maybe we could add a comment on vtd_pasid_to_pci_pasid to explain why and
>when
>this translation is required so that future developers can find the information
>without digging into the mailing list or git history.
Yes, will add.
BRs,
Zhenzhong
>
>On Mon, 2026-04-27 at 06:56 +0000, Duan, Zhenzhong wrote:
>> 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 <[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, 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])](mailto:[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]](mailt
>o:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailt
>o:
>> > [[email protected]](mailto:[email protected])))>
>> >
>> > >
>> > > >
>> > > > >
>> > > > > > Signed-off-by: Zhenzhong Duan
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>> >
><[[[[email protected]](mailto:[email protected])](mailto:[zhen
>[email protected]](mailto:[email protected]))](mailto:[zhen
>> >
>[[email protected]](mailto:[email protected])](mailto:[zhenzhong.duan
>@intel.com](mailto:[email protected])))>
>> >
>> > >
>> > > >
>> > > > >
>> > > > >
>> > > > > > Tested-by: Xudong Hao
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>> >
><[[[[email protected]](mailto:[email protected])](mailto:[xudong.hao@
>intel.com](mailto:[email protected]))](mailto:[xudong.hao@i
>> > ntel.com](mailto:[[email protected]](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
>> > > > >
>> > > >
>> > >
>> >
>> > */
>> >
>> > >
>> > > >
>> > > > >
>> > > >
>> > > >
>> > >
>> > >
>> >
>>
