On 9/13/23 10:25 AM, Tian, Kevin wrote:
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? 😊
More precisely, the idxd devices have pasid_required set. :-)
Anyway, I will post a formal fix for this.
Best regards,
baolu