Skip to contentSkip to conversation

Amazon's Annapurna Labs DT-based PCIe host controller driver

PATCH Superseded Jonathan Chocron 4 participants 20 messages Sep 5, 2019 – Sep 12, 2019

Subsystems (5)
Device tree bindings PCI PCI native host bridge and endpoint drivers PCI driver for synopsys DesignWare +1 more subsystems

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Jonathan Chocron

author maintainer
#0 lore raw

v5 cover Amazon's Annapurna Labs DT-based PCIe host controller driver

This series adds support for Amazon's Annapurna Labs DT-based PCIe host controller driver.
Additionally, it adds 3 quirks (ACS, VPD and MSI-X) and 2 generic DWC patches.

Changes since v4:
- Moved the HEADER_TYPE validations to after pp->ops->host_init() and ep->ops->ep_init()
- Changed to dw_pcie_rd_own_conf() instead of dw_pcie_readb_dbi() for reading the HEADER_TYPE
- Used exsitng quirk_blacklist_vpd() instead of quirk_al_vpd_release()
- Added a newline in ACS quirk comment

Changes since v3:
- Removed PATCH 8/8 since the usage of the PCI flags will be discussed in the upcoming LPC
- Align commit subject with the folder convention
- Added explanation regarding ECAM "overload" mechanism
- Switched to read/write{_relaxed} APIs
- Modified a dev_err to dev_dbg
- Removed unnecessary variable
- Removed driver details from dt-binding description
- Changed to SoC specific compatibles
- Fixed typo in a commit message
- Added comment regarding MSI in the MSI-X quirk

Changes since v2:
- Added al_pcie_controller_readl/writel() wrappers
- Reorganized local vars in several functions according to reverse tree structure
- Removed unnecessary check of ret value
- Changed return type of al_pcie_config_prepare() from int to void
- Removed check if link is up from probe() [done internally in dw_pcie_rd/wr_conf()]

Changes since v1:
- Added comment regarding 0x0031 being used as a dev_id for non root-port devices as well
- Fixed different message/comment/print wordings
- Added panic stacktrace to commit message of MSI-x quirk patch
- Changed to pci_warn() instead of dev_warn()
- Added unit_address after node_name in dt-binding
- Updated Kconfig help description
- Used GENMASK and FIELD_PREP/GET where appropriate
- Removed leftover field from struct al_pcie and moved all ptrs to
the beginning
- Re-wrapped function definitions and invocations to use fewer lines
- Change %p to %px in dbg prints in rd/wr_conf() functions
- Removed validation that the port is configured to RC mode (as this is added generically in PATCH 7/8)
- Removed unnecessary variable initializations
- Swtiched to %pR for printing resources

Ali Saidi (1):
PCI: Add ACS quirk for Amazon Annapurna Labs root ports

Jonathan Chocron (6):
PCI: Add Amazon's Annapurna Labs vendor ID
PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port
PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs
Root Port
dt-bindings: PCI: Add Amazon's Annapurna Labs PCIe host bridge binding
PCI: dwc: al: Add support for DW based driver type
PCI: dwc: Add validation that PCIe core is set to correct mode

 .../devicetree/bindings/pci/pcie-al.txt       |  46 +++
 MAINTAINERS                                   |   3 +-
 drivers/pci/controller/dwc/Kconfig            |  12 +
 drivers/pci/controller/dwc/pcie-al.c          | 365 ++++++++++++++++++
 .../pci/controller/dwc/pcie-designware-ep.c   |   8 +
 .../pci/controller/dwc/pcie-designware-host.c |  16 +
 drivers/pci/quirks.c                          |  38 ++
 drivers/pci/vpd.c                             |   6 +
 include/linux/pci_ids.h                       |   2 +
 9 files changed, 495 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-al.txt
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH v5 1/7 PCI: Add Amazon's Annapurna Labs vendor ID

Add Amazon's Annapurna Labs vendor ID to pci_ids.h.

Signed-off-by Jonathan Chocron <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 include/linux/pci_ids.h | 2 ++
 1 file changed, 2 insertions(+)
include/linux/pci_ids.h +2 -0
index f59a6f98900c..f3130542c752 100644
@@ -2571,6 +2571,8 @@
 
 #define PCI_VENDOR_ID_ASMEDIA		0x1b21
 
+#define PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS	0x1c36
+
 #define PCI_VENDOR_ID_CIRCUITCO		0x1cc8
 #define PCI_SUBSYSTEM_ID_CIRCUITCO_MINNOWBOARD	0x0001
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH v5 2/7 PCI: Add ACS quirk for Amazon Annapurna Labs root ports

In reply to Jonathan Chocron

From: Ali Saidi <[email protected]>

The Amazon's Annapurna Labs root ports don't advertise an ACS capability, but they don't allow peer-to-peer transactions and do validate bus numbers through the SMMU. Additionally, it's not possible for one RP to pass traffic to another RP.

Signed-off-by Ali Saidi <[email protected]>
Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/quirks.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/pci/quirks.c +20 -0
index ded60757a573..8fe765592943 100644
@@ -4418,6 +4418,24 @@ static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags)
 	return ret;
 }
 
+static int pci_quirk_al_acs(struct pci_dev *dev, u16 acs_flags)
+{
+	/*
+	 * Amazon's Annapurna Labs root ports don't include an ACS capability,
+	 * but do include ACS-like functionality. The hardware doesn't support
+	 * peer-to-peer transactions via the root port and each has a unique
+	 * segment number.
+	 *
+	 * Additionally, the root ports cannot send traffic to each other.
+	 */
+	acs_flags &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_SV | PCI_ACS_UF);
+
+	if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+		return -ENOTTY;
+
+	return acs_flags ? 0 : 1;
+}
+
 /*
  * Sunrise Point PCH root ports implement ACS, but unfortunately as shown in
  * the datasheet (Intel 100 Series Chipset Family PCH Datasheet, Vol. 2,

@@ -4611,6 +4629,8 @@ static const struct pci_dev_acs_enabled {
 	{ PCI_VENDOR_ID_AMPERE, 0xE00A, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00B, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00C, pci_quirk_xgene_acs },
+	/* Amazon Annapurna Labs */
+	{ PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031, pci_quirk_al_acs },
 	{ 0 }
 };
Signature · 1 line
2.17.1
Acked-by on v5 2/7
#3 lore raw

Re: [PATCH v5 2/7] PCI: Add ACS quirk for Amazon Annapurna Labs root ports

Quoted 11 lines · Jonathan Chocron

From: Ali Saidi <[email protected]>

The Amazon's Annapurna Labs root ports don't advertise an ACS capability, but they don't allow peer-to-peer transactions and do validate bus numbers through the SMMU. Additionally, it's not possible for one RP to pass traffic to another RP.

Signed-off-by Ali Saidi <[email protected]>
Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>

But please tweak it as below ...

Quoted 13 lines
---
 drivers/pci/quirks.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/pci/quirks.c +1 -0
