From: "Pali Rohár" <pali@kernel.org>
To: Stefan Roese <sr@denx.de>
Cc: "Marek Behún" <marek.behun@nic.cz>,
"Konstantin Porotchkin" <kostap@marvell.com>,
u-boot@lists.denx.de
Subject: [PATCH] arm: a37xx: pci: Fix handling PIO config error responses
Date: Mon, 9 Aug 2021 09:53:13 +0200 [thread overview]
Message-ID: <20210809075313.24868-1-pali@kernel.org> (raw)
Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to OS is
allowed only for 4-byte PCI_VENDOR_ID config read request and only when
CRSSVE bit in Root Port PCIe device is enabled. In all other error PCIe
Root Complex must return all-ones.
So implement this logic in pci-aardvark.c driver properly.
aardvark HW does not have Root Port PCIe device and U-Boot does not
implement emulation of this device. So expect that CRSSVE bit is set as
U-Boot can already handle CRS value for PCI_VENDOR_ID config read request.
More callers of pci_bus_read_config() function in U-Boot do not check for
return value, but check readback value. Therefore always fill readback
value in pcie_advk_read_config() function. On error fill all-ones of
correct size as it is required for PCIe Root Complex.
And also correctly propagates error from failed config write request to
return value of pcie_advk_write_config() function. Most U-Boot callers
ignores this return value, but it is a good idea to return correct value
from function.
These issues about return value of failed config read requests, including
special handling of CRS were reported by Lorenzo and Bjorn for Linux kernel
driver pci-aardvark together with quotes from PCIe r4.0 spec, see details:
https://lore.kernel.org/linux-pci/20210624213345.3617-1-pali@kernel.org/t/#u
Signed-off-by: Pali Rohár <pali@kernel.org>
---
drivers/pci/pci-aardvark.c | 52 +++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 1b9bae7cca72..815b26162f15 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -177,7 +177,6 @@
#define LINK_MAX_RETRIES 10
#define LINK_WAIT_TIMEOUT 100000
-#define CFG_RD_UR_VAL 0xFFFFFFFF
#define CFG_RD_CRS_VAL 0xFFFF0001
/**
@@ -263,12 +262,12 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie)
* pcie_advk_check_pio_status() - Validate PIO status and get the read result
*
* @pcie: Pointer to the PCI bus
- * @read: Read from or write to configuration space - true(read) false(write)
- * @read_val: Pointer to the read result, only valid when read is true
+ * @allow_crs: Only for read requests, if CRS response is allowed
+ * @read_val: Pointer to the read result
*
*/
static int pcie_advk_check_pio_status(struct pcie_advk *pcie,
- bool read,
+ bool allow_crs,
uint *read_val)
{
uint reg;
@@ -286,22 +285,16 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie,
break;
}
/* Get the read result */
- if (read)
+ if (read_val)
*read_val = advk_readl(pcie, PIO_RD_DATA);
/* No error */
strcomp_status = NULL;
break;
case PIO_COMPLETION_STATUS_UR:
- if (read) {
- /* For reading, UR is not an error status. */
- *read_val = CFG_RD_UR_VAL;
- strcomp_status = NULL;
- } else {
- strcomp_status = "UR";
- }
+ strcomp_status = "UR";
break;
case PIO_COMPLETION_STATUS_CRS:
- if (read) {
+ if (allow_crs && read_val) {
/* For reading, CRS is not an error status. */
*read_val = CFG_RD_CRS_VAL;
strcomp_status = NULL;
@@ -352,6 +345,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
enum pci_size_t size)
{
struct pcie_advk *pcie = dev_get_priv(bus);
+ bool allow_crs;
uint reg;
int ret;
@@ -364,13 +358,17 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
return 0;
}
+ allow_crs = (offset == PCI_VENDOR_ID) && (size == 4);
+
if (advk_readl(pcie, PIO_START)) {
dev_err(pcie->dev,
"Previous PIO read/write transfer is still running\n");
- if (offset != PCI_VENDOR_ID)
- return -EINVAL;
- *valuep = CFG_RD_CRS_VAL;
- return 0;
+ if (allow_crs) {
+ *valuep = CFG_RD_CRS_VAL;
+ return 0;
+ }
+ *valuep = pci_get_ff(size);
+ return -EINVAL;
}
/* Program the control register */
@@ -392,16 +390,20 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
advk_writel(pcie, 1, PIO_START);
if (!pcie_advk_wait_pio(pcie)) {
- if (offset != PCI_VENDOR_ID)
- return -EINVAL;
- *valuep = CFG_RD_CRS_VAL;
- return 0;
+ if (allow_crs) {
+ *valuep = CFG_RD_CRS_VAL;
+ return 0;
+ }
+ *valuep = pci_get_ff(size);
+ return -EINVAL;
}
/* Check PIO status and get the read result */
- ret = pcie_advk_check_pio_status(pcie, true, ®);
- if (ret)
+ ret = pcie_advk_check_pio_status(pcie, allow_crs, ®);
+ if (ret) {
+ *valuep = pci_get_ff(size);
return ret;
+ }
dev_dbg(pcie->dev, "(addr,size,val)=(0x%04x, %d, 0x%08x)\n",
offset, size, reg);
@@ -511,9 +513,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
}
/* Check PIO status */
- pcie_advk_check_pio_status(pcie, false, ®);
-
- return 0;
+ return pcie_advk_check_pio_status(pcie, false, NULL);
}
/**
--
2.20.1
next reply other threads:[~2021-08-09 7:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-09 7:53 Pali Rohár [this message]
2021-08-11 6:38 ` [PATCH] arm: a37xx: pci: Fix handling PIO config error responses Stefan Roese
2021-08-11 8:29 ` Stefan Roese
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210809075313.24868-1-pali@kernel.org \
--to=pali@kernel.org \
--cc=kostap@marvell.com \
--cc=marek.behun@nic.cz \
--cc=sr@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox