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 +2222,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.
"