index ded60757a573..8fe765592943 100644
@@ -4418,6 +4418,24 @@ static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags)
 	return ret;
 }
 
+static int pci_quirk_al_acs(struct pci_dev *dev, u16 acs_flags)

drivers/pci/quirks.c static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags) · line 4431

Quoted 10 lines
drivers/pci/quirks.c +10 -0
+{
+	/*
+	 * Amazon's Annapurna Labs root ports don't include an ACS capability,
+	 * but do include ACS-like functionality. The hardware doesn't support
+	 * peer-to-peer transactions via the root port and each has a unique
+	 * segment number.
+	 *
+	 * Additionally, the root ports cannot send traffic to each other.
+	 */
+	acs_flags &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_SV | PCI_ACS_UF);

There are several quirks that use this same set of bits, but they don't use the same order, which is a needless difference.

Can you reorder them in the bit 0 ... bit 7 order? I.e.,

PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF

drivers/pci/quirks.c static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags) · line 4434

Quoted 2 lines
drivers/pci/quirks.c +2 -0
+	if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+		return -ENOTTY;

This could go first (above the comment) so all the acs_flags stuff is together.

Quoted 18 lines
drivers/pci/quirks.c +5 -0
+	return acs_flags ? 0 : 1;
+}
+
 /*
  * Sunrise Point PCH root ports implement ACS, but unfortunately as shown in
  * the datasheet (Intel 100 Series Chipset Family PCH Datasheet, Vol. 2,

@@ -4611,6 +4629,8 @@ static const struct pci_dev_acs_enabled {
 	{ PCI_VENDOR_ID_AMPERE, 0xE00A, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00B, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00C, pci_quirk_xgene_acs },
+	/* Amazon Annapurna Labs */
+	{ PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031, pci_quirk_al_acs },
 	{ 0 }
 };
Signature · 1 line
2.17.1
author maintainer
#4 lore raw

Re: [PATCH v5 2/7] PCI: Add ACS quirk for Amazon Annapurna Labs root ports

Quoted 9 lines · Bjorn Helgaas
Quoted 8 lines · Jonathan Chocron

From: Ali Saidi <[email protected]>

The Amazon's Annapurna Labs root ports don't advertise an ACS capability, but they don't allow peer-to-peer transactions and do validate bus numbers through the SMMU. Additionally, it's not possible
for one RP to pass traffic to another RP.

Quoted 9 lines
Quoted 4 lines
Signed-off-by Ali Saidi <[email protected]>
Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>

But please tweak it as below ...

Will be part of v6.

Quoted 33 lines
Quoted 28 lines
---
 drivers/pci/quirks.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/pci/quirks.c +5 -0
index ded60757a573..8fe765592943 100644
@@ -4418,6 +4418,24 @@ static int pci_quirk_qcom_rp_acs(struct
 pci_dev *dev, u16 acs_flags)
 	return ret;
 }
 
+static int pci_quirk_al_acs(struct pci_dev *dev, u16 acs_flags)
+{
+	/*
+	 * Amazon's Annapurna Labs root ports don't include an ACS
 capability,
+	 * but do include ACS-like functionality. The hardware doesn't

support

drivers/pci/quirks.c +1 -0
+	 * peer-to-peer transactions via the root port and each has a

unique

drivers/pci/quirks.c +3 -0
+	 * segment number.
+	 *
+	 * Additionally, the root ports cannot send traffic to each

other.

drivers/pci/quirks.c +2 -0
+	 */
+	acs_flags &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_SV |
Quoted 1 line

PCI_ACS_UF);

There are several quirks that use this same set of bits, but they don't use the same order, which is a needless difference.

Quoted 4 lines

Can you reorder them in the bit 0 ... bit 7 order? I.e.,

PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF

Done.

Quoted 6 lines

drivers/pci/quirks.c static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags) · line 4434

Quoted 2 lines
drivers/pci/quirks.c +2 -0
+	if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+		return -ENOTTY;

This could go first (above the comment) so all the acs_flags stuff is together.

Done.

Quoted 21 lines
Quoted 21 lines
drivers/pci/quirks.c +3 -0
+	return acs_flags ? 0 : 1;
+}
+
 /*
  * Sunrise Point PCH root ports implement ACS, but unfortunately

as shown in
* the datasheet (Intel 100 Series Chipset Family PCH Datasheet,
Vol. 2,

drivers/pci/quirks.c +2 -0
@@ -4611,6 +4629,8 @@ static const struct pci_dev_acs_enabled {
 	{ PCI_VENDOR_ID_AMPERE, 0xE00A, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00B, pci_quirk_xgene_acs },
 	{ PCI_VENDOR_ID_AMPERE, 0xE00C, pci_quirk_xgene_acs },
+	/* Amazon Annapurna Labs */
+	{ PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031, pci_quirk_al_acs
 },
 	{ 0 }
 };
Signature · 2 lines
-- 
2.17.1
author maintainer
#5 lore raw

PATCH v5 3/7 PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port

In reply to Jonathan Chocron

The Amazon Annapurna Labs PCIe Root Port exposes the VPD capability, but there is no actual support for it.

Trying to access the VPD (for example, as part of lspci -vv or when reading the vpd sysfs file), results in the following warning print:

pcieport 0001:00:00.0: VPD access failed. This is likely a firmware bug on this device. Contact the card vendor for a firmware update

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
---
 drivers/pci/vpd.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/pci/vpd.c +6 -0
index 4963c2e2bd4c..7915d10f9aa1 100644
@@ -571,6 +571,12 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005f, quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, PCI_ANY_ID,
 		quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_QLOGIC, 0x2261, quirk_blacklist_vpd);
+/*
+ * The Amazon Annapurna Labs 0x0031 device id is reused for other non Root Port
+ * device types, so the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_blacklist_vpd);
 
 /*
  * For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the
Signature · 1 line
2.17.1
Reviewed-by on v5 3/7
#6 lore raw

Re: [PATCH v5 3/7] PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port

Quoted 10 lines · Jonathan Chocron

The Amazon Annapurna Labs PCIe Root Port exposes the VPD capability, but there is no actual support for it.

Trying to access the VPD (for example, as part of lspci -vv or when reading the vpd sysfs file), results in the following warning print:

pcieport 0001:00:00.0: VPD access failed. This is likely a firmware bug on this device. Contact the card vendor for a firmware update

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
Quoted 24 lines
---
 drivers/pci/vpd.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/pci/vpd.c +6 -0
index 4963c2e2bd4c..7915d10f9aa1 100644
@@ -571,6 +571,12 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005f, quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, PCI_ANY_ID,
 		quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_QLOGIC, 0x2261, quirk_blacklist_vpd);
+/*
+ * The Amazon Annapurna Labs 0x0031 device id is reused for other non Root Port
+ * device types, so the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_blacklist_vpd);
 
 /*
  * For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the
Signature · 1 line
2.17.1
Acked-by on v5 3/7
#7 lore raw

Re: [PATCH v5 3/7] PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port

In reply to Jonathan Chocron
Quoted 2 lines · Jonathan Chocron

The Amazon Annapurna Labs PCIe Root Port exposes the VPD capability, but there is no actual support for it.

Oops. Another oops for the device ID reuse mentioned below.

Quoted 4 lines

Trying to access the VPD (for example, as part of lspci -vv or when reading the vpd sysfs file), results in the following warning print:

pcieport 0001:00:00.0: VPD access failed. This is likely a firmware bug on this device. Contact the card vendor for a firmware update

Thanks for not wrapping the message (keeping it together makes it easier to grep for). Maybe indent it two spaces since it's quoted material.

*Is* this a firmware defect? E.g., could firmware disable this capability so it doesn't appear in config space, as it apparently can for the MSI-X capability?

Quoted 2 lines
Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>
Quoted 24 lines
---
 drivers/pci/vpd.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/pci/vpd.c +6 -0
index 4963c2e2bd4c..7915d10f9aa1 100644
@@ -571,6 +571,12 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005f, quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, PCI_ANY_ID,
 		quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_QLOGIC, 0x2261, quirk_blacklist_vpd);
+/*
+ * The Amazon Annapurna Labs 0x0031 device id is reused for other non Root Port
+ * device types, so the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_blacklist_vpd);
 
 /*
  * For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the
Signature · 1 line
2.17.1
author maintainer
#8 lore raw

Re: [PATCH v5 3/7] PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port

Quoted 11 lines · Bjorn Helgaas
Quoted 3 lines · Jonathan Chocron

The Amazon Annapurna Labs PCIe Root Port exposes the VPD capability,
but there is no actual support for it.

Oops. Another oops for the device ID reuse mentioned below.

Quoted 4 lines

Trying to access the VPD (for example, as part of lspci -vv or when reading the vpd sysfs file), results in the following warning print:

Quoted 6 lines
Quoted 3 lines

pcieport 0001:00:00.0: VPD access failed. This is likely a firmware bug on this device. Contact the card vendor for a firmware update

Thanks for not wrapping the message (keeping it together makes it easier to grep for).

My thoughts exactly :)

Quoted 3 lines

Maybe indent it two spaces since it's quoted
material.

Will be done as part of v6.

Quoted 4 lines

*Is* this a firmware defect? E.g., could firmware disable this capability so it doesn't appear in config space, as it apparently can for the MSI-X capability?

Yes, the FW could potentially disable it so it doesn't appear in the config space.

In theory, VPD should be able to work if some other entity would listen to the VPD requests and respond, but there are no plans to add support for such an entity.

Quoted 36 lines
Quoted 2 lines
Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>
Quoted 31 lines
---
 drivers/pci/vpd.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/pci/vpd.c +2 -0
index 4963c2e2bd4c..7915d10f9aa1 100644
@@ -571,6 +571,12 @@
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005f,
 quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, PCI_ANY_ID,
 		quirk_blacklist_vpd);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_QLOGIC, 0x2261,
 quirk_blacklist_vpd);
+/*
+ * The Amazon Annapurna Labs 0x0031 device id is reused for other

non Root Port
+ * device types, so the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */ +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,

drivers/pci/vpd.c +1 -0
+			      PCI_CLASS_BRIDGE_PCI, 8,

quirk_blacklist_vpd);

/*
* For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the

Signature · 1 line
2.17.1
author maintainer
#9 lore raw

PATCH v5 4/7 PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs Root Port

In reply to Jonathan Chocron

The Root Port (identified by [1c36:0031]) doesn't support MSI-X. On some platforms it is configured to not advertise the capability at all, while on others it (mistakenly) does. This causes a panic during initialization by the pcieport driver, since it tries to configure the MSI-X capability. Specifically, when trying to access the MSI-X table a "non-existing addr" exception occurs.

Example stacktrace snippet:

[    1.632363] SError Interrupt on CPU2, code 0xbf000000 -- SError
[    1.632364] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632365] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632365] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    1.632366] pc : __pci_enable_msix_range+0x4e4/0x608
[    1.632367] lr : __pci_enable_msix_range+0x498/0x608
[    1.632367] sp : ffffff80117db700
[    1.632368] x29: ffffff80117db700 x28: 0000000000000001
[    1.632370] x27: 0000000000000001 x26: 0000000000000000
[    1.632372] x25: ffffffd3e9d8c0b0 x24: 0000000000000000
[    1.632373] x23: 0000000000000000 x22: 0000000000000000
[    1.632375] x21: 0000000000000001 x20: 0000000000000000
[    1.632376] x19: ffffffd3e9d8c000 x18: ffffffffffffffff
[    1.632378] x17: 0000000000000000 x16: 0000000000000000
[    1.632379] x15: ffffff80116496c8 x14: ffffffd3e9844503
[    1.632380] x13: ffffffd3e9844502 x12: 0000000000000038
[    1.632382] x11: ffffffffffffff00 x10: 0000000000000040
[    1.632384] x9 : ffffff801165e270 x8 : ffffff801165e268
[    1.632385] x7 : 0000000000000002 x6 : 00000000000000b2
[    1.632387] x5 : ffffffd3e9d8c2c0 x4 : 0000000000000000
[    1.632388] x3 : 0000000000000000 x2 : 0000000000000000
[    1.632390] x1 : 0000000000000000 x0 : ffffffd3e9844680
[    1.632392] Kernel panic - not syncing: Asynchronous SError Interrupt
[    1.632393] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632394] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632394] Call trace:
[    1.632395]  dump_backtrace+0x0/0x140
[    1.632395]  show_stack+0x14/0x20
[    1.632396]  dump_stack+0xa8/0xcc
[    1.632396]  panic+0x140/0x334
[    1.632397]  nmi_panic+0x6c/0x70
[    1.632398]  arm64_serror_panic+0x74/0x88
[    1.632398]  __pte_error+0x0/0x28
[    1.632399]  el1_error+0x84/0xf8
[    1.632400]  __pci_enable_msix_range+0x4e4/0x608
[    1.632400]  pci_alloc_irq_vectors_affinity+0xdc/0x150
[    1.632401]  pcie_port_device_register+0x2b8/0x4e0
[    1.632402]  pcie_portdrv_probe+0x34/0xf0

Notice that this quirk also disables MSI (which may work, but hasn't been tested nor has a current use case), since currently there is no standard way to disable only MSI-X.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/pci/quirks.c +18 -0
index 8fe765592943..5a8ea5fdeae7 100644
@@ -2977,6 +2977,24 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0x10a1,
 			quirk_msi_intx_disable_qca_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0xe091,
 			quirk_msi_intx_disable_qca_bug);
+
+/*
+ * Amazon's Annapurna Labs 1c36:0031 Root Ports don't support MSI-X, so it
+ * should be disabled on platforms where the device (mistakenly) advertises it.
+ *
+ * Notice that this quirk also disables MSI (which may work, but hasn't been
+ * tested), since currently there is no standard way to disable only MSI-X.
+ *
+ * The 0031 device id is reused for other non Root Port device types,
+ * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+static void quirk_al_msi_disable(struct pci_dev *dev)
+{
+	dev->no_msi = 1;
+	pci_warn(dev, "Disabling MSI/MSI-X\n");
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_al_msi_disable);
 #endif /* CONFIG_PCI_MSI */
 
 /*
Signature · 1 line
2.17.1
#10 lore raw

Re: [PATCH v5 4/7] PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs Root Port

s/Add quirk to disable/Disable/ in subject

Quoted 6 lines · Jonathan Chocron

The Root Port (identified by [1c36:0031]) doesn't support MSI-X. On some platforms it is configured to not advertise the capability at all, while on others it (mistakenly) does. This causes a panic during initialization by the pcieport driver, since it tries to configure the MSI-X capability. Specifically, when trying to access the MSI-X table a "non-existing addr" exception occurs.

MSI-X configuration is all in memory space (not config space), so I guess this device has a BAR (maybe more) and the MSI-X table access would be to something in that BAR? Or is there junk in the capability so we try to access something completely outside the BAR? [I wonder if it'd be worth adding some validation to make sure the Table and PBA areas are contained in the BAR?]

The reason I'm curious about all this is because on the PCI side this is probably an Unsupported Request or similar error, and on *most* platforms, this does not cause a panic. If it was a read, the read usually gets ~0 data, and writes are dropped.

So my concern is that we'll avoid this panic by disabling MSI, but we'll see other panics in other places.

Quoted 1 line

Example stacktrace snippet:

[Remove the timestamps since they're not really relevant and indent the quote a couple spaces]

Quoted 82 lines
[    1.632363] SError Interrupt on CPU2, code 0xbf000000 -- SError
[    1.632364] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632365] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632365] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    1.632366] pc : __pci_enable_msix_range+0x4e4/0x608
[    1.632367] lr : __pci_enable_msix_range+0x498/0x608
[    1.632367] sp : ffffff80117db700
[    1.632368] x29: ffffff80117db700 x28: 0000000000000001
[    1.632370] x27: 0000000000000001 x26: 0000000000000000
[    1.632372] x25: ffffffd3e9d8c0b0 x24: 0000000000000000
[    1.632373] x23: 0000000000000000 x22: 0000000000000000
[    1.632375] x21: 0000000000000001 x20: 0000000000000000
[    1.632376] x19: ffffffd3e9d8c000 x18: ffffffffffffffff
[    1.632378] x17: 0000000000000000 x16: 0000000000000000
[    1.632379] x15: ffffff80116496c8 x14: ffffffd3e9844503
[    1.632380] x13: ffffffd3e9844502 x12: 0000000000000038
[    1.632382] x11: ffffffffffffff00 x10: 0000000000000040
[    1.632384] x9 : ffffff801165e270 x8 : ffffff801165e268
[    1.632385] x7 : 0000000000000002 x6 : 00000000000000b2
[    1.632387] x5 : ffffffd3e9d8c2c0 x4 : 0000000000000000
[    1.632388] x3 : 0000000000000000 x2 : 0000000000000000
[    1.632390] x1 : 0000000000000000 x0 : ffffffd3e9844680
[    1.632392] Kernel panic - not syncing: Asynchronous SError Interrupt
[    1.632393] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632394] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632394] Call trace:
[    1.632395]  dump_backtrace+0x0/0x140
[    1.632395]  show_stack+0x14/0x20
[    1.632396]  dump_stack+0xa8/0xcc
[    1.632396]  panic+0x140/0x334
[    1.632397]  nmi_panic+0x6c/0x70
[    1.632398]  arm64_serror_panic+0x74/0x88
[    1.632398]  __pte_error+0x0/0x28
[    1.632399]  el1_error+0x84/0xf8
[    1.632400]  __pci_enable_msix_range+0x4e4/0x608
[    1.632400]  pci_alloc_irq_vectors_affinity+0xdc/0x150
[    1.632401]  pcie_port_device_register+0x2b8/0x4e0
[    1.632402]  pcie_portdrv_probe+0x34/0xf0

Notice that this quirk also disables MSI (which may work, but hasn't been tested nor has a current use case), since currently there is no standard way to disable only MSI-X.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/pci/quirks.c +18 -0
index 8fe765592943..5a8ea5fdeae7 100644
@@ -2977,6 +2977,24 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0x10a1,
 			quirk_msi_intx_disable_qca_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0xe091,
 			quirk_msi_intx_disable_qca_bug);
+
+/*
+ * Amazon's Annapurna Labs 1c36:0031 Root Ports don't support MSI-X, so it
+ * should be disabled on platforms where the device (mistakenly) advertises it.
+ *
+ * Notice that this quirk also disables MSI (which may work, but hasn't been
+ * tested), since currently there is no standard way to disable only MSI-X.
+ *
+ * The 0031 device id is reused for other non Root Port device types,
+ * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+static void quirk_al_msi_disable(struct pci_dev *dev)
+{
+	dev->no_msi = 1;
+	pci_warn(dev, "Disabling MSI/MSI-X\n");
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_al_msi_disable);
 #endif /* CONFIG_PCI_MSI */
 
 /*
Signature · 1 line
2.17.1
#11 lore raw

Re: [PATCH v5 4/7] PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs Root Port

Quoted 22 lines · Bjorn Helgaas

s/Add quirk to disable/Disable/ in subject

Quoted 6 lines · Jonathan Chocron

The Root Port (identified by [1c36:0031]) doesn't support MSI-X. On some platforms it is configured to not advertise the capability at all, while on others it (mistakenly) does. This causes a panic during initialization by the pcieport driver, since it tries to configure the MSI-X capability. Specifically, when trying to access the MSI-X table a "non-existing addr" exception occurs.

MSI-X configuration is all in memory space (not config space), so I guess this device has a BAR (maybe more) and the MSI-X table access would be to something in that BAR? Or is there junk in the capability so we try to access something completely outside the BAR? [I wonder if it'd be worth adding some validation to make sure the Table and PBA areas are contained in the BAR?]

The reason I'm curious about all this is because on the PCI side this is probably an Unsupported Request or similar error, and on *most* platforms, this does not cause a panic. If it was a read, the read usually gets ~0 data, and writes are dropped.

Quoted 7 lines

So my concern is that we'll avoid this panic by disabling MSI, but we'll see other panics in other places.

Quoted 1 line

Example stacktrace snippet:

[Remove the timestamps since they're not really relevant and indent the quote a couple spaces]

Jonathan, can you please clarify and update the patch so that we can proceed with the series and try to get it into v5.4 ?

Thanks,
Lorenzo

Quoted 83 lines
Quoted 83 lines
[    1.632363] SError Interrupt on CPU2, code 0xbf000000 -- SError
[    1.632364] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632365] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632365] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    1.632366] pc : __pci_enable_msix_range+0x4e4/0x608
[    1.632367] lr : __pci_enable_msix_range+0x498/0x608
[    1.632367] sp : ffffff80117db700
[    1.632368] x29: ffffff80117db700 x28: 0000000000000001
[    1.632370] x27: 0000000000000001 x26: 0000000000000000
[    1.632372] x25: ffffffd3e9d8c0b0 x24: 0000000000000000
[    1.632373] x23: 0000000000000000 x22: 0000000000000000
[    1.632375] x21: 0000000000000001 x20: 0000000000000000
[    1.632376] x19: ffffffd3e9d8c000 x18: ffffffffffffffff
[    1.632378] x17: 0000000000000000 x16: 0000000000000000
[    1.632379] x15: ffffff80116496c8 x14: ffffffd3e9844503
[    1.632380] x13: ffffffd3e9844502 x12: 0000000000000038
[    1.632382] x11: ffffffffffffff00 x10: 0000000000000040
[    1.632384] x9 : ffffff801165e270 x8 : ffffff801165e268
[    1.632385] x7 : 0000000000000002 x6 : 00000000000000b2
[    1.632387] x5 : ffffffd3e9d8c2c0 x4 : 0000000000000000
[    1.632388] x3 : 0000000000000000 x2 : 0000000000000000
[    1.632390] x1 : 0000000000000000 x0 : ffffffd3e9844680
[    1.632392] Kernel panic - not syncing: Asynchronous SError Interrupt
[    1.632393] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632394] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632394] Call trace:
[    1.632395]  dump_backtrace+0x0/0x140
[    1.632395]  show_stack+0x14/0x20
[    1.632396]  dump_stack+0xa8/0xcc
[    1.632396]  panic+0x140/0x334
[    1.632397]  nmi_panic+0x6c/0x70
[    1.632398]  arm64_serror_panic+0x74/0x88
[    1.632398]  __pte_error+0x0/0x28
[    1.632399]  el1_error+0x84/0xf8
[    1.632400]  __pci_enable_msix_range+0x4e4/0x608
[    1.632400]  pci_alloc_irq_vectors_affinity+0xdc/0x150
[    1.632401]  pcie_port_device_register+0x2b8/0x4e0
[    1.632402]  pcie_portdrv_probe+0x34/0xf0

Notice that this quirk also disables MSI (which may work, but hasn't been tested nor has a current use case), since currently there is no standard way to disable only MSI-X.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/pci/quirks.c +18 -0
index 8fe765592943..5a8ea5fdeae7 100644
@@ -2977,6 +2977,24 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0x10a1,
 			quirk_msi_intx_disable_qca_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0xe091,
 			quirk_msi_intx_disable_qca_bug);
+
+/*
+ * Amazon's Annapurna Labs 1c36:0031 Root Ports don't support MSI-X, so it
+ * should be disabled on platforms where the device (mistakenly) advertises it.
+ *
+ * Notice that this quirk also disables MSI (which may work, but hasn't been
+ * tested), since currently there is no standard way to disable only MSI-X.
+ *
+ * The 0031 device id is reused for other non Root Port device types,
+ * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */
+static void quirk_al_msi_disable(struct pci_dev *dev)
+{
+	dev->no_msi = 1;
+	pci_warn(dev, "Disabling MSI/MSI-X\n");
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_al_msi_disable);
 #endif /* CONFIG_PCI_MSI */
 
 /*
Signature · 1 line
2.17.1
author maintainer
#12 lore raw

Re: [PATCH v5 4/7] PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs Root Port

In reply to Bjorn Helgaas
Quoted 14 lines · Bjorn Helgaas

s/Add quirk to disable/Disable/ in subject

Quoted 10 lines · Jonathan Chocron

The Root Port (identified by [1c36:0031]) doesn't support MSI-X. On some
platforms it is configured to not advertise the capability at all, while
on others it (mistakenly) does. This causes a panic during initialization by the pcieport driver, since it tries to configure the
MSI-X capability. Specifically, when trying to access the MSI-X table
a "non-existing addr" exception occurs.

Quoted 9 lines

MSI-X configuration is all in memory space (not config space), so I guess this device has a BAR (maybe more) and the MSI-X table access would be to something in that BAR? Or is there junk in the capability
so we try to access something completely outside the BAR? [I wonder if
it'd be worth adding some validation to make sure the Table and PBA areas are contained in the BAR?]

There is no actual memory allocated for the MSI-X table behind the BAR, so it attempts to access non existing regs, resulting in the SError.

Quoted 5 lines

The reason I'm curious about all this is because on the PCI side this is probably an Unsupported Request or similar error, and on *most* platforms, this does not cause a panic. If it was a read, the read usually gets ~0 data, and writes are dropped.

I'm not entirely sure how this is implemented in the HW (i.e. accessing the Host controller's memory space), but I assume that an AXI error returns, which translates to the Serror. I can verify with our HW engineers if you think it is blocking.

Quoted 8 lines

So my concern is that we'll avoid this panic by disabling MSI, but we'll see other panics in other places.

Quoted 1 line

Example stacktrace snippet:

[Remove the timestamps since they're not really relevant and indent the quote a couple spaces]

Ack.

Quoted 96 lines
Quoted 96 lines
[    1.632363] SError Interrupt on CPU2, code 0xbf000000 -- SError
[    1.632364] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-
Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632365] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632365] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    1.632366] pc : __pci_enable_msix_range+0x4e4/0x608
[    1.632367] lr : __pci_enable_msix_range+0x498/0x608
[    1.632367] sp : ffffff80117db700
[    1.632368] x29: ffffff80117db700 x28: 0000000000000001
[    1.632370] x27: 0000000000000001 x26: 0000000000000000
[    1.632372] x25: ffffffd3e9d8c0b0 x24: 0000000000000000
[    1.632373] x23: 0000000000000000 x22: 0000000000000000
[    1.632375] x21: 0000000000000001 x20: 0000000000000000
[    1.632376] x19: ffffffd3e9d8c000 x18: ffffffffffffffff
[    1.632378] x17: 0000000000000000 x16: 0000000000000000
[    1.632379] x15: ffffff80116496c8 x14: ffffffd3e9844503
[    1.632380] x13: ffffffd3e9844502 x12: 0000000000000038
[    1.632382] x11: ffffffffffffff00 x10: 0000000000000040
[    1.632384] x9 : ffffff801165e270 x8 : ffffff801165e268
[    1.632385] x7 : 0000000000000002 x6 : 00000000000000b2
[    1.632387] x5 : ffffffd3e9d8c2c0 x4 : 0000000000000000
[    1.632388] x3 : 0000000000000000 x2 : 0000000000000000
[    1.632390] x1 : 0000000000000000 x0 : ffffffd3e9844680
[    1.632392] Kernel panic - not syncing: Asynchronous SError
Interrupt
[    1.632393] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-
Jonny-14847-ge76f1d4a1828-dirty #33
[    1.632394] Hardware name: Annapurna Labs Alpine V3 EVP (DT)
[    1.632394] Call trace:
[    1.632395]  dump_backtrace+0x0/0x140
[    1.632395]  show_stack+0x14/0x20
[    1.632396]  dump_stack+0xa8/0xcc
[    1.632396]  panic+0x140/0x334
[    1.632397]  nmi_panic+0x6c/0x70
[    1.632398]  arm64_serror_panic+0x74/0x88
[    1.632398]  __pte_error+0x0/0x28
[    1.632399]  el1_error+0x84/0xf8
[    1.632400]  __pci_enable_msix_range+0x4e4/0x608
[    1.632400]  pci_alloc_irq_vectors_affinity+0xdc/0x150
[    1.632401]  pcie_port_device_register+0x2b8/0x4e0
[    1.632402]  pcie_portdrv_probe+0x34/0xf0

Notice that this quirk also disables MSI (which may work, but hasn't
been tested nor has a current use case), since currently there is no
standard way to disable only MSI-X.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/quirks.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/pci/quirks.c +3 -0
index 8fe765592943..5a8ea5fdeae7 100644
@@ -2977,6 +2977,24 @@
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0x10a1,
 			quirk_msi_intx_disable_qca_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, 0xe091,
 			quirk_msi_intx_disable_qca_bug);
+
+/*
+ * Amazon's Annapurna Labs 1c36:0031 Root Ports don't support MSI-

X, so it
+ * should be disabled on platforms where the device (mistakenly) advertises it.
+ *
+ * Notice that this quirk also disables MSI (which may work, but hasn't been
+ * tested), since currently there is no standard way to disable only MSI-X.
+ *
+ * The 0031 device id is reused for other non Root Port device types,
+ * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class.
+ */

