This is a partial revert of commit 8b3517f88ff2 ("PCI:
loongson: Prevent LS7A MRRS increases") for MIPS based Loongson.
There are many MIPS based Loongson systems in wild that
shipped with firmware which does not set maximum MRRS properly.
Limiting MRRS to 256 for all as MIPS Loongson comes with higher
MRRS support is considered rare.
It must be done at device enablement stage because MRRS setting
may get lost if the parent bridge lost PCI_COMMAND_MASTER, and
we are only sure parent bridge is enabled at this point.
Cc: [email protected]
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
Signed-off-by: Jiaxun Yang <[email protected]>
---
v4: Improve commit message
v5:
- Improve commit message and comments.
- Style fix from Huacai's off-list input.
---
drivers/pci/controller/pci-loongson.c | 47 ++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
index d45e7b8dc530..128cc95b236f 100644
--- a/drivers/pci/controller/pci-loongson.c
+++ b/drivers/pci/controller/pci-loongson.c
@@ -80,13 +80,50 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DEV_LS7A_LPC, system_bus_quirk);
+/*
+ * Some Loongson PCIe ports have h/w limitations of maximum read
+ * request size. They can't handle anything larger than this.
+ * Sane firmware will set proper MRRS at boot, so we only need
+ * no_inc_mrrs for bridges. However, some MIPS Loongson firmware
+ * won't set MRRS properly, and we have to enforce maximum safe
+ * MRRS, which is 256 bytes.
+ */
+#ifdef CONFIG_MIPS
+static void loongson_set_min_mrrs_quirk(struct pci_dev *pdev)
+{
+ struct pci_bus *bus = pdev->bus;
+ struct pci_dev *bridge;
+ static const struct pci_device_id bridge_devids[] = {
+ { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) },
+ { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) },
+ { 0, },
+ };
+
+ /* look for the matching bridge */
+ while (!pci_is_root_bus(bus)) {
+ bridge = bus->self;
+ bus = bus->parent;
+
+ if (pci_match_id(bridge_devids, bridge)) {
+ if (pcie_get_readrq(pdev) > 256) {
+ pci_info(pdev, "limiting MRRS to 256\n");
+ pcie_set_readrq(pdev, 256);
+ }
+ break;
+ }
+ }
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
+#endif
+
static void loongson_mrrs_quirk(struct pci_dev *pdev)
{
- /*
- * Some Loongson PCIe ports have h/w limitations of maximum read
- * request size. They can't handle anything larger than this. So
- * force this limit on any devices attached under these ports.
- */
struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
bridge->no_inc_mrrs = 1;
--
2.34.1
在2023年11月19日十一月 下午9:56，Jiaxun Yang写道：
> This is a partial revert of commit 8b3517f88ff2 ("PCI:
> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson.
>
> There are many MIPS based Loongson systems in wild that
> shipped with firmware which does not set maximum MRRS properly.
>
> Limiting MRRS to 256 for all as MIPS Loongson comes with higher
> MRRS support is considered rare.
>
> It must be done at device enablement stage because MRRS setting
> may get lost if the parent bridge lost PCI_COMMAND_MASTER, and
> we are only sure parent bridge is enabled at this point.
>
> Cc: [email protected]
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> v4: Improve commit message
> v5:
> - Improve commit message and comments.
> - Style fix from Huacai's off-list input.
> ---
> drivers/pci/controller/pci-loongson.c | 47 ++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-loongson.c
> b/drivers/pci/controller/pci-loongson.c
> index d45e7b8dc530..128cc95b236f 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -80,13 +80,50 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, system_bus_quirk);
>
> +/*
> + * Some Loongson PCIe ports have h/w limitations of maximum read
> + * request size. They can't handle anything larger than this.
> + * Sane firmware will set proper MRRS at boot, so we only need
> + * no_inc_mrrs for bridges. However, some MIPS Loongson firmware
> + * won't set MRRS properly, and we have to enforce maximum safe
> + * MRRS, which is 256 bytes.
> + */
> +#ifdef CONFIG_MIPS
> +static void loongson_set_min_mrrs_quirk(struct pci_dev *pdev)
> +{
> + struct pci_bus *bus = pdev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) },
> + { 0, },
> + };
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> +
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(pdev) > 256) {
> + pci_info(pdev, "limiting MRRS to 256\n");
> + pcie_set_readrq(pdev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
^ Oops sed issue.
Will fix in next version if everybody is happy with the implementation.
Thanks
--
- Jiaxun
LGTM, so
Acked-by: Huacai Chen <[email protected]>
On Mon, Nov 20, 2023 at 5:56 AM Jiaxun Yang <[email protected]> wrote:
>
> This is a partial revert of commit 8b3517f88ff2 ("PCI:
> loongson: Prevent LS7A MRRS increases") for MIPS based Loongson.
>
> There are many MIPS based Loongson systems in wild that
> shipped with firmware which does not set maximum MRRS properly.
>
> Limiting MRRS to 256 for all as MIPS Loongson comes with higher
> MRRS support is considered rare.
>
> It must be done at device enablement stage because MRRS setting
> may get lost if the parent bridge lost PCI_COMMAND_MASTER, and
> we are only sure parent bridge is enabled at this point.
>
> Cc: [email protected]
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217680
> Fixes: 8b3517f88ff2 ("PCI: loongson: Prevent LS7A MRRS increases")
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> v4: Improve commit message
> v5:
> - Improve commit message and comments.
> - Style fix from Huacai's off-list input.
> ---
> drivers/pci/controller/pci-loongson.c | 47 ++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index d45e7b8dc530..128cc95b236f 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -80,13 +80,50 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, system_bus_quirk);
>
> +/*
> + * Some Loongson PCIe ports have h/w limitations of maximum read
> + * request size. They can't handle anything larger than this.
> + * Sane firmware will set proper MRRS at boot, so we only need
> + * no_inc_mrrs for bridges. However, some MIPS Loongson firmware
> + * won't set MRRS properly, and we have to enforce maximum safe
> + * MRRS, which is 256 bytes.
> + */
> +#ifdef CONFIG_MIPS
> +static void loongson_set_min_mrrs_quirk(struct pci_dev *pdev)
> +{
> + struct pci_bus *bus = pdev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) },
> + { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) },
> + { 0, },
> + };
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> +
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(pdev) > 256) {
> + pci_info(pdev, "limiting MRRS to 256\n");
> + pcie_set_readrq(pdev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
> +#endif
> +
> static void loongson_mrrs_quirk(struct pci_dev *pdev)
> {
> - /*
> - * Some Loongson PCIe ports have h/w limitations of maximum read
> - * request size. They can't handle anything larger than this. So
> - * force this limit on any devices attached under these ports.
> - */
> struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
>
> bridge->no_inc_mrrs = 1;
> --
> 2.34.1
>
Hi Jiaxun,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.7-rc2 next-20231120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jiaxun-Yang/pci-loongson-Workaround-MIPS-firmware-MRRS-settings/20231120-055946
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231119215635.52810-1-jiaxun.yang%40flygoat.com
patch subject: [PATCH v5] pci: loongson: Workaround MIPS firmware MRRS settings
config: mips-allmodconfig (https://download.01.org/0day-ci/archive/20231120/[email protected]/config)
compiler: mips-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231120/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/pci/controller/pci-loongson.c:10:
drivers/pci/controller/pci-loongson.c:122:50: error: 'loongson_mrrs_quirk_old' undeclared here (not in a function)
122 | DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/pci.h:2241:57: note: in definition of macro 'DECLARE_PCI_FIXUP_SECTION'
2241 | = { vendor, device, class, class_shift, hook };
| ^~~~
drivers/pci/controller/pci-loongson.c:122:1: note: in expansion of macro 'DECLARE_PCI_FIXUP_ENABLE'
122 | DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/pci-loongson.c:92:13: warning: 'loongson_set_min_mrrs_quirk' defined but not used [-Wunused-function]
92 | static void loongson_set_min_mrrs_quirk(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/loongson_set_min_mrrs_quirk +92 drivers/pci/controller/pci-loongson.c
54
55 /* Fixup wrong class code in PCIe bridges */
56 static void bridge_class_quirk(struct pci_dev *dev)
57 {
58 dev->class = PCI_CLASS_BRIDGE_PCI_NORMAL;
59 }
60 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
61 DEV_LS7A_PCIE_PORT0, bridge_class_quirk);
62 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
63 DEV_LS7A_PCIE_PORT1, bridge_class_quirk);
64 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
65 DEV_LS7A_PCIE_PORT2, bridge_class_quirk);
66
67 static void system_bus_quirk(struct pci_dev *pdev)
68 {
69 /*
70 * The address space consumed by these devices is outside the
71 * resources of the host bridge.
72 */
73 pdev->mmio_always_on = 1;
74 pdev->non_compliant_bars = 1;
75 }
76 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
77 DEV_LS2K_APB, system_bus_quirk);
78 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
79 DEV_LS7A_CONF, system_bus_quirk);
80 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
81 DEV_LS7A_LPC, system_bus_quirk);
82
83 /*
84 * Some Loongson PCIe ports have h/w limitations of maximum read
85 * request size. They can't handle anything larger than this.
86 * Sane firmware will set proper MRRS at boot, so we only need
87 * no_inc_mrrs for bridges. However, some MIPS Loongson firmware
88 * won't set MRRS properly, and we have to enforce maximum safe
89 * MRRS, which is 256 bytes.
90 */
91 #ifdef CONFIG_MIPS
> 92 static void loongson_set_min_mrrs_quirk(struct pci_dev *pdev)
93 {
94 struct pci_bus *bus = pdev->bus;
95 struct pci_dev *bridge;
96 static const struct pci_device_id bridge_devids[] = {
97 { PCI_VDEVICE(LOONGSON, DEV_LS2K_PCIE_PORT0) },
98 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT0) },
99 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT1) },
100 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT2) },
101 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT3) },
102 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT4) },
103 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT5) },
104 { PCI_VDEVICE(LOONGSON, DEV_LS7A_PCIE_PORT6) },
105 { 0, },
106 };
107
108 /* look for the matching bridge */
109 while (!pci_is_root_bus(bus)) {
110 bridge = bus->self;
111 bus = bus->parent;
112
113 if (pci_match_id(bridge_devids, bridge)) {
114 if (pcie_get_readrq(pdev) > 256) {
115 pci_info(pdev, "limiting MRRS to 256\n");
116 pcie_set_readrq(pdev, 256);
117 }
118 break;
119 }
120 }
121 }
122 DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk_old);
123 #endif
124
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki