Re: [PATCH v4 09/10] iommu: Make iommu_queue_iopf() more generic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux