* [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported
@ 2018-05-19 16:52 Sinan Kaya
2018-05-21 13:25 ` Sinan Kaya
2018-06-19 22:13 ` Bjorn Helgaas
0 siblings, 2 replies; 3+ messages in thread
From: Sinan Kaya @ 2018-05-19 16:52 UTC (permalink / raw)
To: linux-pci, timur
Cc: Sinan Kaya, linux-arm-msm, Bjorn Helgaas, open list,
linux-arm-kernel
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.
A translation agent takes PASID value and the address information from the
TLP to look up the physical address in the system.
If a bridge drops the TLP prefix, the translation agent can resolve the
address to an incorrect location and cause data corruption. Prevent
this condition by requiring End-to-End TLP prefix to be supported on the
entire data path between the endpoint and the root port.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/pci/ats.c | 16 ++++++++++++++++
include/uapi/linux/pci_regs.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 89305b5..0bcded5 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -265,7 +265,9 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
int pci_enable_pasid(struct pci_dev *pdev, int features)
{
u16 control, supported;
+ struct pci_dev *bridge;
int pos;
+ u32 cap;
if (WARN_ON(pdev->pasid_enabled))
return -EBUSY;
@@ -274,6 +276,20 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
if (!pos)
return -EINVAL;
+ bridge = pci_upstream_bridge(pdev);
+ while (bridge) {
+ if (!pci_find_capability(bridge, PCI_CAP_ID_EXP))
+ return -EINVAL;
+
+ if (pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap))
+ return -EINVAL;
+
+ if (!(cap & PCI_EXP_DEVCAP2_E2ETLP))
+ return -EINVAL;
+
+ bridge = pci_upstream_bridge(bridge);
+ }
+
pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 103ba79..d91dea5 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -634,6 +634,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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported
2018-05-19 16:52 [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported Sinan Kaya
@ 2018-05-21 13:25 ` Sinan Kaya
2018-06-19 22:13 ` Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Sinan Kaya @ 2018-05-21 13:25 UTC (permalink / raw)
To: linux-pci, timur
Cc: Bjorn Helgaas, linux-arm-msm, iommu, open list, linux-arm-kernel
+iommu folks.
On 5/19/2018 12:52 PM, 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.
>
> A translation agent takes PASID value and the address information from the
> TLP to look up the physical address in the system.
>
> If a bridge drops the TLP prefix, the translation agent can resolve the
> address to an incorrect location and cause data corruption. Prevent
> this condition by requiring End-to-End TLP prefix to be supported on the
> entire data path between the endpoint and the root port.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/pci/ats.c | 16 ++++++++++++++++
> include/uapi/linux/pci_regs.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 89305b5..0bcded5 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -265,7 +265,9 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
> int pci_enable_pasid(struct pci_dev *pdev, int features)
> {
> u16 control, supported;
> + struct pci_dev *bridge;
> int pos;
> + u32 cap;
>
> if (WARN_ON(pdev->pasid_enabled))
> return -EBUSY;
> @@ -274,6 +276,20 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
> if (!pos)
> return -EINVAL;
>
> + bridge = pci_upstream_bridge(pdev);
> + while (bridge) {
> + if (!pci_find_capability(bridge, PCI_CAP_ID_EXP))
> + return -EINVAL;
> +
> + if (pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap))
> + return -EINVAL;
> +
> + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP))
> + return -EINVAL;
> +
> + bridge = pci_upstream_bridge(bridge);
> + }
> +
> pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
> supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 103ba79..d91dea5 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -634,6 +634,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 */
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported
2018-05-19 16:52 [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported Sinan Kaya
2018-05-21 13:25 ` Sinan Kaya
@ 2018-06-19 22:13 ` Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2018-06-19 22:13 UTC (permalink / raw)
To: Sinan Kaya
Cc: linux-pci, timur, open list, linux-arm-msm, Bjorn Helgaas,
linux-arm-kernel
On Sat, May 19, 2018 at 12:52:49PM -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.
>
> A translation agent takes PASID value and the address information from the
> TLP to look up the physical address in the system.
>
> If a bridge drops the TLP prefix, the translation agent can resolve the
> address to an incorrect location and cause data corruption. Prevent
> this condition by requiring End-to-End TLP prefix to be supported on the
> entire data path between the endpoint and the root port.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/pci/ats.c | 16 ++++++++++++++++
> include/uapi/linux/pci_regs.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 89305b5..0bcded5 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -265,7 +265,9 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
> int pci_enable_pasid(struct pci_dev *pdev, int features)
> {
> u16 control, supported;
> + struct pci_dev *bridge;
> int pos;
> + u32 cap;
>
> if (WARN_ON(pdev->pasid_enabled))
> return -EBUSY;
> @@ -274,6 +276,20 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
> if (!pos)
> return -EINVAL;
>
> + bridge = pci_upstream_bridge(pdev);
> + while (bridge) {
> + if (!pci_find_capability(bridge, PCI_CAP_ID_EXP))
> + return -EINVAL;
> +
> + if (pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap))
> + return -EINVAL;
> +
> + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP))
> + return -EINVAL;
> +
> + bridge = pci_upstream_bridge(bridge);
> + }
Hi Sinan,
Would you consider implementing this in the style of c46fd358070f
("PCI/ASPM: Enable Latency Tolerance Reporting when supported"), i.e.,
add a bit in struct pci_dev so we don't have to search up the tree and
re-lookup the PCIe capability several times for the endpoints of the
hierarchy?
This loop looks much like the one in pci_enable_atomic_ops_to_root()
but doesn't use exactly the same iteration.
Maybe we should someday collect up and combine all the places that
read the capability registers so we don't have to read them multiple
times? Not sure if that would make readability better or worse -- it
would add a second place to look at, while with this patch, everything
is all in one place.
> +
> pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
> supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 103ba79..d91dea5 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -634,6 +634,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-19 22:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-19 16:52 [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported Sinan Kaya
2018-05-21 13:25 ` Sinan Kaya
2018-06-19 22:13 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).