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
> > > <[[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:[[email protected]](mailto:[email protected]))>
> > >
> > > > Tested-by: Xudong Hao
> > >
> > > <[[[email protected]](mailto:[email protected])](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 */
> > >
> >
>
