> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Monday, September 11, 2023 8:27 PM
>
> >
> > Out of curiosity. Is it a valid configuration which has
> REQUEST_PASID_VALID
> > set but RESP_PASID_VALID cleared? I'm unclear why another response
> > flag is required beyond what the request flag has told...
>
> This seems to have uncovered a bug in VT-d driver.
>
> The PCIe spec (Section 10.4.2.2) states:
>
> "
> If a Page Request has a PASID, the corresponding PRG Response Message
> may optionally contain one as well.
>
> If the PRG Response PASID Required bit is Clear, PRG Response Messages
> do not have a PASID. If the PRG Response PASID Required bit is Set, PRG
> Response Messages have a PASID if the Page Request also had one. The
> Function is permitted to use the PASID value from the prefix in
> conjunction with the PRG Index to match requests and responses.
> "
>
> The "PRG Response PASID Required bit" is a read-only field in the PCI
> page request status register. It is represented by
> "pdev->pasid_required".
>
> So below code in VT-d driver is not correct:
>
> 542 static int intel_svm_prq_report(struct intel_iommu *iommu, struct
> device *dev,
> 543 struct page_req_dsc *desc)
> 544 {
>
> [...]
>
> 556
> 557 if (desc->lpig)
> 558 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
> 559 if (desc->pasid_present) {
> 560 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> 561 event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
> 562 }
> [...]
>
> The right logic should be
>
> if (pdev->pasid_required)
> event.fault.prm.flags |=
> IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
>
> Thoughts?
>
yes, it's the right fix. We haven't seen any bug report probably because
all SVM-capable devices have pasid_required set? 😊
