Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-22 Thread Yi Liu 

On 5/22/26 15:17, Duan, Zhenzhong wrote:




-Original Message-
From: Liu, Yi L 
Subject: Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use
IOMMU_NO_PASID internally

On 5/20/26 19:00, Duan, Zhenzhong wrote:

@@ -2121,10 +2112,6 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,


 vtd_iommu_lock(s);

-if (pasid == PCI_NO_PASID && s->root_scalable) {
-pasid = IOMMU_NO_PASID;
-}
-
 /* Try to fetch pte from IOTLB */
 iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
 if (iotlb_entry) {
@@ -2235,7 +,7 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,

 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);
+addr, is_write, s->root_scalable, pasid);


a typo here? s->root_scalable should be "pasid != IOMMU_NO_PASID"?


This is intentional.
pasid = vtd_as->pasid = iommu_pasid, so it never equals PCI_NO_PASID now.


not quite get. It can still possibly be IOMMU_NO_PASID, right? If not,
this looks to have introduced functional change, which should not be
expected by this patch.


Before this patch, If pasid == PCI_NO_PASID && s->root_scalable is true, then we

pass is_pasid=true to vtd_report_fault().

If we change to use "pasid != IOMMU_NO_PASID" in this patch, then we pass

is_pasid=false to vtd_report_fault() for the same scenario because pasid is
IOMMU_NO_PASID now.


But using s->root_scalable could keep functions unchanged, right?


got your point. But this helper should cover both translation w/ and w/o
pasid, it should not reply on the s->root_scalable, but rely on if pasid
is included. So the logic does not look correct.

checked the original commit. It seems recording the rid_pasid in the
fault reporting wrongly. And that's why you got the current logic here.
May need a fix. Below is the original commit, and the spec definition.

- commit: 1b2b12376c8a513a0c7b5e3b8ea702038d3d7db5

- Spec says pasid is only valid when the fault request is request with
PASID.

   "
   When Set, indicates the faulted request has a PASID TLP Prefix. The
   value of the PASID field is reported in the PASID Value (PV) field.
   "


I see, you are right, will use "pasid != IOMMU_NO_PASID" in this patch,
let me know if you want a separate fix patch instead.


go with a separate fix patch is better. :)

RE: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-22 Thread Duan, Zhenzhong 


>-Original Message-
>From: Liu, Yi L 
>Subject: Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use
>IOMMU_NO_PASID internally
>
>On 5/20/26 19:00, Duan, Zhenzhong wrote:
>>>>>> @@ -2121,10 +2112,6 @@ static bool
>>>>> vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
>>>>>>
>>>>>> vtd_iommu_lock(s);
>>>>>>
>>>>>> -if (pasid == PCI_NO_PASID && s->root_scalable) {
>>>>>> -pasid = IOMMU_NO_PASID;
>>>>>> -}
>>>>>> -
>>>>>> /* Try to fetch pte from IOTLB */
>>>>>> iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
>>>>>> if (iotlb_entry) {
>>>>>> @@ -2235,7 +,7 @@ static bool
>>>>> vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
>>>>>> 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);
>>>>>> +addr, is_write, s->root_scalable, pasid);
>>>>>
>>>>> a typo here? s->root_scalable should be "pasid != IOMMU_NO_PASID"?
>>>>
>>>> This is intentional.
>>>> pasid = vtd_as->pasid = iommu_pasid, so it never equals PCI_NO_PASID now.
>>>
>>> not quite get. It can still possibly be IOMMU_NO_PASID, right? If not,
>>> this looks to have introduced functional change, which should not be
>>> expected by this patch.
>>
>> Before this patch, If pasid == PCI_NO_PASID && s->root_scalable is true, 
>> then we
>pass is_pasid=true to vtd_report_fault().
>> If we change to use "pasid != IOMMU_NO_PASID" in this patch, then we pass
>is_pasid=false to vtd_report_fault() for the same scenario because pasid is
>IOMMU_NO_PASID now.
>>
>> But using s->root_scalable could keep functions unchanged, right?
>
>got your point. But this helper should cover both translation w/ and w/o
>pasid, it should not reply on the s->root_scalable, but rely on if pasid
>is included. So the logic does not look correct.
>
>checked the original commit. It seems recording the rid_pasid in the
>fault reporting wrongly. And that's why you got the current logic here.
>May need a fix. Below is the original commit, and the spec definition.
>
>- commit: 1b2b12376c8a513a0c7b5e3b8ea702038d3d7db5
>
>- Spec says pasid is only valid when the fault request is request with
>PASID.
>
>   "
>   When Set, indicates the faulted request has a PASID TLP Prefix. The
>   value of the PASID field is reported in the PASID Value (PV) field.
>   "

I see, you are right, will use "pasid != IOMMU_NO_PASID" in this patch,
let me know if you want a separate fix patch instead.

Thanks
Zhenzhong

Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-21 Thread Yi Liu 

On 5/20/26 19:00, Duan, Zhenzhong wrote:

@@ -2121,10 +2112,6 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,


vtd_iommu_lock(s);

-if (pasid == PCI_NO_PASID && s->root_scalable) {
-pasid = IOMMU_NO_PASID;
-}
-
/* Try to fetch pte from IOTLB */
iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
if (iotlb_entry) {
@@ -2235,7 +,7 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,

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);
+addr, is_write, s->root_scalable, pasid);


a typo here? s->root_scalable should be "pasid != IOMMU_NO_PASID"?


This is intentional.
pasid = vtd_as->pasid = iommu_pasid, so it never equals PCI_NO_PASID now.


not quite get. It can still possibly be IOMMU_NO_PASID, right? If not,
this looks to have introduced functional change, which should not be
expected by this patch.


Before this patch, If pasid == PCI_NO_PASID && s->root_scalable is true, then 
we pass is_pasid=true to vtd_report_fault().
If we change to use "pasid != IOMMU_NO_PASID" in this patch, then we pass 
is_pasid=false to vtd_report_fault() for the same scenario because pasid is 
IOMMU_NO_PASID now.

But using s->root_scalable could keep functions unchanged, right?


got your point. But this helper should cover both translation w/ and w/o
pasid, it should not reply on the s->root_scalable, but rely on if pasid
is included. So the logic does not look correct.

checked the original commit. It seems recording the rid_pasid in the
fault reporting wrongly. And that's why you got the current logic here.
May need a fix. Below is the original commit, and the spec definition.

- commit: 1b2b12376c8a513a0c7b5e3b8ea702038d3d7db5

- Spec says pasid is only valid when the fault request is request with
   PASID.

  "
  When Set, indicates the faulted request has a PASID TLP Prefix. The
  value of the PASID field is reported in the PASID Value (PV) field.
  "

RE: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-20 Thread Duan, Zhenzhong 
 @@ -2121,10 +2112,6 @@ static bool
>>> vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,

vtd_iommu_lock(s);

 -if (pasid == PCI_NO_PASID && s->root_scalable) {
 -pasid = IOMMU_NO_PASID;
 -}
 -
/* Try to fetch pte from IOTLB */
iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
if (iotlb_entry) {
 @@ -2235,7 +,7 @@ static bool
>>> vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
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);
 +addr, is_write, s->root_scalable, pasid);
>>>
>>> a typo here? s->root_scalable should be "pasid != IOMMU_NO_PASID"?
>>
>> This is intentional.
>> pasid = vtd_as->pasid = iommu_pasid, so it never equals PCI_NO_PASID now.
>
>not quite get. It can still possibly be IOMMU_NO_PASID, right? If not,
>this looks to have introduced functional change, which should not be
>expected by this patch.

Before this patch, If pasid == PCI_NO_PASID && s->root_scalable is true, then 
we pass is_pasid=true to vtd_report_fault().
If we change to use "pasid != IOMMU_NO_PASID" in this patch, then we pass 
is_pasid=false to vtd_report_fault() for the same scenario because pasid is 
IOMMU_NO_PASID now.

But using s->root_scalable could keep functions unchanged, right?

Thanks
Zhenzhong

Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-20 Thread Yi Liu 

On 5/19/26 15:20, Duan, Zhenzhong wrote:




-Original Message-
From: Liu, Yi L 
Subject: Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use
IOMMU_NO_PASID internally

On 5/9/26 12:08, Zhenzhong Duan wrote:

