Messages in this thread | | | Subject | Re: Re: [PATCH] PCI: Relax ACS requirement for Intel RCiEP devices. | From | Darrel Goeddel <> | Date | Fri, 29 May 2020 10:59:32 -0500 |
| |
On 5/28/20 4:38 PM, Alex Williamson wrote: > On Thu, 28 May 2020 13:57:42 -0700 > Ashok Raj <ashok.raj@intel.com> wrote: > >> All Intel platforms guarantee that all root complex implementations >> must send transactions up to IOMMU for address translations. Hence for >> RCiEP devices that are Vendor ID Intel, can claim exception for lack of >> ACS support. >> >> >> 3.16 Root-Complex Peer to Peer Considerations >> When DMA remapping is enabled, peer-to-peer requests through the >> Root-Complex must be handled >> as follows: >> • The input address in the request is translated (through first-level, >> second-level or nested translation) to a host physical address (HPA). >> The address decoding for peer addresses must be done only on the >> translated HPA. Hardware implementations are free to further limit >> peer-to-peer accesses to specific host physical address regions >> (or to completely disallow peer-forwarding of translated requests). >> • Since address translation changes the contents (address field) of >> the PCI Express Transaction Layer Packet (TLP), for PCI Express >> peer-to-peer requests with ECRC, the Root-Complex hardware must use >> the new ECRC (re-computed with the translated address) if it >> decides to forward the TLP as a peer request. >> • Root-ports, and multi-function root-complex integrated endpoints, may >> support additional peerto-peer control features by supporting PCI Express >> Access Control Services (ACS) capability. Refer to ACS capability in >> PCI Express specifications for details. >> >> Since Linux didn't give special treatment to allow this exception, certain >> RCiEP MFD devices are getting grouped in a single iommu group. This >> doesn't permit a single device to be assigned to a guest for instance. >> >> In one vendor system: Device 14.x were grouped in a single IOMMU group. >> >> /sys/kernel/iommu_groups/5/devices/0000:00:14.0 >> /sys/kernel/iommu_groups/5/devices/0000:00:14.2 >> /sys/kernel/iommu_groups/5/devices/0000:00:14.3 >> >> After the patch: >> /sys/kernel/iommu_groups/5/devices/0000:00:14.0 >> /sys/kernel/iommu_groups/5/devices/0000:00:14.2 >> /sys/kernel/iommu_groups/6/devices/0000:00:14.3 <<< new group >> >> 14.0 and 14.2 are integrated devices, but legacy end points. >> Whereas 14.3 was a PCIe compliant RCiEP. >> >> 00:14.3 Network controller: Intel Corporation Device 9df0 (rev 30) >> Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 >> >> This permits assigning this device to a guest VM. >> >> Fixes: f096c061f552 ("iommu: Rework iommu_group_get_for_pci_dev()") > > I don't really understand this Fixes tag. This seems like a feature, > not a fix. If you want it in stable releases as a feature, request it > via Cc: stable@vger.kernel.org. I'd drop that tag, that's my nit. > Otherwise: > > Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
I have tested this patch with 5.6.14 as well as a slightly modified version (without pci_acs_ctrl_enabled()) in a 3.10 enterprise linux kernel.
Tested-by: Darrel Goeddel <dgoeddel@forcepoint.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com> >> To: Joerg Roedel <joro@8bytes.org> >> To: Bjorn Helgaas <bhelgaas@google.com> >> Cc: linux-kernel@vger.kernel.org >> Cc: iommu@lists.linux-foundation.org >> Cc: Lu Baolu <baolu.lu@linux.intel.com> >> Cc: Alex Williamson <alex.williamson@redhat.com> >> Cc: Darrel Goeddel <DGoeddel@forcepoint.com> >> Cc: Mark Scott <mscott@forcepoint.com>, >> Cc: Romil Sharma <rsharma@forcepoint.com> >> Cc: Ashok Raj <ashok.raj@intel.com> >> --- >> v2: Moved functionality from iommu to pci quirks - Alex Williamson >> >> drivers/pci/quirks.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 28c9a2409c50..63373ca0a3fe 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -4682,6 +4682,20 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags) >> PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT); >> } >> >> +static int pci_quirk_rciep_acs(struct pci_dev *dev, u16 acs_flags) >> +{ >> + /* >> + * RCiEP's are required to allow p2p only on translated addresses. >> + * Refer to Intel VT-d specification Section 3.16 Root-Complex Peer >> + * to Peer Considerations >> + */ >> + if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END) >> + return -ENOTTY; >> + >> + return pci_acs_ctrl_enabled(acs_flags, >> + PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF); >> +} >> + >> static int pci_quirk_brcm_acs(struct pci_dev *dev, u16 acs_flags) >> { >> /* >> @@ -4764,6 +4778,7 @@ static const struct pci_dev_acs_enabled { >> /* I219 */ >> { PCI_VENDOR_ID_INTEL, 0x15b7, pci_quirk_mf_endpoint_acs }, >> { PCI_VENDOR_ID_INTEL, 0x15b8, pci_quirk_mf_endpoint_acs }, >> + { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_rciep_acs }, >> /* QCOM QDF2xxx root ports */ >> { PCI_VENDOR_ID_QCOM, 0x0400, pci_quirk_qcom_rp_acs }, >> { PCI_VENDOR_ID_QCOM, 0x0401, pci_quirk_qcom_rp_acs }, >
| |