All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: "Dan Williams" <dan.j.williams@intel.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [RFC PATCH 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6
Date: Tue,  4 Mar 2025 15:51:08 +0200	[thread overview]
Message-ID: <20250304135108.2599-1-ilpo.jarvinen@linux.intel.com> (raw)

Disallow Extended Tags and Max Read Request Size (MRRS) larger than
128B for devices under Xeon 6 Root Ports if the Root Port is bifurcated
to x2. Also, 10-Bit Tag Requester should be disallowed for device
underneath these Root Ports but there is currently no 10-Bit Tag
support in the kernel.

The normal path that writes MRRS is through
pcie_bus_configure_settings() -> pcie_bus_configure_set() ->
pcie_write_mrrs() and contains a few early returns that are based on
the value of pcie_bus_config. Overriding such checks with the host
bridge flag check on each level seems messy. Thus, simply ensure MRRS
is always written in pci_configure_device() if a device requiring the
quirk is detected.

Link: https://cdrdv2.intel.com/v1/dl/getContent/837176
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---

The normal path that writes MRRS is somewhat convoluted so I ensure MRRS
gets written in a more direct way, I'm not sure if that's the best
approach. Thus sending this as RFC.

 drivers/pci/pci.c    | 15 ++++++++-------
 drivers/pci/probe.c  |  8 +++++++-
 drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
 include/linux/pci.h  |  1 +
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..81ddad81ccb8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5913,7 +5913,7 @@ EXPORT_SYMBOL(pcie_get_readrq);
 int pcie_set_readrq(struct pci_dev *dev, int rq)
 {
 	u16 v;
-	int ret;
+	int ret, max_mrrs = 4096;
 	struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
 
 	if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
@@ -5933,13 +5933,14 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
 
 	v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);
 
-	if (bridge->no_inc_mrrs) {
-		int max_mrrs = pcie_get_readrq(dev);
+	if (bridge->no_inc_mrrs)
+		max_mrrs = pcie_get_readrq(dev);
+	if (bridge->only_128b_mrrs)
+		max_mrrs = 128;
 
-		if (rq > max_mrrs) {
-			pci_info(dev, "can't set Max_Read_Request_Size to %d; max is %d\n", rq, max_mrrs);
-			return -EINVAL;
-		}
+	if (rq > max_mrrs) {
+		pci_info(dev, "can't set Max_Read_Request_Size to %d; max is %d\n", rq, max_mrrs);
+		return -EINVAL;
 	}
 
 	ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b6536ed599c3..ceaa34b0525b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2342,7 +2342,11 @@ static void pci_configure_serr(struct pci_dev *dev)
 
 static void pci_configure_device(struct pci_dev *dev)
 {
+	struct pci_host_bridge *host_bridge = pci_find_host_bridge(dev->bus);
+
 	pci_configure_mps(dev);
+	if (host_bridge && host_bridge->only_128b_mrrs)
+		pcie_set_readrq(dev, 128);
 	pci_configure_extended_tags(dev, NULL);
 	pci_configure_relaxed_ordering(dev);
 	pci_configure_ltr(dev);
@@ -2851,13 +2855,15 @@ static void pcie_write_mps(struct pci_dev *dev, int mps)
 
 static void pcie_write_mrrs(struct pci_dev *dev)
 {
+	struct pci_host_bridge *host_bridge = pci_find_host_bridge(dev->bus);
 	int rc, mrrs;
 
 	/*
 	 * In the "safe" case, do not configure the MRRS.  There appear to be
 	 * issues with setting MRRS to 0 on a number of devices.
 	 */
-	if (pcie_bus_config != PCIE_BUS_PERFORMANCE)
+	if (pcie_bus_config != PCIE_BUS_PERFORMANCE &&
+	    (!host_bridge || !host_bridge->only_128b_mrrs))
 		return;
 
 	/*
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b84ff7bade82..987cd94028e1 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5564,6 +5564,33 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144, quirk_no_ext_tags);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0420, quirk_no_ext_tags);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0422, quirk_no_ext_tags);
 
+static void quirk_pcie2x_no_tags_no_mrrs(struct pci_dev *pdev)
+{
+	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+	u32 linkcap;
+
+	if (!bridge)
+		return;
+
+	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &linkcap);
+	if (FIELD_GET(PCI_EXP_LNKCAP_MLW, linkcap) != 0x2)
+		return;
+
+	bridge->no_ext_tags = 1;
+	bridge->only_128b_mrrs = 1;
+	pci_info(pdev, "Disabling Extended Tags and forcing MRRS to 128B (performance reasons due to 2x PCIe link)\n");
+}
+
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db0, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db1, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db2, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db3, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db6, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db7, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db8, quirk_pcie2x_no_tags_no_mrrs);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db9, quirk_pcie2x_no_tags_no_mrrs);
+
+
 #ifdef CONFIG_PCI_ATS
 static void quirk_no_ats(struct pci_dev *pdev)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..def29c8c0f84 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -601,6 +601,7 @@ struct pci_host_bridge {
 	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	only_128b_mrrs:1;	/* Only 128B 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 */

base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
-- 
2.39.5


             reply	other threads:[~2025-03-04 13:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-04 13:51 Ilpo Järvinen [this message]
2025-03-04 21:14 ` [RFC PATCH 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6 Bjorn Helgaas
2025-03-05 20:38   ` Dan Williams
2025-03-07 13:06   ` Ilpo Järvinen
2025-03-07 16:39     ` Bjorn Helgaas
2025-03-07 20:50     ` Dan Williams
2025-03-07  8:34 ` Lukas Wunner
2025-03-07 13:13   ` Ilpo Järvinen

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=20250304135108.2599-1-ilpo.jarvinen@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.