The PCI subsystem uses PCI_NO_PASID for requests-without-PASID, but VT-d
uses IOMMU_NO_PASID internally. This leads to conversion and checking code


s/VT-d uses IOMMU_NO_PASID internally/VT-d emulation uses
IOMMU_NO_PASID
internally (ecap.RPS==0)/


Will do.




between PCI_NO_PASID and IOMMU_NO_PASID throughout the

implementation.


Refactor to use IOMMU PASID consistently within Intel IOMMU by storing
IOMMU PASID value in vtd_as->pasid. After this change, PCI_NO_PASID is
only used at three boundary points:


a typo or the third boundary is missed?


Section 2 contains two points:

1. Convert when notifying UNMAP events via memory_region_notify_iommu()
2. Convert when returning IOMMUTLBEntry in vtd_iommu_translate()


got it.





1. PCI_NO_PASID -> IOMMU_NO_PASID: Convert PCI PASID to IOMMU PASID in
 vtd_find_add_as() and cache in vtd_as->pasid.
2. IOMMU_NO_PASID -> PCI_NO_PASID: Convert when notifying UNMAP events
 via memory_region_notify_iommu() and returning IOMMUTLBEntry in
 vtd_iommu_translate().

This eliminates conversion/checks in PASID table lookups, simplifies
invalidation logic with consistent PASID values, and improves code
readability. The PCI subsystem interface remains unchanged to maintain
compatibility with other IOMMU implementations that may not use PASID 0
for requests-without-PASID.

