[PATCH 0/3] Add command to set MPS of all PCI Express devices

[PATCH 0/3] Add command to set MPS of all PCI Express devices

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

This patch set adds support for configuring the Maximum Payload Size
(MPS) for attached PCI Express devices. Modifying the MPS can increase
PCI Express performance or ensure compatibility with hot-plugged
devices.

New config CONFIG_CMD_PCI_MPS. If enabled, a new "pci_mps" command will
be added that accepts an argument describing a strategy to configure the
PCI Express payload size for devices that are currently attached. Two
strategies are currently supported:

"safe" emulates the Linux kernel's pci=pcie_bus_safe and sets all devices
to the minimum MPS supported by all devices.

"peer2peer" sets all devices to an MPS of 128 bytes, the only size which
must be supported by all compliant PCI Express devices, to ensure that
all devices can directly communicate with each other and with devices that
are hotplugged later.

A test for the sandbox environment is also added by advertising a
larger supported Maximum Payload Size in the swapcase device and
verifying that the pci_mps command can read and configure the MPS.

Stephen Carlson (3):
  cmd: pci: Add command to set MPS of all PCIe devices
  drivers: pci: sandbox: Add stub sandbox PCI MPS support
  test: Add test for new command pci_mps

 MAINTAINERS                   |   6 ++
 cmd/Kconfig                   |  10 ++
 cmd/Makefile                  |   1 +
 cmd/pci_mps.c                 | 167 ++++++++++++++++++++++++++++++++++
 configs/sandbox_defconfig     |   1 +
 drivers/misc/swap_case.c      |   3 +
 include/pci.h                 |   7 ++
 test/py/tests/test_pci_mps.py |  13 +++
 8 files changed, 208 insertions(+)
 create mode 100644 cmd/pci_mps.c
 create mode 100644 test/py/tests/test_pci_mps.py

-- 
2.25.1

[PATCH 1/3] cmd: pci: Add command to set MPS of all PCIe devices

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Enable tuning of the PCI Express MPS (Maximum Payload Size) of
each device. The Maximum Read Request Size is not altered.

The SAFE method uses the largest MPS value supported by all devices in the
system for each device. This method is the same algorithm as used by Linux
pci=pcie_bus_safe.

The PEER2PEER method sets all devices to the minimal (128 byte) MPS, which
allows hot plug of devices later that might only support the minimum size,
and ensures compatibility of DMA between two devices on the bus.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
---
 cmd/Kconfig   |  10 +++
 cmd/Makefile  |   1 +
 cmd/pci_mps.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/pci.h |   7 +++
 4 files changed, 185 insertions(+)
 create mode 100644 cmd/pci_mps.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index dc0446e02e..632c5c45db 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1390,6 +1390,16 @@ config CMD_PCI
 	  peripherals. Sub-commands allow bus enumeration, displaying and
 	  changing configuration space and a few other features.
 
+config CMD_PCI_MPS
+	bool "pci_mps - Configure PCI device MPS"
+	depends on PCI
+	help
+	  Enables PCI Express Maximum Packet Size (MPS) tuning. This
+	  command configures the PCI Express MPS of each endpoint to the
+	  largest value supported by all devices below the root complex.
+	  The Maximum Read Request Size will not be altered. This method is
+	  the same algorithm as used by Linux pci=pcie_bus_safe.
+
 config CMD_PINMUX
 	bool "pinmux - show pins muxing"
 	depends on PINCTRL
diff --git a/cmd/Makefile b/cmd/Makefile
index 7b6ff73186..3365634843 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_CMD_PART) += part.o
 obj-$(CONFIG_CMD_PCAP) += pcap.o
 ifdef CONFIG_PCI
 obj-$(CONFIG_CMD_PCI) += pci.o
+obj-$(CONFIG_CMD_PCI_MPS) += pci_mps.o
 endif
 obj-$(CONFIG_CMD_PINMUX) += pinmux.o
 obj-$(CONFIG_CMD_PMC) += pmc.o
