| Message ID | 20210628101027.1372370-4-chenhuacai@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Loongson-related pci quirks | expand |
在 2021/6/28 下午6:10, Huacai Chen 写道: > In new revision of LS7A, some PCIe ports support larger value than 256, > but their maximum supported MRRS values are not detectable. Moreover, > the current loongson_mrrs_quirk() cannot avoid devices increasing its > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169) > will actually set a big value in its driver. So the only possible way > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e., > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations. > > However, according to PCIe Spec, it is legal for an OS to program any > value for MRRS, and it is also legal for an endpoint to generate a Read > Request with any size up to its MRRS. As the hardware engineers say, the > root cause here is LS7A doesn't break up large read requests. In detail, > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read > request with a size that's "too big" (Yes, that is a problem in the LS7A > hardware design). > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > drivers/pci/pci.c | 5 +++++ > drivers/pci/quirks.c | 41 +++++++++++------------------------------ > include/linux/pci.h | 1 + > 3 files changed, 17 insertions(+), 30 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 8d4ebe095d0c..0f1ff4a6fe44 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > v = (ffs(rq) - 8) << 12; > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) { > + if (rq > pcie_get_readrq(dev)) > + return -EINVAL; > + } > + > ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL, > PCI_EXP_DEVCTL_READRQ, v); > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index dee4798a49fc..4bbdf5a5425f 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DEV_LS7A_LPC, loongson_system_bus_quirk); > > -static void loongson_mrrs_quirk(struct pci_dev *dev) > -{ > - struct pci_bus *bus = dev->bus; > - struct pci_dev *bridge; > - static const struct pci_device_id bridge_devids[] = { > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) }, > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) }, > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) }, > - { 0, }, > - }; > - > - /* look for the matching bridge */ > - while (!pci_is_root_bus(bus)) { > - bridge = bus->self; > - bus = bus->parent; > - /* > - * Some Loongson PCIe ports have a h/w limitation of > - * 256 bytes maximum read request size. They can't handle > - * anything larger than this. So force this limit on > - * any devices attached under these ports. > - */ > - if (pci_match_id(bridge_devids, bridge)) { > - if (pcie_get_readrq(dev) > 256) { > - pci_info(dev, "limiting MRRS to 256\n"); > - pcie_set_readrq(dev, 256); > - } > - break; > - } > - } > +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. > + */ > + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS; > } > -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk); > > /* > * The Mellanox Tavor device gives false positive parity errors. Disable > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 24306504226a..b336239b5282 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -240,6 +240,7 @@ enum pci_bus_flags { > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4, > PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8, > + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16, > }; > > /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote: > In new revision of LS7A, some PCIe ports support larger value than 256, > but their maximum supported MRRS values are not detectable. Moreover, > the current loongson_mrrs_quirk() cannot avoid devices increasing its > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169) > will actually set a big value in its driver. So the only possible way > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e., > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations. > > However, according to PCIe Spec, it is legal for an OS to program any > value for MRRS, and it is also legal for an endpoint to generate a Read > Request with any size up to its MRRS. As the hardware engineers say, the > root cause here is LS7A doesn't break up large read requests. In detail, > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read > request with a size that's "too big" (Yes, that is a problem in the LS7A > hardware design). > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > drivers/pci/pci.c | 5 +++++ > drivers/pci/quirks.c | 41 +++++++++++------------------------------ > include/linux/pci.h | 1 + > 3 files changed, 17 insertions(+), 30 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 8d4ebe095d0c..0f1ff4a6fe44 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > v = (ffs(rq) - 8) << 12; > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) { > + if (rq > pcie_get_readrq(dev)) > + return -EINVAL; I'd prefer to make this simpler, so we just never touch MRRS at all, like this: @@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) u16 v; int ret; + if (<loongson-quirk>) + return -EINVAL; + if (rq < 128 || rq > 4096 || !is_power_of_2(rq)) return -EINVAL; What would that break? It's just harder to analyze the behavior if it depends on what the driver is trying to do. AFAIK, devices should *work* correctly with any value of MRRS. > + } > + > ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL, > PCI_EXP_DEVCTL_READRQ, v); > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index dee4798a49fc..4bbdf5a5425f 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > DEV_LS7A_LPC, loongson_system_bus_quirk); > > -static void loongson_mrrs_quirk(struct pci_dev *dev) > -{ > - struct pci_bus *bus = dev->bus; > - struct pci_dev *bridge; > - static const struct pci_device_id bridge_devids[] = { > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) }, > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) }, > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) }, > - { 0, }, > - }; > - > - /* look for the matching bridge */ > - while (!pci_is_root_bus(bus)) { > - bridge = bus->self; > - bus = bus->parent; > - /* > - * Some Loongson PCIe ports have a h/w limitation of > - * 256 bytes maximum read request size. They can't handle > - * anything larger than this. So force this limit on > - * any devices attached under these ports. > - */ > - if (pci_match_id(bridge_devids, bridge)) { > - if (pcie_get_readrq(dev) > 256) { > - pci_info(dev, "limiting MRRS to 256\n"); > - pcie_set_readrq(dev, 256); > - } > - break; > - } > - } > +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. > + */ > + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS; > } > -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk); > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk); Thanks for making this quirk Loongson-specific. Can you reverse the order of patches 2 and 3, so this fix happens before moving the quirk to drivers/pci/quirks.c? > /* > * The Mellanox Tavor device gives false positive parity errors. Disable > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 24306504226a..b336239b5282 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -240,6 +240,7 @@ enum pci_bus_flags { > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4, > PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8, > + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16, This is not a property of the *bus*. Apparently it's a property of the Root Port or maybe of the Root Complex itself. What about RCiePs, which don't have a Root Port above them? They still have an MRRS field in their Device Control registers. Are there restrictions on how MRRS can be set for an RCiEP? If you need to restrict MRRS for RCiEPs as well as for devices below LS7A Root Ports, I think setting a bit in struct pci_host_bridge and using pci_find_host_bridge() would work. If you don't need to restrict MRRS for RCiEPs (or if there are no RCiEPs at all) you could put a bit in the struct pci_dev and use pcie_find_root_port(). But this would consume a bit in *every* pci_dev on every system, so it's a little more wasteful in that sense. > }; > > /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ > -- > 2.27.0 >
Hi, Bjorn, On Tue, Jun 29, 2021 at 6:35 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote: > > In new revision of LS7A, some PCIe ports support larger value than 256, > > but their maximum supported MRRS values are not detectable. Moreover, > > the current loongson_mrrs_quirk() cannot avoid devices increasing its > > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169) > > will actually set a big value in its driver. So the only possible way > > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e., > > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations. > > > > However, according to PCIe Spec, it is legal for an OS to program any > > value for MRRS, and it is also legal for an endpoint to generate a Read > > Request with any size up to its MRRS. As the hardware engineers say, the > > root cause here is LS7A doesn't break up large read requests. In detail, > > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read > > request with a size that's "too big" (Yes, that is a problem in the LS7A > > hardware design). > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > --- > > drivers/pci/pci.c | 5 +++++ > > drivers/pci/quirks.c | 41 +++++++++++------------------------------ > > include/linux/pci.h | 1 + > > 3 files changed, 17 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 8d4ebe095d0c..0f1ff4a6fe44 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > > > v = (ffs(rq) - 8) << 12; > > > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) { > > + if (rq > pcie_get_readrq(dev)) > > + return -EINVAL; > > I'd prefer to make this simpler, so we just never touch MRRS at all, > like this: > > @@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > u16 v; > int ret; > > + if (<loongson-quirk>) > + return -EINVAL; > + > if (rq < 128 || rq > 4096 || !is_power_of_2(rq)) > return -EINVAL; > > What would that break? It's just harder to analyze the behavior if it > depends on what the driver is trying to do. AFAIK, devices should > *work* correctly with any value of MRRS. This disables both increase and decrease MRRS, and so make pcie_bus_config completely useless. I think allowing decrease MRRS is useful in the PEER2PEER case. > > > + } > > + > > ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL, > > PCI_EXP_DEVCTL_READRQ, v); > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index dee4798a49fc..4bbdf5a5425f 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, > > DEV_LS7A_LPC, loongson_system_bus_quirk); > > > > -static void loongson_mrrs_quirk(struct pci_dev *dev) > > -{ > > - struct pci_bus *bus = dev->bus; > > - struct pci_dev *bridge; > > - static const struct pci_device_id bridge_devids[] = { > > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) }, > > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) }, > > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) }, > > - { 0, }, > > - }; > > - > > - /* look for the matching bridge */ > > - while (!pci_is_root_bus(bus)) { > > - bridge = bus->self; > > - bus = bus->parent; > > - /* > > - * Some Loongson PCIe ports have a h/w limitation of > > - * 256 bytes maximum read request size. They can't handle > > - * anything larger than this. So force this limit on > > - * any devices attached under these ports. > > - */ > > - if (pci_match_id(bridge_devids, bridge)) { > > - if (pcie_get_readrq(dev) > 256) { > > - pci_info(dev, "limiting MRRS to 256\n"); > > - pcie_set_readrq(dev, 256); > > - } > > - break; > > - } > > - } > > +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. > > + */ > > + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS; > > } > > -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk); > > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk); > > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk); > > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk); > > Thanks for making this quirk Loongson-specific. Can you reverse the > order of patches 2 and 3, so this fix happens before moving the quirk > to drivers/pci/quirks.c? OK, I will reverse them. > > > /* > > * The Mellanox Tavor device gives false positive parity errors. Disable > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 24306504226a..b336239b5282 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -240,6 +240,7 @@ enum pci_bus_flags { > > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > > PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4, > > PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8, > > + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16, > > This is not a property of the *bus*. > > Apparently it's a property of the Root Port or maybe of the Root > Complex itself. What about RCiePs, which don't have a Root Port above > them? They still have an MRRS field in their Device Control > registers. Are there restrictions on how MRRS can be set for an > RCiEP? > > If you need to restrict MRRS for RCiEPs as well as for devices below > LS7A Root Ports, I think setting a bit in struct pci_host_bridge and > using pci_find_host_bridge() would work. OK, I will set a bit in pci_host_bridge. Huacai > > If you don't need to restrict MRRS for RCiEPs (or if there are no > RCiEPs at all) you could put a bit in the struct pci_dev and use > pcie_find_root_port(). But this would consume a bit in *every* > pci_dev on every system, so it's a little more wasteful in that sense. > > > }; > > > > /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */ > > -- > > 2.27.0 > >
On Tue, Jun 29, 2021 at 11:20:45AM +0800, Huacai Chen wrote: > On Tue, Jun 29, 2021 at 6:35 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote: > > > In new revision of LS7A, some PCIe ports support larger value than 256, > > > but their maximum supported MRRS values are not detectable. Moreover, > > > the current loongson_mrrs_quirk() cannot avoid devices increasing its > > > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169) > > > will actually set a big value in its driver. So the only possible way > > > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e., > > > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations. > > > > > > However, according to PCIe Spec, it is legal for an OS to program any > > > value for MRRS, and it is also legal for an endpoint to generate a Read > > > Request with any size up to its MRRS. As the hardware engineers say, the > > > root cause here is LS7A doesn't break up large read requests. In detail, > > > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read > > > request with a size that's "too big" (Yes, that is a problem in the LS7A > > > hardware design). > > > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > > --- > > > drivers/pci/pci.c | 5 +++++ > > > drivers/pci/quirks.c | 41 +++++++++++------------------------------ > > > include/linux/pci.h | 1 + > > > 3 files changed, 17 insertions(+), 30 deletions(-) > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > index 8d4ebe095d0c..0f1ff4a6fe44 100644 > > > --- a/drivers/pci/pci.c > > > +++ b/drivers/pci/pci.c > > > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > > > > > v = (ffs(rq) - 8) << 12; > > > > > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) { > > > + if (rq > pcie_get_readrq(dev)) > > > + return -EINVAL; > > > > I'd prefer to make this simpler, so we just never touch MRRS at all, > > like this: > > > > @@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > u16 v; > > int ret; > > > > + if (<loongson-quirk>) > > + return -EINVAL; > > + > > if (rq < 128 || rq > 4096 || !is_power_of_2(rq)) > > return -EINVAL; > > > > What would that break? It's just harder to analyze the behavior if it > > depends on what the driver is trying to do. AFAIK, devices should > > *work* correctly with any value of MRRS. > > This disables both increase and decrease MRRS, and so make > pcie_bus_config completely useless. I think allowing decrease MRRS is > useful in the PEER2PEER case. True. The MPS/MRRS algorithm, including pcie_bus_config, is very complicated and poorly understood, so I'm a little hesitant to complicate it even more with one-off quirks. I'd really prefer to push LS7A off into a corner and have the generic MPS/MRRS algorithm ignore it completely. But I guess just keeping pcie_set_readrq() from *increasing* MRRS doesn't add too much complication and should always be safe. Bjorn
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 8d4ebe095d0c..0f1ff4a6fe44 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) v = (ffs(rq) - 8) << 12; + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) { + if (rq > pcie_get_readrq(dev)) + return -EINVAL; + } + ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_READRQ, v); diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index dee4798a49fc..4bbdf5a5425f 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_LPC, loongson_system_bus_quirk); -static void loongson_mrrs_quirk(struct pci_dev *dev) -{ - struct pci_bus *bus = dev->bus; - struct pci_dev *bridge; - static const struct pci_device_id bridge_devids[] = { - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) }, - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) }, - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) }, - { 0, }, - }; - - /* look for the matching bridge */ - while (!pci_is_root_bus(bus)) { - bridge = bus->self; - bus = bus->parent; - /* - * Some Loongson PCIe ports have a h/w limitation of - * 256 bytes maximum read request size. They can't handle - * anything larger than this. So force this limit on - * any devices attached under these ports. - */ - if (pci_match_id(bridge_devids, bridge)) { - if (pcie_get_readrq(dev) > 256) { - pci_info(dev, "limiting MRRS to 256\n"); - pcie_set_readrq(dev, 256); - } - break; - } - } +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. + */ + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS; } -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk); +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk); +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk); +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk); /* * The Mellanox Tavor device gives false positive parity errors. Disable diff --git a/include/linux/pci.h b/include/linux/pci.h index 24306504226a..b336239b5282 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -240,6 +240,7 @@ enum pci_bus_flags { PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4, PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8, + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16, }; /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
In new revision of LS7A, some PCIe ports support larger value than 256, but their maximum supported MRRS values are not detectable. Moreover, the current loongson_mrrs_quirk() cannot avoid devices increasing its MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169) will actually set a big value in its driver. So the only possible way is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e., PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations. However, according to PCIe Spec, it is legal for an OS to program any value for MRRS, and it is also legal for an endpoint to generate a Read Request with any size up to its MRRS. As the hardware engineers say, the root cause here is LS7A doesn't break up large read requests. In detail, LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read request with a size that's "too big" (Yes, that is a problem in the LS7A hardware design). Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> --- drivers/pci/pci.c | 5 +++++ drivers/pci/quirks.c | 41 +++++++++++------------------------------ include/linux/pci.h | 1 + 3 files changed, 17 insertions(+), 30 deletions(-)