Suggested-by: Clement Mathieu--Drif 
Signed-off-by: Zhenzhong Duan 
---
   include/system/memory.h |   2 +-
   hw/i386/intel_iommu.c   | 164 +---
   hw/i386/intel_iommu_accel.c |   2 +-
   3 files changed, 80 insertions(+), 88 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 1417132f6d..1edb38b07d 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -150,7 +150,7 @@ struct IOMMUTLBEntry {
   hwaddr   translated_addr;
   hwaddr   addr_mask;  /* 0xfff = 4k translation */
   IOMMUAccessFlags perm;
-uint32_t pasid;
+uint32_t pasid; /* PCI pasid */
   };

   /*
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5e5dcdc274..b50c556c40 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -938,12 +938,8 @@ static int

vtd_get_pe_from_pasid_table(IntelIOMMUState *s,

   int vtd_ce_get_pasid_entry(IntelIOMMUState *s, VTDContextEntry *ce,
  VTDPASIDEntry *pe, uint32_t pasid)
   {
-dma_addr_t pasid_dir_base;
+dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);

-if (pasid == PCI_NO_PASID) {
-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);
   }

@@ -953,15 +949,10 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,
   uint32_t pasid)
   {
   int ret;
-dma_addr_t pasid_dir_base;
+dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
   VTDPASIDDirEntry pdire;
   VTDPASIDEntry pe;

-if (pasid == PCI_NO_PASID) {
-pasid = IOMMU_NO_PASID;
-}
-pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
-
   /*
* No present bit check since fpd is meaningful even
* if the present bit is clear.
@@ -1750,7 +1741,7 @@ static bool

vtd_switch_address_space(VTDAddressSpace *as)

*
* Need to disable ir for as with PASID.
*/
-if (as->pasid != PCI_NO_PASID) {
+if (as->pasid != IOMMU_NO_PASID) {
   memory_region_set_enabled(&as->iommu_ir, false);
   } else {
   memory_region_set_enabled(&as->iommu_ir, true);
@@ -1780,7 +1771,7 @@ static bool

vtd_switch_address_space(VTDAddressSpace *as)

* We enable per as memory region (iommu_ir_fault) for catching
* the translation for interrupt range through PASID + PT.
*/
-if (pt && as->pasid != PCI_NO_PASID) {
+if (pt && as->pasid != IOMMU_NO_PASID) {
   memory_region_set_enabled(&as->iommu_ir_fault, true);
   } else {
   memory_region_set_enabled(&as->iommu_ir_fault, false);
@@ -1892,7 +1883,7 @@ static VTDAddressSpace

*vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,


   VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
   {
-return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
+return vtd_get_as_by_sid_and_pasid(s, sid, IOMMU_NO_PASID);
   }

   static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)
@@ -2121,10 +2112,6 @@ static bool

vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,


   vtd_iommu_lock(s);

-if (pasid == PCI_NO_PASID && s->root_s

RE: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-19 Thread Duan, Zhenzhong 
>> @@ -4488,7 +4471,6 @@ static MemTxResult vtd_mem_ir_write(void *opaque,
>hwaddr addr,
>>  int ret = 0;
>>  MSIMessage from = {}, to = {};
>>  uint16_t sid = X86_IOMMU_SID_INVALID;
>> -    uint32_t pasid;
>>
>>  from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
>>  from.data = (uint32_t) value;
>> @@ -4496,11 +4478,11 @@ static MemTxResult vtd_mem_ir_write(void
>*opaque, hwaddr addr,
>>  if (!attrs.unspecified) {
>>  /* We have explicit Source ID */
>>  sid = attrs.requester_id;
>> -    pasid = attrs.pid != 0 ? attrs.pid : PCI_NO_PASID;
>
>Are we relying on the fact that IOMMU_NO_PASID is 0?
>Maybe we could add a comment/assert about that?

Yes, will add below to prevent anyone changing its value.

--- a/include/hw/core/iommu.h
+++ b/include/hw/core/iommu.h
@@ -41,6 +41,10 @@ enum host_iommu_quirks {
 HOST_IOMMU_QUIRK_NESTING_PARENT_BYPASS_RO = BIT_ULL(0),
 };

+/*
+ * Value 0 is shared between kernel and QEMU for IOMMU_NO_PASID, so never
+ * change it
+ */
 #define IOMMU_NO_PASID 0

Thanks
Zhenzhong

RE: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-19 Thread Duan, Zhenzhong 


>-Original Message-
>From: Liu, Yi L 
>Subject: Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use
>IOMMU_NO_PASID internally
>
>On 5/9/26 12:08, Zhenzhong Duan wrote:
>> The PCI subsystem uses PCI_NO_PASID for requests-without-PASID, but VT-d
>> uses IOMMU_NO_PASID internally. This leads to conversion and checking code
>
>s/VT-d uses IOMMU_NO_PASID internally/VT-d emulation uses
>IOMMU_NO_PASID
>internally (ecap.RPS==0)/

Will do.

>
>> between PCI_NO_PASID and IOMMU_NO_PASID throughout the
>implementation.
>>
>> Refactor to use IOMMU PASID consistently within Intel IOMMU by storing
>> IOMMU PASID value in vtd_as->pasid. After this change, PCI_NO_PASID is
>> only used at three boundary points:
>
>a typo or the third boundary is missed?

Section 2 contains two points:

1. Convert when notifying UNMAP events via memory_region_notify_iommu()
2. Convert when returning IOMMUTLBEntry in vtd_iommu_translate()

>
>>
>> 1. PCI_NO_PASID -> IOMMU_NO_PASID: Convert PCI PASID to IOMMU PASID in
>> vtd_find_add_as() and cache in vtd_as->pasid.
>> 2. IOMMU_NO_PASID -> PCI_NO_PASID: Convert when notifying UNMAP events
>> via memory_region_notify_iommu() and returning IOMMUTLBEntry in
>> vtd_iommu_translate().
>>
>> This eliminates conversion/checks in PASID table lookups, simplifies
>> invalidation logic with consistent PASID values, and improves code
>> readability. The PCI subsystem interface remains unchanged to maintain
>> compatibility with other IOMMU implementations that may not use PASID 0
>> for requests-without-PASID.
>>
>> Suggested-by: Clement Mathieu--Drif 
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>   include/system/memory.h |   2 +-
>>   hw/i386/intel_iommu.c   | 164 +---
>>   hw/i386/intel_iommu_accel.c |   2 +-
>>   3 files changed, 80 insertions(+), 88 deletions(-)
>>
>> diff --git a/include/system/memory.h b/include/system/memory.h
>> index 1417132f6d..1edb38b07d 100644
>> --- a/include/system/memory.h
>> +++ b/include/system/memory.h
>> @@ -150,7 +150,7 @@ struct IOMMUTLBEntry {
>>   hwaddr   translated_addr;
>>   hwaddr   addr_mask;  /* 0xfff = 4k translation */
>>   IOMMUAccessFlags perm;
>> -uint32_t pasid;
>> +uint32_t pasid; /* PCI pasid */
>>   };
>>
>>   /*
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 5e5dcdc274..b50c556c40 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -938,12 +938,8 @@ static int
>vtd_get_pe_from_pasid_table(IntelIOMMUState *s,
>>   int vtd_ce_get_pasid_entry(IntelIOMMUState *s, VTDContextEntry *ce,
>>  VTDPASIDEntry *pe, uint32_t pasid)
>>   {
>> -dma_addr_t pasid_dir_base;
>> +dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
>>
>> -if (pasid == PCI_NO_PASID) {
>> -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);
>>   }
>>
>> @@ -953,15 +949,10 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,
>>   uint32_t pasid)
>>   {
>>   int ret;
>> -dma_addr_t pasid_dir_base;
>> +dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
>>   VTDPASIDDirEntry pdire;
>>   VTDPASIDEntry pe;
>>
>> -if (pasid == PCI_NO_PASID) {
>> -pasid = IOMMU_NO_PASID;
>> -}
>> -pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
>> -
>>   /*
>>* No present bit check since fpd is meaningful even
>>* if the present bit is clear.
>> @@ -1750,7 +1741,7 @@ static bool
>vtd_switch_address_space(VTDAddressSpace *as)
>>*
>>* Need to disable ir for as with PASID.
>>*/
>> -if (as->pasid != PCI_NO_PASID) {
>> +if (as->pasid != IOMMU_NO_PASID) {
>>   memory_region_set_enabled(&as->iommu_ir, false);
>>   } else {
>>   memory_region_set_enabled(&as->iommu_ir, true);
>> @@ -1780,7 +1771,7 @@ static bool
>vtd_switch_address_space(VTDAddressSpace *as)
>>* We enable per as memory region (iommu_ir_fault) for catching
>>* the translation for interrupt range through PASID + PT.
>>*/
>> -if (pt && as->pasid != PCI_NO_PASID) {
>>

Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-16 Thread Clément MATHIEU--DRIF 


On Sat, 2026-05-09 at 00:08 -0400, Zhenzhong Duan wrote:
> The PCI subsystem uses PCI_NO_PASID for requests-without-PASID, but VT-d  
> uses IOMMU_NO_PASID internally. This leads to conversion and checking code  
> between PCI_NO_PASID and IOMMU_NO_PASID throughout the implementation.
> 
> Refactor to use IOMMU PASID consistently within Intel IOMMU by storing  
> IOMMU PASID value in vtd_as->pasid. After this change, PCI_NO_PASID is  
> only used at three boundary points:
> 
> 1. PCI_NO_PASID -> IOMMU_NO_PASID: Convert PCI PASID to IOMMU PASID in  
>    vtd_find_add_as() and cache in vtd_as->pasid.  
> 2. IOMMU_NO_PASID -> PCI_NO_PASID: Convert when notifying UNMAP events  
>    via memory_region_notify_iommu() and returning IOMMUTLBEntry in  
>    vtd_iommu_translate().
> 
> This eliminates conversion/checks in PASID table lookups, simplifies  
> invalidation logic with consistent PASID values, and improves code  
> readability. The PCI subsystem interface remains unchanged to maintain  
> compatibility with other IOMMU implementations that may not use PASID 0  
> for requests-without-PASID.
> 
> Suggested-by: Clement Mathieu--Drif 
> <[[email protected]](mailto:[email protected])>  
> Signed-off-by: Zhenzhong Duan 
> <[[email protected]](mailto:[email protected])>  
> ---  
>  include/system/memory.h |   2 +-  
>  hw/i386/intel_iommu.c   | 164 +---  
>  hw/i386/intel_iommu_accel.c |   2 +-  
>  3 files changed, 80 insertions(+), 88 deletions(-)
> 
> diff --git a/include/system/memory.h b/include/system/memory.h  
> index 1417132f6d..1edb38b07d 100644  
> --- a/include/system/memory.h  
> +++ b/include/system/memory.h  
> @@ -150,7 +150,7 @@ struct IOMMUTLBEntry {  
>  hwaddr   translated_addr;  
>  hwaddr   addr_mask;  /* 0xfff = 4k translation */  
>  IOMMUAccessFlags perm;  
> -    uint32_t pasid;  
> +    uint32_t pasid; /* PCI pasid */  
>  };  
>    
>  /*  
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c  
> index 5e5dcdc274..b50c556c40 100644  
> --- a/hw/i386/intel_iommu.c  
> +++ b/hw/i386/intel_iommu.c  
> @@ -938,12 +938,8 @@ static int vtd_get_pe_from_pasid_table(IntelIOMMUState 
> *s,  
>  int vtd_ce_get_pasid_entry(IntelIOMMUState *s, VTDContextEntry *ce,  
>     VTDPASIDEntry *pe, uint32_t pasid)  
>  {  
> -    dma_addr_t pasid_dir_base;  
> +    dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);  
>    
> -    if (pasid == PCI_NO_PASID) {  
> -    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);  
>  }  
>    
> @@ -953,15 +949,10 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,  
>  uint32_t pasid)  
>  {  
>  int ret;  
> -    dma_addr_t pasid_dir_base;  
> +    dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);  
>  VTDPASIDDirEntry pdire;  
>  VTDPASIDEntry pe;  
>    
> -    if (pasid == PCI_NO_PASID) {  
> -    pasid = IOMMU_NO_PASID;  
> -    }  
> -    pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);  
> -  
>  /*  
>   * No present bit check since fpd is meaningful even  
>   * if the present bit is clear.  
> @@ -1750,7 +1741,7 @@ static bool vtd_switch_address_space(VTDAddressSpace 
> *as)  
>   *  
>   * Need to disable ir for as with PASID.  
>   */  
> -    if (as->pasid != PCI_NO_PASID) {  
> +    if (as->pasid != IOMMU_NO_PASID) {  
>  memory_region_set_enabled(&as->iommu_ir, false);  
>  } else {  
>  memory_region_set_enabled(&as->iommu_ir, true);  
> @@ -1780,7 +1771,7 @@ static bool vtd_switch_address_space(VTDAddressSpace 
> *as)  
>   * We enable per as memory region (iommu_ir_fault) for catching  
>   * the translation for interrupt range through PASID + PT.  
>   */  
> -    if (pt && as->pasid != PCI_NO_PASID) {  
> +    if (pt && as->pasid != IOMMU_NO_PASID) {  
>  memory_region_set_enabled(&as->iommu_ir_fault, true);  
>  } else {  
>  memory_region_set_enabled(&as->iommu_ir_fault, false);  
> @@ -1892,7 +1883,7 @@ static VTDAddressSpace 
> *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,  
>    
>  VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)  
>  {  
> -    return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);  
> +    return vtd_get_as_by_sid_and_pasid(s, sid, IOMMU_NO_PASID);  
>  }  
>    
>  static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)  
> @@ -2121,10 +2112,6 @@ static bool vtd_do_iommu_translate(VTDAddressSpace 
> *vtd_as, PCIBus *bus,  
>    
>  vtd_iommu_lock(s);  
>    
> -    if (pasid == PCI_NO_PASID && s->root_scalable) {  
> -    pasid = IOMMU_NO_PASID;  
> -    }  
> -  
>  /* Try to fetch pte from IOTLB */  
>  iot

Re: [PATCH v5 08/15] intel_iommu: Refactor PASID processing to use IOMMU_NO_PASID internally

2026-05-14 Thread Yi Liu 

On 5/9/26 12:08, Zhenzhong Duan wrote:

The PCI subsystem uses PCI_NO_PASID for requests-without-PASID, but VT-d
uses IOMMU_NO_PASID internally. This leads to conversion and checking code


s/VT-d uses IOMMU_NO_PASID internally/VT-d emulation uses IOMMU_NO_PASID
internally (ecap.RPS==0)/


between PCI_NO_PASID and IOMMU_NO_PASID throughout the implementation.

Refactor to use IOMMU PASID consistently within Intel IOMMU by storing
IOMMU PASID value in vtd_as->pasid. After this change, PCI_NO_PASID is
only used at three boundary points:


a typo or the third boundary is missed?



1. PCI_NO_PASID -> IOMMU_NO_PASID: Convert PCI PASID to IOMMU PASID in
vtd_find_add_as() and cache in vtd_as->pasid.
2. IOMMU_NO_PASID -> PCI_NO_PASID: Convert when notifying UNMAP events
via memory_region_notify_iommu() and returning IOMMUTLBEntry in
vtd_iommu_translate().

This eliminates conversion/checks in PASID table lookups, simplifies
invalidation logic with consistent PASID values, and improves code
readability. The PCI subsystem interface remains unchanged to maintain
compatibility with other IOMMU implementations that may not use PASID 0
for requests-without-PASID.

Suggested-by: Clement Mathieu--Drif 
Signed-off-by: Zhenzhong Duan 
---
  include/system/memory.h |   2 +-
  hw/i386/intel_iommu.c   | 164 +---
  hw/i386/intel_iommu_accel.c |   2 +-
  3 files changed, 80 insertions(+), 88 deletions(-)

diff --git a/include/system/memory.h b/include/system/memory.h
index 1417132f6d..1edb38b07d 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -150,7 +150,7 @@ struct IOMMUTLBEntry {
  hwaddr   translated_addr;
  hwaddr   addr_mask;  /* 0xfff = 4k translation */
  IOMMUAccessFlags perm;
-uint32_t pasid;
+uint32_t pasid; /* PCI pasid */
  };
  
  /*

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5e5dcdc274..b50c556c40 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -938,12 +938,8 @@ static int vtd_get_pe_from_pasid_table(IntelIOMMUState *s,
  int vtd_ce_get_pasid_entry(IntelIOMMUState *s, VTDContextEntry *ce,
 VTDPASIDEntry *pe, uint32_t pasid)
  {
-dma_addr_t pasid_dir_base;
+dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
  
-if (pasid == PCI_NO_PASID) {

-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);
  }
  
@@ -953,15 +949,10 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,

  uint32_t pasid)
  {
  int ret;
-dma_addr_t pasid_dir_base;
+dma_addr_t pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
  VTDPASIDDirEntry pdire;
  VTDPASIDEntry pe;
  
-if (pasid == PCI_NO_PASID) {

-pasid = IOMMU_NO_PASID;
-}
-pasid_dir_base = VTD_CE_GET_PASID_DIR_TABLE(ce);
-
  /*
   * No present bit check since fpd is meaningful even
   * if the present bit is clear.
@@ -1750,7 +1741,7 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
   *
   * Need to disable ir for as with PASID.
   */
-if (as->pasid != PCI_NO_PASID) {
+if (as->pasid != IOMMU_NO_PASID) {
  memory_region_set_enabled(&as->iommu_ir, false);
  } else {
  memory_region_set_enabled(&as->iommu_ir, true);
@@ -1780,7 +1771,7 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
   * We enable per as memory region (iommu_ir_fault) for catching
   * the translation for interrupt range through PASID + PT.
   */
-if (pt && as->pasid != PCI_NO_PASID) {
+if (pt && as->pasid != IOMMU_NO_PASID) {
  memory_region_set_enabled(&as->iommu_ir_fault, true);
  } else {
  memory_region_set_enabled(&as->iommu_ir_fault, false);
@@ -1892,7 +1883,7 @@ static VTDAddressSpace 
*vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,
  
  VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)

  {
-return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
+return vtd_get_as_by_sid_and_pasid(s, sid, IOMMU_NO_PASID);
  }
  
  static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)

@@ -2121,10 +2112,6 @@ static bool vtd_do_iommu_translate(VTDAddressSpace 
*vtd_as, PCIBus *bus,
  
  vtd_iommu_lock(s);
  
-if (pasid == PCI_NO_PASID && s->root_scalable) {

-pasid = IOMMU_NO_PASID;
-}
-
  /* Try to fetch pte from IOTLB */
  iotlb_entry = vtd_lookup_iotlb(s, source_id, pasid, addr);
  if (iotlb_entry) {
@@ -2235,7 +,7 @@ static bool vtd_do_iommu_translate(VTDAddressSpace 
*vtd_as, PCIBus *bus,
  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, p