diff --git a/cmd/pci_mps.c b/cmd/pci_mps.c
new file mode 100644
index 0000000000..c67d007838
--- /dev/null
+++ b/cmd/pci_mps.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2022 Microsoft Corporation <www.microsoft.com>
+ * Stephen Carlson <stcarlso@linux.microsoft.com>
+ *
+ * PCI Express Maximum Packet Size (MPS) configuration
+ */
+
+#include <common.h>
+#include <bootretry.h>
+#include <cli.h>
+#include <command.h>
+#include <console.h>
+#include <dm.h>
+#include <init.h>
+#include <asm/processor.h>
+#include <asm/io.h>
+#include <pci.h>
+
+#define PCI_MPS_SAFE 0
+#define PCI_MPS_PEER2PEER 1
+
+static int pci_mps_find_safe(struct udevice *bus, unsigned int *min_mps,
+			     unsigned int *n)
+{
+	struct udevice *dev;
+	int res = 0, addr;
+	unsigned int mpss;
+	u32 regval;
+
+	if (!min_mps || !n)
+		return -EINVAL;
+
+	for (device_find_first_child(bus, &dev);
+	     dev;
+	     device_find_next_child(&dev)) {
+		addr = dm_pci_find_capability(dev, PCI_CAP_ID_EXP);
+		if (addr <= 0)
+			continue;
+
+		res = dm_pci_read_config32(dev, addr + PCI_EXP_DEVCAP,
+					   &regval);
+		if (res != 0)
+			return res;
+		mpss = (unsigned int)(regval & PCI_EXP_DEVCAP_PAYLOAD);
+		*n += 1;
+		if (mpss < *min_mps)
+			*min_mps = mpss;
+	}
+
+	return res;
+}
+
+static int pci_mps_set_bus(struct udevice *bus, unsigned int target)
+{
+	struct udevice *dev;
+	u32 mpss, target_mps = (u32)(target << 5);
+	u16 mps;
+	int res = 0, addr;
+	int busnum = dev_seq(bus);
+
+	for (device_find_first_child(bus, &dev);
+	     dev && res == 0;
+	     device_find_next_child(&dev)) {
+		struct pci_child_plat *pplat;
+
+		addr = dm_pci_find_capability(dev, PCI_CAP_ID_EXP);
+		if (addr <= 0)
+			continue;
+
+		res = dm_pci_read_config32(dev, addr + PCI_EXP_DEVCAP,
+					   &mpss);
+		if (res != 0)
+			return res;
+
+		/* Do not set device above its maximum MPSS */
+		mpss = (mpss & PCI_EXP_DEVCAP_PAYLOAD) << 5;
+		if (target_mps < mpss)
+			mps = (u16)target_mps;
+		else
+			mps = (u16)mpss;
+		res = dm_pci_clrset_config16(dev, addr + PCI_EXP_DEVCTL,
+					     PCI_EXP_DEVCTL_PAYLOAD, mps);
+	}
+
+	return res;
+}
+
+/*
+ * Sets the MPS of each PCI Express device to the specified policy.
+ */
+static int pci_mps_set(int policy)
+{
+	struct udevice *bus;
+	int i, res = 0;
+	/* 0 = 128B, min value for hotplug */
+	unsigned int mps = 0;
+
+	if (policy == PCI_MPS_SAFE) {
+		unsigned int min_mps = PCI_EXP_DEVCAP_PAYLOAD_4096B, n = 0;
+
+		/* Find maximum MPS supported by all devices */
+		for (i = 0;
+		     uclass_get_device_by_seq(UCLASS_PCI, i, &bus) == 0 &&
+		     res == 0;
+		     i++)
+			res = pci_mps_find_safe(bus, &min_mps, &n);
+
+		/* If no devices were found, do not reconfigure */
+		if (n == 0)
+			return res;
+		mps = min_mps;
+	}
+
+	/* This message is checked by the sandbox test */
+	printf("Setting MPS of all devices to %uB\n", 128U << mps);
+	for (i = 0;
+	     uclass_get_device_by_seq(UCLASS_PCI, i, &bus) == 0 && res == 0;
+	     i++)
+		res = pci_mps_set_bus(bus, mps);
+
+	return res;
+}
+
+/*
+ * PCI MPS tuning commands
+ *
+ * Syntax:
+ *	pci_mps safe
+ *	pci_mps peer2peer
+ */
+static int do_pci_mps(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+	char cmd = 'u';
+	int ret = 0;
+
+	if (argc > 1)
+		cmd = argv[1][0];
+
+	switch (cmd) {
+	case 's':		/* safe */
+		ret = pci_mps_set(PCI_MPS_SAFE);
+		break;
+	case 'p':		/* peer2peer/hotplug */
+		ret = pci_mps_set(PCI_MPS_PEER2PEER);
+		break;
+	default:		/* usage, help */
+		goto usage;
+	}
+
+	return ret;
+usage:
+	return CMD_RET_USAGE;
+}
+
+/***************************************************/
+
+#ifdef CONFIG_SYS_LONGHELP
+static char pci_mps_help_text[] =
+	"safe\n"
+	"    - Set PCI Express MPS of all devices to safe values\n"
+	"pci_mps peer2peer\n"
+	"    - Set PCI Express MPS of all devices to support hotplug and peer-to-peer DMA\n";
+#endif
+
+U_BOOT_CMD(pci_mps, 2, 0, do_pci_mps,
+	   "configure PCI Express MPS", pci_mps_help_text);
diff --git a/include/pci.h b/include/pci.h
index c55d6107a4..2f5eb30b83 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -360,6 +360,13 @@
 #define   PCI_EXP_TYPE_PCIE_BRIDGE 0x8	/* PCI/PCI-X to PCIe Bridge */
 #define PCI_EXP_DEVCAP		4	/* Device capabilities */
 #define  PCI_EXP_DEVCAP_FLR	0x10000000 /* Function Level Reset */
