RE: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-29 Thread Duan, Zhenzhong


>-Original Message-
>From: Clément MATHIEU--DRIF 
>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 +, 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 {
>> -    /*
>> -   

Re: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-29 Thread Clément MATHIEU--DRIF
   .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]](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;
> > ```
> &g

RE: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-26 Thread Duan, Zhenzhong
   };
+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 
>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 +, 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 :/

Re: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-26 Thread Clément MATHIEU--DRIF
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 +, 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  0xffe0ULL
> > > >  #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;
> > > > +

RE: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-13 Thread Duan, Zhenzhong
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 
>>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])>
>>> 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  | 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  0xffe0ULL
>>>  #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.
>

RE: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-12 Thread Duan, Zhenzhong
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 
>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])>
>> 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  | 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  0xffe0ULL
>>  #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.
>>   */
>>  i

Re: [PATCH v3 07/14] intel_iommu: Use IOMMU_NO_PASID and delete PASID_0

2026-04-12 Thread Clément MATHIEU--DRIF
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])>  
> 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  | 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  0xffe0ULL  
>  #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  
> +++