drivers/pci/quirks.c +6 -0
+static void quirk_al_msi_disable(struct pci_dev *dev)
+{
+	dev->no_msi = 1;
+	pci_warn(dev, "Disabling MSI/MSI-X\n");
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS,

0x0031,

drivers/pci/quirks.c +1 -0
+			      PCI_CLASS_BRIDGE_PCI, 8,

quirk_al_msi_disable);
#endif /* CONFIG_PCI_MSI */

/*

Signature · 1 line
2.17.1
author maintainer
#13 lore raw

PATCH v5 5/7 dt-bindings: PCI: Add Amazon's Annapurna Labs PCIe host bridge binding

In reply to Jonathan Chocron

Document Amazon's Annapurna Labs PCIe host bridge.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
Reviewed-by Rob Herring <[email protected]>
---
 .../devicetree/bindings/pci/pcie-al.txt       | 46 +++++++++++++++++++
 MAINTAINERS                                   |  3 +-
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-al.txt
Documentation/devicetree/bindings/pci/pcie-al.txt new file +46 -0
new file mode 100644
index 000000000000..557a5089229d
@@ -0,0 +1,46 @@
+* Amazon Annapurna Labs PCIe host bridge
+
+Amazon's Annapurna Labs PCIe Host Controller is based on the Synopsys DesignWare
+PCI core. It inherits common properties defined in
+Documentation/devicetree/bindings/pci/designware-pcie.txt.
+
+Properties of the host controller node that differ from it are:
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Value should contain
+			- "amazon,al-alpine-v2-pcie" for alpine_v2
+			- "amazon,al-alpine-v3-pcie" for alpine_v3
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Register ranges as listed in the reg-names property
+
+- reg-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Must include the following entries
+			- "config"	PCIe ECAM space
+			- "controller"	AL proprietary registers
+			- "dbi"		Designware PCIe registers
+
+Example:
+
+	pcie-external0: pcie@fb600000 {
+		compatible = "amazon,al-alpine-v3-pcie";
+		reg = <0x0 0xfb600000 0x0 0x00100000
+		       0x0 0xfd800000 0x0 0x00010000
+		       0x0 0xfd810000 0x0 0x00001000>;
+		reg-names = "config", "controller", "dbi";
+		bus-range = <0 255>;
+		device_type = "pci";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		#interrupt-cells = <1>;
+		interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-map-mask = <0x00 0 0 7>;
+		interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>; /* INTa */
+		ranges = <0x02000000 0x0 0xc0010000 0x0 0xc0010000 0x0 0x07ff0000>;
+	};
MAINTAINERS +2 -1
index 783569e3c4b4..d200b16fa95c 100644
@@ -12448,10 +12448,11 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/
 S:	Supported
 F:	drivers/pci/controller/
 
-PCIE DRIVER FOR ANNAPURNA LABS
+PCIE DRIVER FOR AMAZON ANNAPURNA LABS
 M:	Jonathan Chocron <[email protected]>
 L:	[email protected]
 S:	Maintained
+F:	Documentation/devicetree/bindings/pci/pcie-al.txt
 F:	drivers/pci/controller/dwc/pcie-al.c
 
 PCIE DRIVER FOR AMLOGIC MESON
Signature · 1 line
2.17.1
author maintainer
#14 lore raw

PATCH v5 6/7 PCI: dwc: al: Add support for DW based driver type

In reply to Jonathan Chocron

This driver is DT based and utilizes the DesignWare APIs.

It allows using a smaller ECAM range for a larger bus range - usually an entire bus uses 1MB of address space, but the driver can use it for a larger number of buses. This is achieved by using a HW mechanism which allows changing the BUS part of the "final" outgoing config transaction. There are 2 HW regs, one which is basically a bitmask determining which bits to take from the AXI transaction itself and another which holds the complementary part programmed by the driver.

All link initializations are handled by the boot FW.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/controller/dwc/Kconfig   |  12 +
 drivers/pci/controller/dwc/pcie-al.c | 365 +++++++++++++++++++++++++++
 2 files changed, 377 insertions(+)
drivers/pci/controller/dwc/Kconfig +12 -0
index 4fada2e93285..0ba988b5b5bc 100644
@@ -256,4 +256,16 @@ config PCIE_UNIPHIER
 	  Say Y here if you want PCIe controller support on UniPhier SoCs.
 	  This driver supports LD20 and PXs3 SoCs.
 
+config PCIE_AL
+	bool "Amazon Annapurna Labs PCIe controller"
+	depends on OF && (ARM64 || COMPILE_TEST)
+	depends on PCI_MSI_IRQ_DOMAIN
+	select PCIE_DW_HOST
+	help
+	  Say Y here to enable support of the Amazon's Annapurna Labs PCIe
+	  controller IP on Amazon SoCs. The PCIe controller uses the DesignWare
+	  core plus Annapurna Labs proprietary hardware wrappers. This is
+	  required only for DT-based platforms. ACPI platforms with the
+	  Annapurna Labs PCIe controller don't need to enable this.
+
 endmenu
drivers/pci/controller/dwc/pcie-al.c +365 -0

368 lines not on this page · raw message

Signature · 1 line
2.17.1
#15 lore raw

Re: [PATCH v5 6/7] PCI: dwc: al: Add support for DW based driver type

s/Add support for DW based driver type/Add Amazon Annapurna Labs PCIe controller driver/

Quoted 30 lines · Jonathan Chocron

This driver is DT based and utilizes the DesignWare APIs.

It allows using a smaller ECAM range for a larger bus range - usually an entire bus uses 1MB of address space, but the driver can use it for a larger number of buses. This is achieved by using a HW mechanism which allows changing the BUS part of the "final" outgoing config transaction. There are 2 HW regs, one which is basically a bitmask determining which bits to take from the AXI transaction itself and another which holds the complementary part programmed by the driver.

All link initializations are handled by the boot FW.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/controller/dwc/Kconfig   |  12 +
 drivers/pci/controller/dwc/pcie-al.c | 365 +++++++++++++++++++++++++++
 2 files changed, 377 insertions(+)
drivers/pci/controller/dwc/Kconfig +1 -0
index 4fada2e93285..0ba988b5b5bc 100644
@@ -256,4 +256,16 @@ config PCIE_UNIPHIER
 	  Say Y here if you want PCIe controller support on UniPhier SoCs.
 	  This driver supports LD20 and PXs3 SoCs.
 
+config PCIE_AL

drivers/pci/controller/dwc/Kconfig config PCIE_UNIPHIER · line 269

Quoted 10 lines
drivers/pci/controller/dwc/Kconfig +10 -0
+	bool "Amazon Annapurna Labs PCIe controller"
+	depends on OF && (ARM64 || COMPILE_TEST)
+	depends on PCI_MSI_IRQ_DOMAIN
+	select PCIE_DW_HOST
+	help
+	  Say Y here to enable support of the Amazon's Annapurna Labs PCIe
+	  controller IP on Amazon SoCs. The PCIe controller uses the DesignWare
+	  core plus Annapurna Labs proprietary hardware wrappers. This is
+	  required only for DT-based platforms. ACPI platforms with the
+	  Annapurna Labs PCIe controller don't need to enable this.

Interesting. How do you deal with the funky ECAM space on ACPI platforms? Oh, never mind, I see, it's the pcie-al.c ECAM ops quirk that's already in the tree.

Bjorn

author maintainer
#16 lore raw

Re: [PATCH v5 6/7] PCI: dwc: al: Add support for DW based driver type

Quoted 3 lines · Bjorn Helgaas

s/Add support for DW based driver type/Add Amazon Annapurna Labs PCIe controller driver/

Ack.

Quoted 56 lines · Jonathan Chocron
Quoted 47 lines

This driver is DT based and utilizes the DesignWare APIs.

It allows using a smaller ECAM range for a larger bus range - usually an entire bus uses 1MB of address space, but the driver can use it for a larger number of buses. This is achieved by using a HW
mechanism which allows changing the BUS part of the "final" outgoing
config transaction. There are 2 HW regs, one which is basically a bitmask determining which bits to take from the AXI transaction itself
and another which holds the complementary part programmed by the driver.

All link initializations are handled by the boot FW.

Signed-off-by Jonathan Chocron <[email protected]>
Reviewed-by Gustavo Pimentel <[email protected]>
Reviewed-by Andrew Murray <[email protected]>
---
 drivers/pci/controller/dwc/Kconfig   |  12 +
 drivers/pci/controller/dwc/pcie-al.c | 365
+++++++++++++++++++++++++++
 2 files changed, 377 insertions(+)
drivers/pci/controller/dwc/Kconfig +0 -0
index 4fada2e93285..0ba988b5b5bc 100644
@@ -256,4 +256,16 @@ config PCIE_UNIPHIER
 	  Say Y here if you want PCIe controller support on UniPhier

SoCs.

drivers/pci/controller/dwc/Kconfig +7 -0
 	  This driver supports LD20 and PXs3 SoCs.
 
+config PCIE_AL
+	bool "Amazon Annapurna Labs PCIe controller"
+	depends on OF && (ARM64 || COMPILE_TEST)
+	depends on PCI_MSI_IRQ_DOMAIN
+	select PCIE_DW_HOST
+	help
+	  Say Y here to enable support of the Amazon's Annapurna Labs

PCIe

drivers/pci/controller/dwc/Kconfig +1 -0
+	  controller IP on Amazon SoCs. The PCIe controller uses the

DesignWare

drivers/pci/controller/dwc/Kconfig +1 -0
+	  core plus Annapurna Labs proprietary hardware wrappers. This

is

drivers/pci/controller/dwc/Kconfig config PCIE_UNIPHIER · line 269

Quoted 2 lines
drivers/pci/controller/dwc/Kconfig +2 -0
+	  required only for DT-based platforms. ACPI platforms with the
+	  Annapurna Labs PCIe controller don't need to enable this.

Interesting. How do you deal with the funky ECAM space on ACPI platforms? Oh, never mind, I see, it's the pcie-al.c ECAM ops quirk that's already in the tree.

Bjorn

author maintainer
#17 lore raw

PATCH v5 7/7 PCI: dwc: Add validation that PCIe core is set to correct mode

In reply to Jonathan Chocron

Some PCIe controllers can be set to either Host or EP according to some early boot FW. To make sure there is no discrepancy (e.g. FW configured the port to EP mode while the DT specifies it as a host bridge or vice versa), a check has been added for each mode.

Signed-off-by Jonathan Chocron <[email protected]>
Acked-by Gustavo Pimentel <[email protected]>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c  |  8 ++++++++
 .../pci/controller/dwc/pcie-designware-host.c    | 16 ++++++++++++++++
 2 files changed, 24 insertions(+)
drivers/pci/controller/dwc/pcie-designware-ep.c +8 -0
index 65f479250087..3dd2e2697294 100644
@@ -498,6 +498,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	int ret;
 	u32 reg;
 	void *addr;
+	u8 hdr_type;
 	unsigned int nbars;
 	unsigned int offset;
 	struct pci_epc *epc;

@@ -562,6 +563,13 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	if (ep->ops->ep_init)
 		ep->ops->ep_init(ep);
 
+	hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
+	if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
+		dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
+			hdr_type);
+		return -EIO;
+	}
+
 	ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
 	if (ret < 0)
 		epc->max_functions = 1;
drivers/pci/controller/dwc/pcie-designware-host.c +16 -0
index d3156446ff27..0f36a926059a 100644
@@ -323,6 +323,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	struct pci_bus *child;
 	struct pci_host_bridge *bridge;
 	struct resource *cfg_res;
+	u32 hdr_type;
 	int ret;
 
 	raw_spin_lock_init(&pci->pp.lock);

@@ -464,6 +465,21 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			goto err_free_msi;
 	}
 
+	ret = dw_pcie_rd_own_conf(pp, PCI_HEADER_TYPE, 1, &hdr_type);
+	if (ret != PCIBIOS_SUCCESSFUL) {
+		dev_err(pci->dev, "Failed reading PCI_HEADER_TYPE cfg space reg (ret: 0x%x)\n",
+			ret);
+		ret = pcibios_err_to_errno(ret);
+		goto err_free_msi;
+	}
+	if (hdr_type != PCI_HEADER_TYPE_BRIDGE) {
+		dev_err(pci->dev,
+			"PCIe controller is not set to bridge type (hdr_type: 0x%x)!\n",
+			hdr_type);
+		ret = -EIO;
+		goto err_free_msi;
+	}
+
 	pp->root_bus_nr = pp->busn->start;
 
 	bridge->dev.parent = dev;