+#define  PCI_EXP_DEVCAP_PAYLOAD 0x0007	/* Max payload size supported */
+#define  PCI_EXP_DEVCAP_PAYLOAD_128B 0x0000 /* 128 Bytes */
+#define  PCI_EXP_DEVCAP_PAYLOAD_256B 0x0001 /* 256 Bytes */
+#define  PCI_EXP_DEVCAP_PAYLOAD_512B 0x0002 /* 512 Bytes */
+#define  PCI_EXP_DEVCAP_PAYLOAD_1024B 0x0003 /* 1024 Bytes */
+#define  PCI_EXP_DEVCAP_PAYLOAD_2048B 0x0004 /* 2048 Bytes */
+#define  PCI_EXP_DEVCAP_PAYLOAD_4096B 0x0005 /* 4096 Bytes */
 #define PCI_EXP_DEVCTL		8	/* Device Control */
 #define  PCI_EXP_DEVCTL_PAYLOAD	0x00e0	/* Max_Payload_Size */
 #define  PCI_EXP_DEVCTL_PAYLOAD_128B 0x0000 /* 128 Bytes */
-- 
2.25.1

[PATCH 2/3] drivers: pci: sandbox: Add stub sandbox PCI MPS support

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Reports the sandbox swapcase PCI Express device to support a 256 byte
Maximum Payload Size for MPS tuning tests.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
---
 drivers/misc/swap_case.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c
