All PCIe devices are expected to be able to handle 8-bit tags.
'commit 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")'
enabled extended tags for all devices based on the spec direction.
The Broadcom HT2100 seems to be having issues with handling
8-bit tags. Mark it as broken.
If a device with extended tags quirk is found, disable extended tags for
all devices in the tree assuming peer-to-peer is possible.
The pci_walk_bus() in the quirk handles all devices we've already
enumerated, and all devices we'll enumerate in the future are handled in
pci_configure_device().
Reported-by: Wim ten Have <[email protected]>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
Signed-off-by: Sinan Kaya <[email protected]>
---
drivers/pci/pci.h | 1 +
drivers/pci/probe.c | 35 ++++++++++++++++++++++++++++++-----
drivers/pci/quirks.c | 19 +++++++++++++++++++
include/linux/pci.h | 1 +
4 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f8113e5..7153c92 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -239,6 +239,7 @@ enum pci_bar_type {
pci_bar_mem64, /* A 64-bit memory BAR */
};
+int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
int crs_timeout);
int pci_setup_device(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..bc067fb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1684,22 +1684,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
*/
}
-static void pci_configure_extended_tags(struct pci_dev *dev)
+int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
{
+ struct pci_host_bridge *host;
u32 dev_cap;
int ret;
+ u16 ctl;
if (!pci_is_pcie(dev))
- return;
+ return 0;
ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
if (ret)
- return;
+ return 0;
+
+ if (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG))
+ return 0;
+
+ ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
+ if (ret)
+ return 0;
+
+ host = pci_find_host_bridge(dev->bus);
+ if (!host)
+ return 0;
- if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+ if (host->broken_ext_tags) {
+ if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
+ dev_info(&dev->dev, "disabling Extended Tags\n");
+ pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+ PCI_EXP_DEVCTL_EXT_TAG);
+ }
+ return 0;
+ }
+ if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
+ dev_info(&dev->dev, "enabling Extended Tags\n");
pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
PCI_EXP_DEVCTL_EXT_TAG);
+ }
+ return 0;
}
+EXPORT_SYMBOL(pci_configure_extended_tags);
static void pci_configure_device(struct pci_dev *dev)
{
@@ -1707,7 +1732,7 @@ static void pci_configure_device(struct pci_dev *dev)
int ret;
pci_configure_mps(dev);
- pci_configure_extended_tags(dev);
+ pci_configure_extended_tags(dev, NULL);
memset(&hpp, 0, sizeof(hpp));
ret = pci_get_hp_params(dev, &hpp);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 085fb78..b5dc99c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
+
+static void quirk_exttags_completer(struct pci_dev *pdev)
+{
+ struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+
+ if (!bridge)
+ return;
+
+ bridge->broken_ext_tags = 1;
+ dev_info(&pdev->dev, "Extended Tag handling is broken\n");
+
+ pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140,
+ quirk_exttags_completer);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142,
+ quirk_exttags_completer);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144,
+ quirk_exttags_completer);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8039f9f..2b22c3e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -441,6 +441,7 @@ struct pci_host_bridge {
void *release_data;
struct msi_controller *msi;
unsigned int ignore_reset_delay:1; /* for entire hierarchy */
+ unsigned int broken_ext_tags:1;
/* Resource alignment requirements */
resource_size_t (*align_resource)(struct pci_dev *dev,
const struct resource *res,
--
1.9.1
On Wed, 12 Jul 2017 00:04:14 -0400
Sinan Kaya <[email protected]> wrote:
> All PCIe devices are expected to be able to handle 8-bit tags.
> 'commit 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")'
> enabled extended tags for all devices based on the spec direction.
>
> The Broadcom HT2100 seems to be having issues with handling
> 8-bit tags. Mark it as broken.
>
> If a device with extended tags quirk is found, disable extended tags for
> all devices in the tree assuming peer-to-peer is possible.
>
> The pci_walk_bus() in the quirk handles all devices we've already
> enumerated, and all devices we'll enumerate in the future are handled in
> pci_configure_device().
>
> Reported-by: Wim ten Have <[email protected]>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <[email protected]>
Problem seems fixed with this one.
Tested-by: Wim ten Have <[email protected]>
[ 0.000000] Linux version 4.12.0-rc5+ (wtenhave@hagen) (gcc version 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC) ) #1 SMP Wed Jul 12 09:59:10 CEST 2017
[ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.12.0-rc5+ root=/dev/mapper/fedora_hagen-root ro rd.lvm.lv=fedora_hagen/root rd.lvm.lv=fedora_hagen/swap audit=0
...
[ 0.000000] SMBIOS 2.4 present.
[ 0.000000] DMI: Dell Inc. PowerEdge SC1435/0H313M, BIOS 2.2.5 03/21/2008
[ 0.000000] tsc: Fast TSC calibration using PIT
...
[ 0.517737] PCI host bridge to bus 0000:00
[ 0.517937] pci_bus 0000:00: root bus resource [io 0x0000-0x0cf7 window]
[ 0.518131] pci_bus 0000:00: root bus resource [io 0xd000-0xefff window]
[ 0.518326] pci_bus 0000:00: root bus resource [io 0x0d00-0x0fff window]
[ 0.518521] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window]
[ 0.518856] pci_bus 0000:00: root bus resource [mem 0xf0000000-0xf1ffffff window]
[ 0.519192] pci_bus 0000:00: root bus resource [mem 0xefb00000-0xefffffff window]
[ 0.519522] pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff window]
[ 0.519859] pci_bus 0000:00: root bus resource [bus 00-fd]
[ 0.520067] pci 0000:00:01.0: [1166:0036] type 01 class 0x060400
[ 0.520101] pci 0000:00:01.0: Enabling HT MSI Mapping
[ 0.520350] pci 0000:00:01.0: System wakeup disabled by ACPI
[ 0.520583] pci 0000:00:02.0: [1166:0205] type 00 class 0x060000
[ 0.520676] pci 0000:00:02.1: [1166:0214] type 00 class 0x01018a
[ 0.520691] pci 0000:00:02.1: reg 0x10: [io 0x01f0-0x01f7]
[ 0.520700] pci 0000:00:02.1: reg 0x14: [io 0x03f4-0x03f7]
[ 0.520708] pci 0000:00:02.1: reg 0x18: [io 0x0170-0x0177]
[ 0.520716] pci 0000:00:02.1: reg 0x1c: [io 0x0374-0x0377]
[ 0.520725] pci 0000:00:02.1: reg 0x20: [io 0x08c0-0x08cf]
[ 0.520743] pci 0000:00:02.1: legacy IDE quirk: reg 0x10: [io 0x01f0-0x01f7]
[ 0.520947] pci 0000:00:02.1: legacy IDE quirk: reg 0x14: [io 0x03f6]
[ 0.521140] pci 0000:00:02.1: legacy IDE quirk: reg 0x18: [io 0x0170-0x0177]
[ 0.521335] pci 0000:00:02.1: legacy IDE quirk: reg 0x1c: [io 0x0376]
[ 0.521596] pci 0000:00:02.2: [1166:0234] type 00 class 0x060100
[ 0.521712] pci 0000:00:03.0: [1166:0223] type 00 class 0x0c0310
[ 0.521727] pci 0000:00:03.0: reg 0x10: [mem 0xefbed000-0xefbedfff]
[ 0.521736] pci 0000:00:03.0: reg 0x14: [io 0xd000-0xd0ff]
[ 0.521853] pci 0000:00:03.1: [1166:0223] type 00 class 0x0c0310
[ 0.521868] pci 0000:00:03.1: reg 0x10: [mem 0xefbee000-0xefbeefff]
[ 0.521877] pci 0000:00:03.1: reg 0x14: [io 0xd400-0xd4ff]
[ 0.521989] pci 0000:00:03.2: [1166:0223] type 00 class 0x0c0320
[ 0.522005] pci 0000:00:03.2: reg 0x10: [mem 0xefbef000-0xefbeffff]
[ 0.522013] pci 0000:00:03.2: reg 0x14: [io 0xd800-0xd8ff]
[ 0.522078] pci 0000:00:03.2: supports D1 D2
[ 0.522079] pci 0000:00:03.2: PME# supported from D0 D1 D2 D3hot
[ 0.522139] pci 0000:00:04.0: [1002:515e] type 00 class 0x030000
[ 0.522154] pci 0000:00:04.0: reg 0x10: [mem 0xe0000000-0xe7ffffff pref]
[ 0.522163] pci 0000:00:04.0: reg 0x14: [io 0xdc00-0xdcff]
[ 0.522172] pci 0000:00:04.0: reg 0x18: [mem 0xefbf0000-0xefbfffff]
[ 0.522204] pci 0000:00:04.0: reg 0x30: [mem 0x00000000-0x0001ffff pref]
[ 0.522231] pci 0000:00:04.0: supports D1 D2
[ 0.522276] pci 0000:00:07.0: [1166:0140] type 01 class 0x060400
[ 0.522280] pci 0000:00:07.0: Extended Tag handling is broken
[ 0.522508] pci 0000:00:07.0: PME# supported from D0 D3hot D3cold
[ 0.522564] pci 0000:00:08.0: [1166:0142] type 01 class 0x060400
[ 0.522567] pci 0000:00:08.0: Extended Tag handling is broken
[ 0.522797] pci 0000:00:08.0: PME# supported from D0 D3hot D3cold
[ 0.522824] pci 0000:00:08.0: System wakeup disabled by ACPI
[ 0.523058] pci 0000:00:09.0: [1166:0144] type 01 class 0x060400
[ 0.523061] pci 0000:00:09.0: Extended Tag handling is broken
[ 0.523288] pci 0000:00:09.0: PME# supported from D0 D3hot D3cold
[ 0.523316] pci 0000:00:09.0: System wakeup disabled by ACPI
[ 0.523541] pci 0000:00:0a.0: [1166:0142] type 01 class 0x060400
[ 0.523544] pci 0000:00:0a.0: Extended Tag handling is broken
[ 0.523773] pci 0000:00:0a.0: PME# supported from D0 D3hot D3cold
[ 0.523799] pci 0000:00:0a.0: System wakeup disabled by ACPI
[ 0.524032] pci 0000:00:0b.0: [1166:0144] type 01 class 0x060400
[ 0.524035] pci 0000:00:0b.0: Extended Tag handling is broken
[ 0.524261] pci 0000:00:0b.0: PME# supported from D0 D3hot D3cold
[ 0.524330] pci 0000:00:18.0: [1022:1200] type 00 class 0x060000
[ 0.524389] pci 0000:00:18.1: [1022:1201] type 00 class 0x060000
[ 0.524443] pci 0000:00:18.2: [1022:1202] type 00 class 0x060000
[ 0.524492] pci 0000:00:18.3: [1022:1203] type 00 class 0x060000
[ 0.524543] pci 0000:00:18.4: [1022:1204] type 00 class 0x060000
[ 0.524594] pci 0000:00:19.0: [1022:1200] type 00 class 0x060000
[ 0.524650] pci 0000:00:19.1: [1022:1201] type 00 class 0x060000
[ 0.524701] pci 0000:00:19.2: [1022:1202] type 00 class 0x060000
[ 0.524752] pci 0000:00:19.3: [1022:1203] type 00 class 0x060000
[ 0.524806] pci 0000:00:19.4: [1022:1204] type 00 class 0x060000
[ 0.524944] pci 0000:03:0d.0: [1166:0104] type 01 class 0x060400
[ 0.525022] pci 0000:03:0e.0: [1166:024b] type 00 class 0x01018f
[ 0.525033] pci 0000:03:0e.0: reg 0x10: [io 0xecb0-0xecb7]
[ 0.525039] pci 0000:03:0e.0: reg 0x14: [io 0xeca0-0xeca3]
[ 0.525045] pci 0000:03:0e.0: reg 0x18: [io 0xecb8-0xecbf]
[ 0.525051] pci 0000:03:0e.0: reg 0x1c: [io 0xeca4-0xeca7]
[ 0.525056] pci 0000:03:0e.0: reg 0x20: [io 0xece0-0xecef]
[ 0.525062] pci 0000:03:0e.0: reg 0x24: [mem 0xefdfe000-0xefdfffff]
[ 0.525068] pci 0000:03:0e.0: reg 0x30: [mem 0x00000000-0x0001ffff pref]
[ 0.525135] pci 0000:00:01.0: PCI bridge to [bus 03-04]
[ 0.525330] pci 0000:00:01.0: bridge window [io 0xe000-0xefff]
[ 0.525332] pci 0000:00:01.0: bridge window [mem 0xefc00000-0xefdfffff]
[ 0.525391] pci 0000:03:0d.0: PCI bridge to [bus 04]
[ 0.525620] pci 0000:00:07.0: PCI bridge to [bus 05]
[ 0.525862] pci 0000:01:00.0: [14e4:1659] type 00 class 0x020000
[ 0.525885] pci 0000:01:00.0: reg 0x10: [mem 0xefef0000-0xefefffff 64bit]
[ 0.525985] pci 0000:01:00.0: PME# supported from D3hot D3cold
[ 0.528889] pci 0000:00:08.0: PCI bridge to [bus 01]
[ 0.529097] pci 0000:00:08.0: bridge window [mem 0xefe00000-0xefefffff]
[ 0.529153] pci 0000:02:00.0: [14e4:1659] type 00 class 0x020000
[ 0.529176] pci 0000:02:00.0: reg 0x10: [mem 0xefff0000-0xefffffff 64bit]
[ 0.529281] pci 0000:02:00.0: PME# supported from D3hot D3cold
[ 0.531883] pci 0000:00:09.0: PCI bridge to [bus 02]
[ 0.532084] pci 0000:00:09.0: bridge window [mem 0xeff00000-0xefffffff]
[ 0.532129] pci 0000:00:0a.0: PCI bridge to [bus 06]
[ 0.532350] pci 0000:00:0b.0: PCI bridge to [bus 07]
...
[ 3.714744] tg3 0000:01:00.0 eth0: Tigon3 [partno(BCM95721) rev 4201] (PCI Express) MAC address 00:22:19:27:cd:f8
[ 3.714746] tg3 0000:01:00.0 eth0: attached PHY is 5750 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
[ 3.714748] tg3 0000:01:00.0 eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1]
[ 3.714750] tg3 0000:01:00.0 eth0: dma_rwctrl[76180000] dma_mask[64-bit]
...
[ 4.034593] tg3 0000:02:00.0 eth1: Tigon3 [partno(BCM95721) rev 4201] (PCI Express) MAC address 00:22:19:27:cd:f9
[ 4.034747] tg3 0000:02:00.0 eth1: attached PHY is 5750 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
[ 4.034896] tg3 0000:02:00.0 eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1]
[ 4.035030] tg3 0000:02:00.0 eth1: dma_rwctrl[76180000] dma_mask[64-bit]
> ---
> drivers/pci/pci.h | 1 +
> drivers/pci/probe.c | 35 ++++++++++++++++++++++++++++++-----
> drivers/pci/quirks.c | 19 +++++++++++++++++++
> include/linux/pci.h | 1 +
> 4 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index f8113e5..7153c92 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -239,6 +239,7 @@ enum pci_bar_type {
> pci_bar_mem64, /* A 64-bit memory BAR */
> };
>
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
> bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
> int crs_timeout);
> int pci_setup_device(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..bc067fb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,22 +1684,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
> */
> }
>
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
> {
> + struct pci_host_bridge *host;
> u32 dev_cap;
> int ret;
> + u16 ctl;
>
> if (!pci_is_pcie(dev))
> - return;
> + return 0;
>
> ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> if (ret)
> - return;
> + return 0;
> +
> + if (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG))
> + return 0;
> +
> + ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
> + if (ret)
> + return 0;
> +
> + host = pci_find_host_bridge(dev->bus);
> + if (!host)
> + return 0;
>
> - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> + if (host->broken_ext_tags) {
> + if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
> + dev_info(&dev->dev, "disabling Extended Tags\n");
> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> + }
> + if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
> + dev_info(&dev->dev, "enabling Extended Tags\n");
> pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> }
> +EXPORT_SYMBOL(pci_configure_extended_tags);
>
> static void pci_configure_device(struct pci_dev *dev)
> {
> @@ -1707,7 +1732,7 @@ static void pci_configure_device(struct pci_dev *dev)
> int ret;
>
> pci_configure_mps(dev);
> - pci_configure_extended_tags(dev);
> + pci_configure_extended_tags(dev, NULL);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 085fb78..b5dc99c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
> +
> +static void quirk_exttags_completer(struct pci_dev *pdev)
> +{
> + struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +
> + if (!bridge)
> + return;
> +
> + bridge->broken_ext_tags = 1;
> + dev_info(&pdev->dev, "Extended Tag handling is broken\n");
> +
> + pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140,
> + quirk_exttags_completer);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142,
> + quirk_exttags_completer);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144,
> + quirk_exttags_completer);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8039f9f..2b22c3e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -441,6 +441,7 @@ struct pci_host_bridge {
> void *release_data;
> struct msi_controller *msi;
> unsigned int ignore_reset_delay:1; /* for entire hierarchy */
> + unsigned int broken_ext_tags:1;
> /* Resource alignment requirements */
> resource_size_t (*align_resource)(struct pci_dev *dev,
> const struct resource *res,
Hi,
On Wed, Jul 12, 2017 at 4:55 PM, Wim ten Have <[email protected]> wrote:
> On Wed, 12 Jul 2017 00:04:14 -0400
> Sinan Kaya <[email protected]> wrote:
>
>> All PCIe devices are expected to be able to handle 8-bit tags.
>> 'commit 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")'
>> enabled extended tags for all devices based on the spec direction.
>>
>> The Broadcom HT2100 seems to be having issues with handling
>> 8-bit tags. Mark it as broken.
>>
>> If a device with extended tags quirk is found, disable extended tags for
>> all devices in the tree assuming peer-to-peer is possible.
>>
>> The pci_walk_bus() in the quirk handles all devices we've already
>> enumerated, and all devices we'll enumerate in the future are handled in
>> pci_configure_device().
>>
>> Reported-by: Wim ten Have <[email protected]>
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
>> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
>> Signed-off-by: Sinan Kaya <[email protected]>
>
> Problem seems fixed with this one.
>
> Tested-by: Wim ten Have <[email protected]>
>
> [ 0.000000] Linux version 4.12.0-rc5+ (wtenhave@hagen) (gcc version 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC) ) #1 SMP Wed Jul 12 09:59:10 CEST 2017
> [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.12.0-rc5+ root=/dev/mapper/fedora_hagen-root ro rd.lvm.lv=fedora_hagen/root rd.lvm.lv=fedora_hagen/swap audit=0
> ...
> [ 0.000000] SMBIOS 2.4 present.
> [ 0.000000] DMI: Dell Inc. PowerEdge SC1435/0H313M, BIOS 2.2.5 03/21/2008
> [ 0.000000] tsc: Fast TSC calibration using PIT
> ...
> [ 0.517737] PCI host bridge to bus 0000:00
> [ 0.517937] pci_bus 0000:00: root bus resource [io 0x0000-0x0cf7 window]
> [ 0.518131] pci_bus 0000:00: root bus resource [io 0xd000-0xefff window]
> [ 0.518326] pci_bus 0000:00: root bus resource [io 0x0d00-0x0fff window]
> [ 0.518521] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window]
> [ 0.518856] pci_bus 0000:00: root bus resource [mem 0xf0000000-0xf1ffffff window]
> [ 0.519192] pci_bus 0000:00: root bus resource [mem 0xefb00000-0xefffffff window]
> [ 0.519522] pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff window]
> [ 0.519859] pci_bus 0000:00: root bus resource [bus 00-fd]
> [ 0.520067] pci 0000:00:01.0: [1166:0036] type 01 class 0x060400
> [ 0.520101] pci 0000:00:01.0: Enabling HT MSI Mapping
> [ 0.520350] pci 0000:00:01.0: System wakeup disabled by ACPI
> [ 0.520583] pci 0000:00:02.0: [1166:0205] type 00 class 0x060000
> [ 0.520676] pci 0000:00:02.1: [1166:0214] type 00 class 0x01018a
> [ 0.520691] pci 0000:00:02.1: reg 0x10: [io 0x01f0-0x01f7]
> [ 0.520700] pci 0000:00:02.1: reg 0x14: [io 0x03f4-0x03f7]
> [ 0.520708] pci 0000:00:02.1: reg 0x18: [io 0x0170-0x0177]
> [ 0.520716] pci 0000:00:02.1: reg 0x1c: [io 0x0374-0x0377]
> [ 0.520725] pci 0000:00:02.1: reg 0x20: [io 0x08c0-0x08cf]
> [ 0.520743] pci 0000:00:02.1: legacy IDE quirk: reg 0x10: [io 0x01f0-0x01f7]
> [ 0.520947] pci 0000:00:02.1: legacy IDE quirk: reg 0x14: [io 0x03f6]
> [ 0.521140] pci 0000:00:02.1: legacy IDE quirk: reg 0x18: [io 0x0170-0x0177]
> [ 0.521335] pci 0000:00:02.1: legacy IDE quirk: reg 0x1c: [io 0x0376]
> [ 0.521596] pci 0000:00:02.2: [1166:0234] type 00 class 0x060100
> [ 0.521712] pci 0000:00:03.0: [1166:0223] type 00 class 0x0c0310
> [ 0.521727] pci 0000:00:03.0: reg 0x10: [mem 0xefbed000-0xefbedfff]
> [ 0.521736] pci 0000:00:03.0: reg 0x14: [io 0xd000-0xd0ff]
> [ 0.521853] pci 0000:00:03.1: [1166:0223] type 00 class 0x0c0310
> [ 0.521868] pci 0000:00:03.1: reg 0x10: [mem 0xefbee000-0xefbeefff]
> [ 0.521877] pci 0000:00:03.1: reg 0x14: [io 0xd400-0xd4ff]
> [ 0.521989] pci 0000:00:03.2: [1166:0223] type 00 class 0x0c0320
> [ 0.522005] pci 0000:00:03.2: reg 0x10: [mem 0xefbef000-0xefbeffff]
> [ 0.522013] pci 0000:00:03.2: reg 0x14: [io 0xd800-0xd8ff]
> [ 0.522078] pci 0000:00:03.2: supports D1 D2
> [ 0.522079] pci 0000:00:03.2: PME# supported from D0 D1 D2 D3hot
> [ 0.522139] pci 0000:00:04.0: [1002:515e] type 00 class 0x030000
> [ 0.522154] pci 0000:00:04.0: reg 0x10: [mem 0xe0000000-0xe7ffffff pref]
> [ 0.522163] pci 0000:00:04.0: reg 0x14: [io 0xdc00-0xdcff]
> [ 0.522172] pci 0000:00:04.0: reg 0x18: [mem 0xefbf0000-0xefbfffff]
> [ 0.522204] pci 0000:00:04.0: reg 0x30: [mem 0x00000000-0x0001ffff pref]
> [ 0.522231] pci 0000:00:04.0: supports D1 D2
> [ 0.522276] pci 0000:00:07.0: [1166:0140] type 01 class 0x060400
> [ 0.522280] pci 0000:00:07.0: Extended Tag handling is broken
The dmesg yelled "...Tag handling is broken" , but didn't say how it
was handled, that is weird and confusing, just because there is no
device under that bridge ?
How about move the messages together "...is broken, disable Extended tag..."
Thanks,
Ethan
> [ 0.522508] pci 0000:00:07.0: PME# supported from D0 D3hot D3cold
> [ 0.522564] pci 0000:00:08.0: [1166:0142] type 01 class 0x060400
> [ 0.522567] pci 0000:00:08.0: Extended Tag handling is broken
> [ 0.522797] pci 0000:00:08.0: PME# supported from D0 D3hot D3cold
> [ 0.522824] pci 0000:00:08.0: System wakeup disabled by ACPI
> [ 0.523058] pci 0000:00:09.0: [1166:0144] type 01 class 0x060400
> [ 0.523061] pci 0000:00:09.0: Extended Tag handling is broken
> [ 0.523288] pci 0000:00:09.0: PME# supported from D0 D3hot D3cold
> [ 0.523316] pci 0000:00:09.0: System wakeup disabled by ACPI
> [ 0.523541] pci 0000:00:0a.0: [1166:0142] type 01 class 0x060400
> [ 0.523544] pci 0000:00:0a.0: Extended Tag handling is broken
> [ 0.523773] pci 0000:00:0a.0: PME# supported from D0 D3hot D3cold
> [ 0.523799] pci 0000:00:0a.0: System wakeup disabled by ACPI
> [ 0.524032] pci 0000:00:0b.0: [1166:0144] type 01 class 0x060400
> [ 0.524035] pci 0000:00:0b.0: Extended Tag handling is broken
> [ 0.524261] pci 0000:00:0b.0: PME# supported from D0 D3hot D3cold
> [ 0.524330] pci 0000:00:18.0: [1022:1200] type 00 class 0x060000
> [ 0.524389] pci 0000:00:18.1: [1022:1201] type 00 class 0x060000
> [ 0.524443] pci 0000:00:18.2: [1022:1202] type 00 class 0x060000
> [ 0.524492] pci 0000:00:18.3: [1022:1203] type 00 class 0x060000
> [ 0.524543] pci 0000:00:18.4: [1022:1204] type 00 class 0x060000
> [ 0.524594] pci 0000:00:19.0: [1022:1200] type 00 class 0x060000
> [ 0.524650] pci 0000:00:19.1: [1022:1201] type 00 class 0x060000
> [ 0.524701] pci 0000:00:19.2: [1022:1202] type 00 class 0x060000
> [ 0.524752] pci 0000:00:19.3: [1022:1203] type 00 class 0x060000
> [ 0.524806] pci 0000:00:19.4: [1022:1204] type 00 class 0x060000
> [ 0.524944] pci 0000:03:0d.0: [1166:0104] type 01 class 0x060400
> [ 0.525022] pci 0000:03:0e.0: [1166:024b] type 00 class 0x01018f
> [ 0.525033] pci 0000:03:0e.0: reg 0x10: [io 0xecb0-0xecb7]
> [ 0.525039] pci 0000:03:0e.0: reg 0x14: [io 0xeca0-0xeca3]
> [ 0.525045] pci 0000:03:0e.0: reg 0x18: [io 0xecb8-0xecbf]
> [ 0.525051] pci 0000:03:0e.0: reg 0x1c: [io 0xeca4-0xeca7]
> [ 0.525056] pci 0000:03:0e.0: reg 0x20: [io 0xece0-0xecef]
> [ 0.525062] pci 0000:03:0e.0: reg 0x24: [mem 0xefdfe000-0xefdfffff]
> [ 0.525068] pci 0000:03:0e.0: reg 0x30: [mem 0x00000000-0x0001ffff pref]
> [ 0.525135] pci 0000:00:01.0: PCI bridge to [bus 03-04]
> [ 0.525330] pci 0000:00:01.0: bridge window [io 0xe000-0xefff]
> [ 0.525332] pci 0000:00:01.0: bridge window [mem 0xefc00000-0xefdfffff]
> [ 0.525391] pci 0000:03:0d.0: PCI bridge to [bus 04]
> [ 0.525620] pci 0000:00:07.0: PCI bridge to [bus 05]
> [ 0.525862] pci 0000:01:00.0: [14e4:1659] type 00 class 0x020000
> [ 0.525885] pci 0000:01:00.0: reg 0x10: [mem 0xefef0000-0xefefffff 64bit]
> [ 0.525985] pci 0000:01:00.0: PME# supported from D3hot D3cold
> [ 0.528889] pci 0000:00:08.0: PCI bridge to [bus 01]
> [ 0.529097] pci 0000:00:08.0: bridge window [mem 0xefe00000-0xefefffff]
> [ 0.529153] pci 0000:02:00.0: [14e4:1659] type 00 class 0x020000
> [ 0.529176] pci 0000:02:00.0: reg 0x10: [mem 0xefff0000-0xefffffff 64bit]
> [ 0.529281] pci 0000:02:00.0: PME# supported from D3hot D3cold
> [ 0.531883] pci 0000:00:09.0: PCI bridge to [bus 02]
> [ 0.532084] pci 0000:00:09.0: bridge window [mem 0xeff00000-0xefffffff]
> [ 0.532129] pci 0000:00:0a.0: PCI bridge to [bus 06]
> [ 0.532350] pci 0000:00:0b.0: PCI bridge to [bus 07]
> ...
> [ 3.714744] tg3 0000:01:00.0 eth0: Tigon3 [partno(BCM95721) rev 4201] (PCI Express) MAC address 00:22:19:27:cd:f8
> [ 3.714746] tg3 0000:01:00.0 eth0: attached PHY is 5750 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
> [ 3.714748] tg3 0000:01:00.0 eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1]
> [ 3.714750] tg3 0000:01:00.0 eth0: dma_rwctrl[76180000] dma_mask[64-bit]
> ...
> [ 4.034593] tg3 0000:02:00.0 eth1: Tigon3 [partno(BCM95721) rev 4201] (PCI Express) MAC address 00:22:19:27:cd:f9
> [ 4.034747] tg3 0000:02:00.0 eth1: attached PHY is 5750 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
> [ 4.034896] tg3 0000:02:00.0 eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1]
> [ 4.035030] tg3 0000:02:00.0 eth1: dma_rwctrl[76180000] dma_mask[64-bit]
>
>> ---
>> drivers/pci/pci.h | 1 +
>> drivers/pci/probe.c | 35 ++++++++++++++++++++++++++++++-----
>> drivers/pci/quirks.c | 19 +++++++++++++++++++
>> include/linux/pci.h | 1 +
>> 4 files changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index f8113e5..7153c92 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -239,6 +239,7 @@ enum pci_bar_type {
>> pci_bar_mem64, /* A 64-bit memory BAR */
>> };
>>
>> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
>> bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
>> int crs_timeout);
>> int pci_setup_device(struct pci_dev *dev);
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 19c8950..bc067fb 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1684,22 +1684,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>> */
>> }
>>
>> -static void pci_configure_extended_tags(struct pci_dev *dev)
>> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
>> {
>> + struct pci_host_bridge *host;
>> u32 dev_cap;
>> int ret;
>> + u16 ctl;
>>
>> if (!pci_is_pcie(dev))
>> - return;
>> + return 0;
>>
>> ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
>> if (ret)
>> - return;
>> + return 0;
>> +
>> + if (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG))
>> + return 0;
>> +
>> + ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
>> + if (ret)
>> + return 0;
>> +
>> + host = pci_find_host_bridge(dev->bus);
>> + if (!host)
>> + return 0;
>>
>> - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
>> + if (host->broken_ext_tags) {
>> + if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
>> + dev_info(&dev->dev, "disabling Extended Tags\n");
>> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
>> + PCI_EXP_DEVCTL_EXT_TAG);
>> + }
>> + return 0;
>> + }
>> + if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
>> + dev_info(&dev->dev, "enabling Extended Tags\n");
>> pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
>> PCI_EXP_DEVCTL_EXT_TAG);
>> + }
>> + return 0;
>> }
>> +EXPORT_SYMBOL(pci_configure_extended_tags);
>>
>> static void pci_configure_device(struct pci_dev *dev)
>> {
>> @@ -1707,7 +1732,7 @@ static void pci_configure_device(struct pci_dev *dev)
>> int ret;
>>
>> pci_configure_mps(dev);
>> - pci_configure_extended_tags(dev);
>> + pci_configure_extended_tags(dev, NULL);
>>
>> memset(&hpp, 0, sizeof(hpp));
>> ret = pci_get_hp_params(dev, &hpp);
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 085fb78..b5dc99c 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
>> }
>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
>> +
>> +static void quirk_exttags_completer(struct pci_dev *pdev)
>> +{
>> + struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
>> +
>> + if (!bridge)
>> + return;
>> +
>> + bridge->broken_ext_tags = 1;
>> + dev_info(&pdev->dev, "Extended Tag handling is broken\n");
>> +
>> + pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140,
>> + quirk_exttags_completer);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142,
>> + quirk_exttags_completer);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144,
>> + quirk_exttags_completer);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 8039f9f..2b22c3e 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -441,6 +441,7 @@ struct pci_host_bridge {
>> void *release_data;
>> struct msi_controller *msi;
>> unsigned int ignore_reset_delay:1; /* for entire hierarchy */
>> + unsigned int broken_ext_tags:1;
>> /* Resource alignment requirements */
>> resource_size_t (*align_resource)(struct pci_dev *dev,
>> const struct resource *res,
>
>
On 7/12/2017 5:44 AM, Ethan Zhao wrote:
> The dmesg yelled "...Tag handling is broken" , but didn't say how it
> was handled, that is weird and confusing, just because there is no
> device under that bridge ?
>
> How about move the messages together "...is broken, disable Extended tag..."
This was something requested by Bjorn to inform that user has a broken HW
when a quirk is matched.
The new code will disable extended tags only if it was enabled on endpoint
via power on reset value or via some broken BIOS.
It looks like we are not seeing any disabling message. This implies that
both the card and BIOS is good.
That's why, you are no longer seeing a disabling message on this system
and code is just not enabling extended tags.
--
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.
On Wed, Jul 12, 2017 at 08:15:43AM -0400, Sinan Kaya wrote:
> On 7/12/2017 5:44 AM, Ethan Zhao wrote:
> > The dmesg yelled "...Tag handling is broken" , but didn't say how it
> > was handled, that is weird and confusing, just because there is no
> > device under that bridge ?
> >
> > How about move the messages together "...is broken, disable Extended tag..."
I changed the message this:
pci 0000:00:07.0: disabling Extended Tags (this device can't handle them)
We don't see any message from 0000:01:00.0 itself because it powered up
with Extended Tags disabled. If it powered up with Extended Tags *enabled*
(as allowed by the ECN), we would see this:
pci 0000:00:07.0: disabling Extended Tags (this device can't handle them)
pci 0000:01:00.0: disabling Extended Tags
Does that seem reasonable? Complete updated patch attached below.
> This was something requested by Bjorn to inform that user has a broken HW
> when a quirk is matched.
>
> The new code will disable extended tags only if it was enabled on endpoint
> via power on reset value or via some broken BIOS.
>
> It looks like we are not seeing any disabling message. This implies that
> both the card and BIOS is good.
>
> That's why, you are no longer seeing a disabling message on this system
> and code is just not enabling extended tags.
commit 2e6e588b99c4c7fbcea55eb48890fa8d0ab0c3d7
Author: Sinan Kaya <[email protected]>
Date: Wed Jul 12 00:04:14 2017 -0400
PCI: Mark Broadcom HT2100 Root Port Extended Tags as broken
Per PCIe r3.1, sec 2.2.6.2 and 7.8.4, a Requester may not use 8-bit Tags
unless its Extended Tag Field Enable is set, but all Receivers/Completers
must handle 8-bit Tags correctly regardless of their Extended Tag Field
Enable.
Some devices do not handle 8-bit Tags as Completers, so add a quirk for
them. If we find such a device, we disable Extended Tags for the entire
hierarchy to make peer-to-peer DMA possible.
The Broadcom HT2100 seems to have issues with handling 8-bit tags. Mark it
as broken.
The pci_walk_bus() in the quirk handles devices we've enumerated in the
past, and pci_configure_device() handles devices we enumerate in the
future.
Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
Reported-and-tested-by: Wim ten Have <[email protected]>
Signed-off-by: Sinan Kaya <[email protected]>
[bhelgaas: changelog, tweak messages, rename bit and quirk]
Signed-off-by: Bjorn Helgaas <[email protected]>
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 03e3d0285aea..9c3d123d358f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -234,6 +234,7 @@ enum pci_bar_type {
pci_bar_mem64, /* A 64-bit memory BAR */
};
+int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
int crs_timeout);
int pci_setup_device(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c31310db0404..c81c9835f4c7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1745,21 +1745,50 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
*/
}
-static void pci_configure_extended_tags(struct pci_dev *dev)
+int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
{
- u32 dev_cap;
+ struct pci_host_bridge *host;
+ u32 cap;
+ u16 ctl;
int ret;
if (!pci_is_pcie(dev))
- return;
+ return 0;
- ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
+ ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
if (ret)
- return;
+ return 0;
+
+ if (!(cap & PCI_EXP_DEVCAP_EXT_TAG))
+ return 0;
- if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+ ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
+ if (ret)
+ return 0;
+
+ host = pci_find_host_bridge(dev->bus);
+ if (!host)
+ return 0;
+
+ /*
+ * If some device in the hierarchy doesn't handle Extended Tags
+ * correctly, make sure they're disabled.
+ */
+ if (host->no_ext_tags) {
+ if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
+ dev_info(&dev->dev, "disabling Extended Tags\n");
+ pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+ PCI_EXP_DEVCTL_EXT_TAG);
+ }
+ return 0;
+ }
+
+ if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
+ dev_info(&dev->dev, "enabling Extended Tags\n");
pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
PCI_EXP_DEVCTL_EXT_TAG);
+ }
+ return 0;
}
static void pci_configure_device(struct pci_dev *dev)
@@ -1768,7 +1797,7 @@ static void pci_configure_device(struct pci_dev *dev)
int ret;
pci_configure_mps(dev);
- pci_configure_extended_tags(dev);
+ pci_configure_extended_tags(dev, NULL);
memset(&hpp, 0, sizeof(hpp));
ret = pci_get_hp_params(dev, &hpp);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6967c6b4cf6b..f135765555c9 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4681,3 +4681,19 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
+
+static void quirk_no_ext_tags(struct pci_dev *pdev)
+{
+ struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+
+ if (!bridge)
+ return;
+
+ bridge->no_ext_tags = 1;
+ dev_info(&pdev->dev, "disabling Extended Tags (this device can't handle them)\n");
+
+ pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140, quirk_no_ext_tags);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142, quirk_no_ext_tags);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144, quirk_no_ext_tags);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4869e66dd659..3b968d435895 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -451,6 +451,7 @@ struct pci_host_bridge {
void *release_data;
struct msi_controller *msi;
unsigned int ignore_reset_delay:1; /* for entire hierarchy */
+ unsigned int no_ext_tags:1; /* no Extended Tags */
/* Resource alignment requirements */
resource_size_t (*align_resource)(struct pci_dev *dev,
const struct resource *res,
On Wed, Jul 12, 2017 at 12:04:14AM -0400, Sinan Kaya wrote:
> All PCIe devices are expected to be able to handle 8-bit tags.
> 'commit 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")'
> enabled extended tags for all devices based on the spec direction.
>
> The Broadcom HT2100 seems to be having issues with handling
> 8-bit tags. Mark it as broken.
>
> If a device with extended tags quirk is found, disable extended tags for
> all devices in the tree assuming peer-to-peer is possible.
>
> The pci_walk_bus() in the quirk handles all devices we've already
> enumerated, and all devices we'll enumerate in the future are handled in
> pci_configure_device().
>
> Reported-by: Wim ten Have <[email protected]>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <[email protected]>
Applied with Wim's tested-by to pci/enumeration for v4.14, thanks!
> ---
> drivers/pci/pci.h | 1 +
> drivers/pci/probe.c | 35 ++++++++++++++++++++++++++++++-----
> drivers/pci/quirks.c | 19 +++++++++++++++++++
> include/linux/pci.h | 1 +
> 4 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index f8113e5..7153c92 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -239,6 +239,7 @@ enum pci_bar_type {
> pci_bar_mem64, /* A 64-bit memory BAR */
> };
>
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
> bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
> int crs_timeout);
> int pci_setup_device(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..bc067fb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,22 +1684,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
> */
> }
>
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
> {
> + struct pci_host_bridge *host;
> u32 dev_cap;
> int ret;
> + u16 ctl;
>
> if (!pci_is_pcie(dev))
> - return;
> + return 0;
>
> ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> if (ret)
> - return;
> + return 0;
> +
> + if (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG))
> + return 0;
> +
> + ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
> + if (ret)
> + return 0;
> +
> + host = pci_find_host_bridge(dev->bus);
> + if (!host)
> + return 0;
>
> - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> + if (host->broken_ext_tags) {
> + if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
> + dev_info(&dev->dev, "disabling Extended Tags\n");
> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> + }
> + if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
> + dev_info(&dev->dev, "enabling Extended Tags\n");
> pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> }
> +EXPORT_SYMBOL(pci_configure_extended_tags);
>
> static void pci_configure_device(struct pci_dev *dev)
> {
> @@ -1707,7 +1732,7 @@ static void pci_configure_device(struct pci_dev *dev)
> int ret;
>
> pci_configure_mps(dev);
> - pci_configure_extended_tags(dev);
> + pci_configure_extended_tags(dev, NULL);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 085fb78..b5dc99c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4664,3 +4664,22 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
> +
> +static void quirk_exttags_completer(struct pci_dev *pdev)
> +{
> + struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +
> + if (!bridge)
> + return;
> +
> + bridge->broken_ext_tags = 1;
> + dev_info(&pdev->dev, "Extended Tag handling is broken\n");
> +
> + pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140,
> + quirk_exttags_completer);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142,
> + quirk_exttags_completer);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144,
> + quirk_exttags_completer);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8039f9f..2b22c3e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -441,6 +441,7 @@ struct pci_host_bridge {
> void *release_data;
> struct msi_controller *msi;
> unsigned int ignore_reset_delay:1; /* for entire hierarchy */
> + unsigned int broken_ext_tags:1;
> /* Resource alignment requirements */
> resource_size_t (*align_resource)(struct pci_dev *dev,
> const struct resource *res,
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jul 13, 2017 at 3:34 AM, Bjorn Helgaas <[email protected]> wrote:
> On Wed, Jul 12, 2017 at 08:15:43AM -0400, Sinan Kaya wrote:
>> On 7/12/2017 5:44 AM, Ethan Zhao wrote:
>> > The dmesg yelled "...Tag handling is broken" , but didn't say how it
>> > was handled, that is weird and confusing, just because there is no
>> > device under that bridge ?
>> >
>> > How about move the messages together "...is broken, disable Extended tag..."
>
> I changed the message this:
>
> pci 0000:00:07.0: disabling Extended Tags (this device can't handle them)
>
> We don't see any message from 0000:01:00.0 itself because it powered up
> with Extended Tags disabled. If it powered up with Extended Tags *enabled*
> (as allowed by the ECN), we would see this:
>
> pci 0000:00:07.0: disabling Extended Tags (this device can't handle them)
> pci 0000:01:00.0: disabling Extended Tags
>
> Does that seem reasonable? Complete updated patch attached below.
>
>> This was something requested by Bjorn to inform that user has a broken HW
>> when a quirk is matched.
>>
>> The new code will disable extended tags only if it was enabled on endpoint
>> via power on reset value or via some broken BIOS.
>>
>> It looks like we are not seeing any disabling message. This implies that
>> both the card and BIOS is good.
>>
>> That's why, you are no longer seeing a disabling message on this system
>> and code is just not enabling extended tags.
>
>
> commit 2e6e588b99c4c7fbcea55eb48890fa8d0ab0c3d7
> Author: Sinan Kaya <[email protected]>
> Date: Wed Jul 12 00:04:14 2017 -0400
>
> PCI: Mark Broadcom HT2100 Root Port Extended Tags as broken
>
> Per PCIe r3.1, sec 2.2.6.2 and 7.8.4, a Requester may not use 8-bit Tags
> unless its Extended Tag Field Enable is set, but all Receivers/Completers
> must handle 8-bit Tags correctly regardless of their Extended Tag Field
> Enable.
>
> Some devices do not handle 8-bit Tags as Completers, so add a quirk for
> them. If we find such a device, we disable Extended Tags for the entire
> hierarchy to make peer-to-peer DMA possible.
>
> The Broadcom HT2100 seems to have issues with handling 8-bit tags. Mark it
> as broken.
>
> The pci_walk_bus() in the quirk handles devices we've enumerated in the
> past, and pci_configure_device() handles devices we enumerate in the
> future.
>
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Reported-and-tested-by: Wim ten Have <[email protected]>
> Signed-off-by: Sinan Kaya <[email protected]>
> [bhelgaas: changelog, tweak messages, rename bit and quirk]
> Signed-off-by: Bjorn Helgaas <[email protected]>
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 03e3d0285aea..9c3d123d358f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -234,6 +234,7 @@ enum pci_bar_type {
> pci_bar_mem64, /* A 64-bit memory BAR */
> };
>
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
> bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
> int crs_timeout);
> int pci_setup_device(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c31310db0404..c81c9835f4c7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1745,21 +1745,50 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
> */
> }
>
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
> {
> - u32 dev_cap;
> + struct pci_host_bridge *host;
> + u32 cap;
> + u16 ctl;
> int ret;
>
> if (!pci_is_pcie(dev))
> - return;
> + return 0;
>
> - ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> + ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
> if (ret)
> - return;
> + return 0;
> +
> + if (!(cap & PCI_EXP_DEVCAP_EXT_TAG))
> + return 0;
>
> - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> + ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
> + if (ret)
> + return 0;
> +
> + host = pci_find_host_bridge(dev->bus);
> + if (!host)
> + return 0;
> +
> + /*
> + * If some device in the hierarchy doesn't handle Extended Tags
> + * correctly, make sure they're disabled.
> + */
> + if (host->no_ext_tags) {
> + if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
> + dev_info(&dev->dev, "disabling Extended Tags\n");
> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> + }
> +
> + if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
> + dev_info(&dev->dev, "enabling Extended Tags\n");
> pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> }
>
> static void pci_configure_device(struct pci_dev *dev)
> @@ -1768,7 +1797,7 @@ static void pci_configure_device(struct pci_dev *dev)
> int ret;
>
> pci_configure_mps(dev);
> - pci_configure_extended_tags(dev);
> + pci_configure_extended_tags(dev, NULL);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6967c6b4cf6b..f135765555c9 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4681,3 +4681,19 @@ static void quirk_intel_no_flr(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502, quirk_intel_no_flr);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503, quirk_intel_no_flr);
> +
> +static void quirk_no_ext_tags(struct pci_dev *pdev)
> +{
> + struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +
> + if (!bridge)
> + return;
> +
> + bridge->no_ext_tags = 1;
> + dev_info(&pdev->dev, "disabling Extended Tags (this device can't handle them)\n");
A late 'Great !', at least we get reason - (this device can't handle
them) and result - (disabling Extended Tags) at the same time, feel
better.
Thanks,
Ethan
> +
> + pci_walk_bus(bridge->bus, pci_configure_extended_tags, NULL);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0140, quirk_no_ext_tags);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0142, quirk_no_ext_tags);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144, quirk_no_ext_tags);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 4869e66dd659..3b968d435895 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -451,6 +451,7 @@ struct pci_host_bridge {
> void *release_data;
> struct msi_controller *msi;
> unsigned int ignore_reset_delay:1; /* for entire hierarchy */
> + unsigned int no_ext_tags:1; /* no Extended Tags */
> /* Resource alignment requirements */
> resource_size_t (*align_resource)(struct pci_dev *dev,
> const struct resource *res,