| Message ID | 1530372264-20653-1-git-send-email-okaya@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On Sat, Jun 30, 2018 at 11:24:24AM -0400, Sinan Kaya wrote: > A PCIe endpoint carries the process address space identifier (PASID) in > the TLP prefix as part of the memory read/write transaction. The address > information in the TLP is relevant only for a given PASID context. > > An IOMMU takes PASID value and the address information from the > TLP to look up the physical address in the system. > > PASID is an End-End TLP Prefix (PCIe r4.0, sec 6.20). Sec 2.2.10.2 says > > It is an error to receive a TLP with an End-End TLP Prefix by a > Receiver that does not support End-End TLP Prefixes. A TLP in > violation of this rule is handled as a Malformed TLP. This is a > reported error associated with the Receiving Port (see Section 6.2). > > Prevent error condition by proactively requiring End-to-End TLP > prefix to be supported on the entire data path between the endpoint and > the root port before enabling PASID. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Applied to pci/virtualization for v4.19, thanks! > --- > drivers/pci/ats.c | 3 +++ > drivers/pci/probe.c | 24 ++++++++++++++++++++++++ > include/linux/pci.h | 1 + > include/uapi/linux/pci_regs.h | 1 + > 4 files changed, 29 insertions(+) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index 4923a2a..5b78f3b 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -273,6 +273,9 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) > if (WARN_ON(pdev->pasid_enabled)) > return -EBUSY; > > + if (!pdev->eetlp_prefix_path) > + return -EINVAL; > + > pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); > if (!pos) > return -EINVAL; > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index ac876e3..39fd3ac 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2042,6 +2042,29 @@ static void pci_configure_ltr(struct pci_dev *dev) > #endif > } > > +static void pci_configure_eetlp_prefix(struct pci_dev *dev) > +{ > +#ifdef CONFIG_PCI_PASID > + struct pci_dev *bridge; > + u32 cap; > + > + if (!pci_is_pcie(dev)) > + return; > + > + pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap); > + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP)) > + return; > + > + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > + dev->eetlp_prefix_path = 1; > + else { > + bridge = pci_upstream_bridge(dev); > + if (bridge && bridge->eetlp_prefix_path) > + dev->eetlp_prefix_path = 1; > + } > +#endif > +} > + > static void pci_configure_device(struct pci_dev *dev) > { > struct hotplug_params hpp; > @@ -2051,6 +2074,7 @@ static void pci_configure_device(struct pci_dev *dev) > pci_configure_extended_tags(dev, NULL); > pci_configure_relaxed_ordering(dev); > pci_configure_ltr(dev); > + pci_configure_eetlp_prefix(dev); > > memset(&hpp, 0, sizeof(hpp)); > ret = pci_get_hp_params(dev, &hpp); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 340029b..6ba8184 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -350,6 +350,7 @@ struct pci_dev { > unsigned int ltr_path:1; /* Latency Tolerance Reporting > supported from root to here */ > #endif > + unsigned int eetlp_prefix_path:1; /* End-to-End TLP Prefix */ > > pci_channel_state_t error_state; /* Current connectivity state */ > struct device dev; /* Generic device interface */ > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index 4da87e2..a617ab2 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -636,6 +636,7 @@ > #define PCI_EXP_DEVCAP2_OBFF_MASK 0x000c0000 /* OBFF support mechanism */ > #define PCI_EXP_DEVCAP2_OBFF_MSG 0x00040000 /* New message signaling */ > #define PCI_EXP_DEVCAP2_OBFF_WAKE 0x00080000 /* Re-use WAKE# for OBFF */ > +#define PCI_EXP_DEVCAP2_E2ETLP 0x00200000 /* End-to-End TLP Prefix */ > #define PCI_EXP_DEVCTL2 40 /* Device Control 2 */ > #define PCI_EXP_DEVCTL2_COMP_TIMEOUT 0x000f /* Completion Timeout Value */ > #define PCI_EXP_DEVCTL2_COMP_TMOUT_DIS 0x0010 /* Completion Timeout Disable */ > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sinan + IOMMU list. On Sat, Jun 30, 2018 at 11:24:24AM -0400, Sinan Kaya wrote: > A PCIe endpoint carries the process address space identifier (PASID) in > the TLP prefix as part of the memory read/write transaction. The address > information in the TLP is relevant only for a given PASID context. > > An IOMMU takes PASID value and the address information from the > TLP to look up the physical address in the system. > > PASID is an End-End TLP Prefix (PCIe r4.0, sec 6.20). Sec 2.2.10.2 says > > It is an error to receive a TLP with an End-End TLP Prefix by a > Receiver that does not support End-End TLP Prefixes. A TLP in > violation of this rule is handled as a Malformed TLP. This is a > reported error associated with the Receiving Port (see Section 6.2). > > Prevent error condition by proactively requiring End-to-End TLP > prefix to be supported on the entire data path between the endpoint and > the root port before enabling PASID. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > > +static void pci_configure_eetlp_prefix(struct pci_dev *dev) > +{ > +#ifdef CONFIG_PCI_PASID > + struct pci_dev *bridge; > + u32 cap; > + > + if (!pci_is_pcie(dev)) > + return; > + > + pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap); > + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP)) > + return; Forgot to notice this.. I'm not sure if the same enforcement is required for devices that are RCIEP. The spec isn't clear about calling any excemption. Although it should be simple for devices to expose e2etlp support, but if they don't that should be ok, since there are nothing between itself and the root port. We are seeking help from our SIG reps, but thought I'll ask here as well if there are other opinions. > + > + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > + dev->eetlp_prefix_path = 1; > + else { > + bridge = pci_upstream_bridge(dev); > + if (bridge && bridge->eetlp_prefix_path) > + dev->eetlp_prefix_path = 1; > + } > +#endif > +} Cheers, Ashok
On 9/26/2018 12:22 PM, Raj, Ashok wrote: >> pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap); >> + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP)) >> + return; > Forgot to notice this.. I'm not sure if the same enforcement is > required for devices that are RCIEP. The spec isn't clear about calling > any excemption. Although it should be simple for devices to expose > e2etlp support, but if they don't that should be ok, since there are > nothing between itself and the root port. > > We are seeking help from our SIG reps, but thought I'll ask here as well > if there are other opinions. > This patch actually broke the integrated endpoints like you were mentioning. It was fixed by a follow up patch from nvidia guys. https://github.com/torvalds/linux/commit/9d27e39d309c93025ae6aa97236af15bef2a5f1f#diff-a7f0acd715e991df5dfb450d2b9abebc
On 9/26/2018 1:20 PM, Sinan Kaya wrote: > > This patch actually broke the integrated endpoints like you were mentioning. > It was fixed by a follow up patch from nvidia guys. > > https://github.com/torvalds/linux/commit/9d27e39d309c93025ae6aa97236af15bef2a5f1f#diff-a7f0acd715e991df5dfb450d2b9abebc > Turns out Felix works at AMD. Apologies for the name confusion.
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 4923a2a..5b78f3b 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -273,6 +273,9 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) if (WARN_ON(pdev->pasid_enabled)) return -EBUSY; + if (!pdev->eetlp_prefix_path) + return -EINVAL; + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); if (!pos) return -EINVAL; diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index ac876e3..39fd3ac 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2042,6 +2042,29 @@ static void pci_configure_ltr(struct pci_dev *dev) #endif } +static void pci_configure_eetlp_prefix(struct pci_dev *dev) +{ +#ifdef CONFIG_PCI_PASID + struct pci_dev *bridge; + u32 cap; + + if (!pci_is_pcie(dev)) + return; + + pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap); + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP)) + return; + + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) + dev->eetlp_prefix_path = 1; + else { + bridge = pci_upstream_bridge(dev); + if (bridge && bridge->eetlp_prefix_path) + dev->eetlp_prefix_path = 1; + } +#endif +} + static void pci_configure_device(struct pci_dev *dev) { struct hotplug_params hpp; @@ -2051,6 +2074,7 @@ static void pci_configure_device(struct pci_dev *dev) pci_configure_extended_tags(dev, NULL); pci_configure_relaxed_ordering(dev); pci_configure_ltr(dev); + pci_configure_eetlp_prefix(dev); memset(&hpp, 0, sizeof(hpp)); ret = pci_get_hp_params(dev, &hpp); diff --git a/include/linux/pci.h b/include/linux/pci.h index 340029b..6ba8184 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -350,6 +350,7 @@ struct pci_dev { unsigned int ltr_path:1; /* Latency Tolerance Reporting supported from root to here */ #endif + unsigned int eetlp_prefix_path:1; /* End-to-End TLP Prefix */ pci_channel_state_t error_state; /* Current connectivity state */ struct device dev; /* Generic device interface */ diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 4da87e2..a617ab2 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -636,6 +636,7 @@ #define PCI_EXP_DEVCAP2_OBFF_MASK 0x000c0000 /* OBFF support mechanism */ #define PCI_EXP_DEVCAP2_OBFF_MSG 0x00040000 /* New message signaling */ #define PCI_EXP_DEVCAP2_OBFF_WAKE 0x00080000 /* Re-use WAKE# for OBFF */ +#define PCI_EXP_DEVCAP2_E2ETLP 0x00200000 /* End-to-End TLP Prefix */ #define PCI_EXP_DEVCTL2 40 /* Device Control 2 */ #define PCI_EXP_DEVCTL2_COMP_TIMEOUT 0x000f /* Completion Timeout Value */ #define PCI_EXP_DEVCTL2_COMP_TMOUT_DIS 0x0010 /* Completion Timeout Disable */
A PCIe endpoint carries the process address space identifier (PASID) in the TLP prefix as part of the memory read/write transaction. The address information in the TLP is relevant only for a given PASID context. An IOMMU takes PASID value and the address information from the TLP to look up the physical address in the system. PASID is an End-End TLP Prefix (PCIe r4.0, sec 6.20). Sec 2.2.10.2 says It is an error to receive a TLP with an End-End TLP Prefix by a Receiver that does not support End-End TLP Prefixes. A TLP in violation of this rule is handled as a Malformed TLP. This is a reported error associated with the Receiving Port (see Section 6.2). Prevent error condition by proactively requiring End-to-End TLP prefix to be supported on the entire data path between the endpoint and the root port before enabling PASID. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/pci/ats.c | 3 +++ drivers/pci/probe.c | 24 ++++++++++++++++++++++++ include/linux/pci.h | 1 + include/uapi/linux/pci_regs.h | 1 + 4 files changed, 29 insertions(+)