From: Rob Herring <robh@kernel.org>
To: Huacai Chen <chenhuacai@loongson.cn>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, Xuefeng Li <lixuefeng@loongson.cn>,
Huacai Chen <chenhuacai@gmail.com>,
Jiaxun Yang <jiaxun.yang@flygoat.com>
Subject: Re: [PATCH V12 4/6] PCI: loongson: Improve the MRRS quirk for LS7A
Date: Tue, 12 Apr 2022 09:49:05 -0500 [thread overview]
Message-ID: <YlWRYTuS6194hVjV@robh.at.kernel.org> (raw)
In-Reply-To: <20220226104731.76776-5-chenhuacai@loongson.cn>
On Sat, Feb 26, 2022 at 06:47:29PM +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 host bridge bit
> flag (i.e., 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" ("too big" means larger than the
> PCIe ports can handle, which means 256 for some ports and 4096 for the
> others, and of course this is a problem in the LS7A's hardware design).
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> drivers/pci/controller/pci-loongson.c | 47 ++++++++++-----------------
> drivers/pci/pci.c | 6 ++++
> include/linux/pci.h | 1 +
> 3 files changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index 48d9d283cb59..ba182f9d5718 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -67,37 +67,26 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, system_bus_quirk);
>
> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> +static void loongson_mrrs_quirk(struct pci_dev *pdev)
> {
> - 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;
> - }
> - }
> + /*
> + * 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);
> +
> + if (!bridge)
Isn't this condition impossible? I'd drop and just let the NULL
dereference fault happen rather than continue on silently.
> + return;
> +
> + bridge->no_inc_mrrs = 1;
> }
> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_0, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_1, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_2, loongson_mrrs_quirk);
>
> static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus)
> {
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 9ecce435fb3f..72a15bf9eee8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5993,6 +5993,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> {
> u16 v;
> int ret;
> + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
>
> if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
> return -EINVAL;
> @@ -6011,6 +6012,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>
> v = (ffs(rq) - 8) << 12;
>
> + if (bridge->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/include/linux/pci.h b/include/linux/pci.h
> index 8253a5413d7c..01a464eb640a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -569,6 +569,7 @@ struct pci_host_bridge {
> void *release_data;
> unsigned int ignore_reset_delay:1; /* For entire hierarchy */
> unsigned int no_ext_tags:1; /* No Extended Tags */
> + unsigned int no_inc_mrrs:1; /* No Increase MRRS */
> unsigned int native_aer:1; /* OS may use PCIe AER */
> unsigned int native_pcie_hotplug:1; /* OS may use PCIe hotplug */
> unsigned int native_shpc_hotplug:1; /* OS may use SHPC hotplug */
> --
> 2.27.0
>
next prev parent reply other threads:[~2022-04-12 14:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-26 10:47 [PATCH V12 0/6] PCI: Loongson pci improvements and quirks Huacai Chen
2022-02-26 10:47 ` [PATCH V12 1/6] PCI: loongson: Use generic 8/16/32-bit config ops on LS2K/LS7A Huacai Chen
2022-04-12 14:40 ` Rob Herring
2022-04-13 7:02 ` Huacai Chen
2022-02-26 10:47 ` [PATCH V12 2/6] PCI: loongson: Add ACPI init support Huacai Chen
2022-02-26 10:47 ` [PATCH V12 3/6] PCI: loongson: Don't access unexisting devices Huacai Chen
2022-02-26 10:47 ` [PATCH V12 4/6] PCI: loongson: Improve the MRRS quirk for LS7A Huacai Chen
2022-04-12 14:49 ` Rob Herring [this message]
2022-04-13 7:06 ` Huacai Chen
2022-02-26 10:47 ` [PATCH V12 5/6] PCI: Add quirk for LS7A to avoid reboot failure Huacai Chen
2022-02-26 10:47 ` [PATCH V12 6/6] PCI: Add quirk for multifunction devices of LS7A Huacai Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YlWRYTuS6194hVjV@robh.at.kernel.org \
--to=robh@kernel.org \
--cc=bhelgaas@google.com \
--cc=chenhuacai@gmail.com \
--cc=chenhuacai@loongson.cn \
--cc=jiaxun.yang@flygoat.com \
--cc=linux-pci@vger.kernel.org \
--cc=lixuefeng@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).