Signature · 1 line
2.17.1
Reviewed-by on v5 7/7
#18 lore raw

Re: [PATCH v5 7/7] PCI: dwc: Add validation that PCIe core is set to correct mode

Quoted 59 lines · Jonathan Chocron

Some PCIe controllers can be set to either Host or EP according to some early boot FW. To make sure there is no discrepancy (e.g. FW configured the port to EP mode while the DT specifies it as a host bridge or vice versa), a check has been added for each mode.

Signed-off-by Jonathan Chocron <[email protected]>
Acked-by Gustavo Pimentel <[email protected]>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c  |  8 ++++++++
 .../pci/controller/dwc/pcie-designware-host.c    | 16 ++++++++++++++++
 2 files changed, 24 insertions(+)
drivers/pci/controller/dwc/pcie-designware-ep.c +8 -0
index 65f479250087..3dd2e2697294 100644
@@ -498,6 +498,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	int ret;
 	u32 reg;
 	void *addr;
+	u8 hdr_type;
 	unsigned int nbars;
 	unsigned int offset;
 	struct pci_epc *epc;

@@ -562,6 +563,13 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	if (ep->ops->ep_init)
 		ep->ops->ep_init(ep);
 
+	hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
+	if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
+		dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
+			hdr_type);
+		return -EIO;
+	}
+
 	ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
 	if (ret < 0)
 		epc->max_functions = 1;
drivers/pci/controller/dwc/pcie-designware-host.c +6 -0
index d3156446ff27..0f36a926059a 100644
@@ -323,6 +323,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	struct pci_bus *child;
 	struct pci_host_bridge *bridge;
 	struct resource *cfg_res;
+	u32 hdr_type;
 	int ret;
 
 	raw_spin_lock_init(&pci->pp.lock);

@@ -464,6 +465,21 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			goto err_free_msi;
 	}
 
+	ret = dw_pcie_rd_own_conf(pp, PCI_HEADER_TYPE, 1, &hdr_type);
+	if (ret != PCIBIOS_SUCCESSFUL) {
+		dev_err(pci->dev, "Failed reading PCI_HEADER_TYPE cfg space reg (ret: 0x%x)\n",
+			ret);
+		ret = pcibios_err_to_errno(ret);

drivers/pci/controller/dwc/pcie-designware-host.c int dw_pcie_host_init(struct pcie_port *pp) · line 482

Quoted 10 lines
drivers/pci/controller/dwc/pcie-designware-host.c +10 -0
+		goto err_free_msi;
+	}
+	if (hdr_type != PCI_HEADER_TYPE_BRIDGE) {
+		dev_err(pci->dev,
+			"PCIe controller is not set to bridge type (hdr_type: 0x%x)!\n",
+			hdr_type);
+		ret = -EIO;
+		goto err_free_msi;
+	}
+
Reviewed-by Andrew Murray <[email protected]>
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware-host.c +0 -0
 	pp->root_bus_nr = pp->busn->start;
 
 	bridge->dev.parent = dev;
Signature · 1 line
2.17.1
#19 lore raw
In reply to Jonathan Chocron
Quoted 64 lines · Jonathan Chocron

This series adds support for Amazon's Annapurna Labs DT-based PCIe host controller driver.
Additionally, it adds 3 quirks (ACS, VPD and MSI-X) and 2 generic DWC patches.

Changes since v4:
- Moved the HEADER_TYPE validations to after pp->ops->host_init() and ep->ops->ep_init()
- Changed to dw_pcie_rd_own_conf() instead of dw_pcie_readb_dbi() for reading the HEADER_TYPE
- Used exsitng quirk_blacklist_vpd() instead of quirk_al_vpd_release()
- Added a newline in ACS quirk comment

Changes since v3:
- Removed PATCH 8/8 since the usage of the PCI flags will be discussed in the upcoming LPC
- Align commit subject with the folder convention
- Added explanation regarding ECAM "overload" mechanism
- Switched to read/write{_relaxed} APIs
- Modified a dev_err to dev_dbg
- Removed unnecessary variable
- Removed driver details from dt-binding description
- Changed to SoC specific compatibles
- Fixed typo in a commit message
- Added comment regarding MSI in the MSI-X quirk

Changes since v2:
- Added al_pcie_controller_readl/writel() wrappers
- Reorganized local vars in several functions according to reverse tree structure
- Removed unnecessary check of ret value
- Changed return type of al_pcie_config_prepare() from int to void
- Removed check if link is up from probe() [done internally in dw_pcie_rd/wr_conf()]

Changes since v1:
- Added comment regarding 0x0031 being used as a dev_id for non root-port devices as well
- Fixed different message/comment/print wordings
- Added panic stacktrace to commit message of MSI-x quirk patch
- Changed to pci_warn() instead of dev_warn()
- Added unit_address after node_name in dt-binding
- Updated Kconfig help description
- Used GENMASK and FIELD_PREP/GET where appropriate
- Removed leftover field from struct al_pcie and moved all ptrs to
the beginning
- Re-wrapped function definitions and invocations to use fewer lines
- Change %p to %px in dbg prints in rd/wr_conf() functions
- Removed validation that the port is configured to RC mode (as this is added generically in PATCH 7/8)
- Removed unnecessary variable initializations
- Swtiched to %pR for printing resources

Ali Saidi (1):
PCI: Add ACS quirk for Amazon Annapurna Labs root ports

Jonathan Chocron (6):
PCI: Add Amazon's Annapurna Labs vendor ID
PCI/VPD: Prevent VPD access for Amazon's Annapurna Labs Root Port
PCI: Add quirk to disable MSI-X support for Amazon's Annapurna Labs
Root Port
dt-bindings: PCI: Add Amazon's Annapurna Labs PCIe host bridge binding
PCI: dwc: al: Add support for DW based driver type
PCI: dwc: Add validation that PCIe core is set to correct mode

Quoted 11 lines
 .../devicetree/bindings/pci/pcie-al.txt       |  46 +++
 MAINTAINERS                                   |   3 +-
 drivers/pci/controller/dwc/Kconfig            |  12 +
 drivers/pci/controller/dwc/pcie-al.c          | 365 ++++++++++++++++++
 .../pci/controller/dwc/pcie-designware-ep.c   |   8 +
 .../pci/controller/dwc/pcie-designware-host.c |  16 +
 drivers/pci/quirks.c                          |  38 ++
 drivers/pci/vpd.c                             |   6 +
 include/linux/pci_ids.h                       |   2 +
 9 files changed, 495 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-al.txt

Hi Bjorn,

I would like to queue this series for v5.4 but I need your ACK on patches 2/3/4 for that to happen, please let me know.

Thanks,
Lorenzo