index 7093ad1cd4..ee5c12bd0a 100644
--- a/drivers/misc/swap_case.c
+++ b/drivers/misc/swap_case.c
@@ -165,6 +165,9 @@ static int sandbox_swap_case_read_config(const struct udevice *emul,
 	case PCI_CAP_ID_EXP_OFFSET + PCI_CAP_LIST_NEXT:
 		*valuep = PCI_CAP_ID_MSIX_OFFSET;
 		break;
+	case PCI_CAP_ID_EXP_OFFSET + PCI_EXP_DEVCAP:
+		*valuep = PCI_EXP_DEVCAP_PAYLOAD_256B;
+		break;
 	case PCI_CAP_ID_MSIX_OFFSET:
 		if (sandbox_swap_case_use_ea(emul))
 			*valuep = (PCI_CAP_ID_EA_OFFSET << 8) | PCI_CAP_ID_MSIX;
-- 
2.25.1

[PATCH 3/3] test: Add test for new command pci_mps

[stcarlso]

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Adds a test for the new pci_mps command to ensure that it can set the
Maximum Payload Size (MPS) of all devices to 256 bytes in the sandbox
environment. Enables the pci_mps command in the sandbox environment so
that this test can be run.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
---
 MAINTAINERS                   |  6 ++++++
 configs/sandbox_defconfig     |  1 +
 test/py/tests/test_pci_mps.py | 13 +++++++++++++
 3 files changed, 20 insertions(+)
 create mode 100644 test/py/tests/test_pci_mps.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 3e8e193ecc..83948b6aa7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1242,6 +1242,12 @@ M:	Heiko Schocher <hs@denx.de>
 S:	Maintained
 F:	drivers/pci/pci_mpc85xx.c
 
+PCI MPS
+M:	Stephen Carlson <stcarlso@linux.microsoft.com>
+S:	Maintained
+F:	cmd/pci_mps.c
+F:	test/py/tests/test_pci_mps.py
+
 POWER
 M:	Jaehoon Chung <jh80.chung@samsung.com>
 S:	Maintained
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 34c342b6f5..cd6bb8e2c4 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -77,6 +77,7 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_MUX=y
 CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
+CONFIG_CMD_PCI_MPS=y
 CONFIG_CMD_READ=y
 CONFIG_CMD_REMOTEPROC=y
 CONFIG_CMD_SPI=y
diff --git a/test/py/tests/test_pci_mps.py b/test/py/tests/test_pci_mps.py
new file mode 100644
index 0000000000..8d33490f9f
--- /dev/null
+++ b/test/py/tests/test_pci_mps.py
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+import pytest
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_pci')
+@pytest.mark.buildconfigspec('cmd_pci_mps')
+def test_pci_mps_safe(u_boot_console):
+    """Tests that PCI MPS can be set to safe mode and uses 256 bytes."""
+
+    response = u_boot_console.run_command('pci e; pci_mps s')
+    expected_response = 'to 256B'
+    assert(expected_response in response)
-- 
2.25.1

Re: [PATCH 3/3] test: Add test for new command pci_mps

[Simon Glass]

Hi Stephen,

On Tue, 7 Mar 2023 at 16:33, <stcarlso@linux.microsoft.com> wrote:
>
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
>
> Adds a test for the new pci_mps command to ensure that it can set the
> Maximum Payload Size (MPS) of all devices to 256 bytes in the sandbox
> environment. Enables the pci_mps command in the sandbox environment so
> that this test can be run.
>
> Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
> ---
>  MAINTAINERS                   |  6 ++++++
>  configs/sandbox_defconfig     |  1 +
>  test/py/tests/test_pci_mps.py | 13 +++++++++++++
>  3 files changed, 20 insertions(+)
>  create mode 100644 test/py/tests/test_pci_mps.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3e8e193ecc..83948b6aa7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1242,6 +1242,12 @@ M:       Heiko Schocher <hs@denx.de>
>  S:     Maintained
>  F:     drivers/pci/pci_mpc85xx.c
>
> +PCI MPS
> +M:     Stephen Carlson <stcarlso@linux.microsoft.com>
> +S:     Maintained
> +F:     cmd/pci_mps.c
> +F:     test/py/tests/test_pci_mps.py
> +
>  POWER
>  M:     Jaehoon Chung <jh80.chung@samsung.com>
>  S:     Maintained
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 34c342b6f5..cd6bb8e2c4 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -77,6 +77,7 @@ CONFIG_CMD_MMC=y
>  CONFIG_CMD_MUX=y
>  CONFIG_CMD_OSD=y
>  CONFIG_CMD_PCI=y
> +CONFIG_CMD_PCI_MPS=y
>  CONFIG_CMD_READ=y
>  CONFIG_CMD_REMOTEPROC=y
>  CONFIG_CMD_SPI=y
> diff --git a/test/py/tests/test_pci_mps.py b/test/py/tests/test_pci_mps.py
> new file mode 100644
> index 0000000000..8d33490f9f
> --- /dev/null
> +++ b/test/py/tests/test_pci_mps.py
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +import pytest
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_pci')
> +@pytest.mark.buildconfigspec('cmd_pci_mps')
> +def test_pci_mps_safe(u_boot_console):
> +    """Tests that PCI MPS can be set to safe mode and uses 256 bytes."""
> +
> +    response = u_boot_console.run_command('pci e; pci_mps s')
> +    expected_response = 'to 256B'
> +    assert(expected_response in response)
> --

Can you please use C for this test? See [1]. For an example, see test/dm/acpi.c

Regards,
Simon

[1] https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html#test-types