Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
@ 2026-08-21  2:05 Marek Vasut
  2026-08-21  2:19 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Marek Vasut @ 2026-08-21  2:05 UTC (permalink / raw)
  To: linux-pci
  Cc: Marek Vasut, stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc

R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
Max_Payload_Size (MPS). PCIe specification indicates that the MPS
must not exceed minimum MPS of any element along the packet path.
Force limit Max_Payload_Size to at most 256 Bytes for each device
connected to this PCIe controller.

R-Car Gen4 Reference Manual, chapter 104.4.8 Usage notes for
MRRS (Max Read Request Size) states:
  Please set "Max Read Request Size" to 128 bytes or 256 bytes.
  If "Max Read Request Size" is set to anything other than the
  above, the transferred data will not match the expected value.
This limitation also seems the apply to devices issuing MRd TLP.
This limitation can be triggered by using non-HMB NVMe SSD with
Max_Read_Request_Size 512 Bytes, for example Crucial P5 Plus.
Any write into the SSD (MRd TLP issued by the SSD) longer than
256 Bytes wraps around at 256 Byte boundary, and the same data
are written into the SSD starting at offset 0 and at 256 Bytes.
Force limit Max_Read_Request_Size to at most 256 Bytes for each
device connected to this PCIe controller to avoid this behavior.

An non-HMB (Host Memory Buffer) NVMe SSD can be identified using
the following command. Affected SSD reports 'hmpre' field as 0:
"
$ nvme id-ctrl /dev/nvme0 | grep hmpre
hmpre     : 0
"

The symptom is a read from the SSD which wraps around at 256 Byte
boundary. The test for this symptom can be implemented by writing
512 Byte of random data into the SSD and reading the data back. If
the read back data repeat after 256 Bytes, the device is affected.
"
$ dd if=/dev/urandom of=/tmp/data.bin bs=256 count=2 ; \
  dd if=/tmp/data.bin of=/dev/nvme0n1 bs=256 count=2 ; \
  dd if=/dev/nvme0n1 bs=256 count=2 of=/tmp/readback.bin
"

Expected data:
"
$ hexdump -vC /tmp/data.bin
00000000  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca
00000010  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b
...
00000100  7a ce 3c b2 e1 d5 d9 11  88 63 10 59 76 3c dc 32 <-- random
00000110  72 32 2a 7d a3 e1 aa 13  7c da 58 a1 7b 21 11 50 <-- data
"

Faulty readback, collected without this change in place:
"
$ hexdump -vC /tmp/readback.bin
00000000  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca <---.
00000010  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b <-. |
...                                                          | |
00000100  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca <-:-+- repeated
00000110  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b <-+--- data
     ^^^
      |
      '--- Repeat starts at offset 0x100 = 256 Bytes
"

Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Koichiro Den <den@valinux.co.jp>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Dispose of the reprogramming of MPS/MRRS altogether
    - Dispose of the entire fixup quirk
    - Replace both with bridge enable_device hook
    - Limit MPS/MRRS along the entire packet path to follow
      MRRS limitation requirement
V3: - Set no_inc_mrrs to prevent periperhal drivers from increasing MRRS
      over the limit set by this PCIe controller driver
    - Warn on MPS > 256 Bytes
    - Recheck updated documentation and update code comments to match
---
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 51 +++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index fbe465a29068f..e38cde2257bb4 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -411,6 +411,54 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
 	return ret;
 }
 
+static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
+					struct pci_dev *dev)
+{
+	/*
+	 * R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
+	 * Max_Payload_Size (MPS). PCIe specification indicates that the MPS
+	 * must not exceed minimum MPS of any element along the packet path.
+	 * The controller reports Max_Payload_Size_Supported (MPSS) 256 Bytes
+	 * for header type 0 and 128 Bytes for header type 1. The PCIe core
+	 * will not allow MPS to be set higher than MPSS, warn here in case
+	 * something went very wrong in the core.
+	 *
+	 * For details, refer to chapter "104.1.1 Features" in either of:
+	 * R-Car S4 R19UH0161EJ0140 Rev.1.40 Jul. 31, 2026 or
+	 * R-Car V4H R19UH0186EJ0140 Rev.1.40 Aug. 7, 2026 or
+	 * R-Car V4M R19UH0217EJ0110 Rev.1.10 Jun. 30, 2026.
+	 */
+	WARN_ON(pcie_get_mps(dev) > 256);
+
+	/*
+	 * R-Car Gen4 Reference Manual, chapter 104.4.8 Usage notes for
+	 * MRRS (Max Read Request Size) states:
+	 *   Please set "Max Read Request Size" to 128 bytes or 256 bytes.
+	 *   If "Max Read Request Size" is set to anything other than the
+	 *   above, the transferred data will not match the expected value.
+	 * This limitation also seems the apply to devices issuing MRd TLP.
+	 * This limitation can be triggered by using non-HMB NVMe SSD with
+	 * Max_Read_Request_Size 512 Bytes, for example Crucial P5 Plus.
+	 * Any write into the SSD (MRd TLP issued by the SSD) longer than
+	 * 256 Bytes wraps around at 256 Byte boundary, and the same data
+	 * are written into the SSD starting at offset 0 and at 256 Bytes.
+	 * Force limit Max_Read_Request_Size to at most 256 Bytes for each
+	 * device connected to this PCIe controller to avoid this behavior.
+	 *
+	 * For details, refer to aforementioned chapter in either of:
+	 * R-Car S4 R19UH0161EJ0140 Rev.1.40 Jul. 31, 2026 or
+	 * R-Car V4H R19UH0186EJ0140 Rev.1.40 Aug. 7, 2026 or
+	 * R-Car V4M R19UH0217EJ0110 Rev.1.10 Jun. 30, 2026.
+	 */
+	bridge->no_inc_mrrs = 1;
+	if (pcie_get_readrq(dev) > 256) {
+		pci_info(dev, "Limiting MRRS to 256 bytes\n");
+		pcie_set_readrq(dev, 256);
+	}
+
+	return 0;
+}
+
 /* Host mode */
 static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 {
@@ -418,6 +466,9 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
 	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
 	int ret;
 
+	if (pp->bridge)
+		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
+
 	gpiod_set_value_cansleep(dw->pe_rst, 1);
 
 	ret = rcar_gen4_pcie_common_init(rcar);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-08-21  2:05 [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes Marek Vasut
@ 2026-08-21  2:19 ` sashiko-bot
  2026-09-01 13:35 ` Manivannan Sadhasivam
  2026-09-03 17:32 ` Bjorn Helgaas
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-21  2:19 UTC (permalink / raw)
  To: Marek Vasut; +Cc: wsa+renesas, linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] System denial of service via kernel panic. Untrusted hardware input maps directly into a `WARN_ON` assertion.
--

--- Patch [1]: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes ---
commit 79fc893274978b76ee056f33053d2211fce68461
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes

This commit forces the Max_Payload_Size and Max_Read_Request_Size to at most
256 bytes for devices connected to the R-Car Gen4 PCIe controller. This works
around a hardware limitation where longer reads wrap around at the 256-byte
boundary.

Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -411,6 +411,54 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
>  	return ret;
>  }
>  
> +static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
> +					struct pci_dev *dev)
> +{
> +	/*
> +	 * R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> +	 * Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> +	 * must not exceed minimum MPS of any element along the packet path.
> +	 * The controller reports Max_Payload_Size_Supported (MPSS) 256 Bytes
> +	 * for header type 0 and 128 Bytes for header type 1. The PCIe core
> +	 * will not allow MPS to be set higher than MPSS, warn here in case
> +	 * something went very wrong in the core.
> +	 *
> +	 * For details, refer to chapter "104.1.1 Features" in either of:
> +	 * R-Car S4 R19UH0161EJ0140 Rev.1.40 Jul. 31, 2026 or
> +	 * R-Car V4H R19UH0186EJ0140 Rev.1.40 Aug. 7, 2026 or
> +	 * R-Car V4M R19UH0217EJ0110 Rev.1.10 Jun. 30, 2026.
> +	 */
> +	WARN_ON(pcie_get_mps(dev) > 256);

[Severity: Medium]
Is it safe to use WARN_ON() with values read directly from the device's
configuration space?

A malfunctioning or malicious PCIe endpoint could spoof its configuration
space and return a value for PCI_EXP_DEVCTL where the payload field is
greater than 256 bytes. When rcar_gen4_pcie_enable_device() calls
pcie_get_mps(dev), it issues a configuration space read directly to the
endpoint device.

If a system has panic_on_warn enabled, feeding this untrusted hardware input
into WARN_ON() could allow a connected PCIe device to intentionally trigger a
kernel panic, resulting in a system-wide denial of service.

Would it be safer to handle this unexpected condition by using pci_warn()
and dynamically adjusting the payload size, rather than risking a system
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821020636.111719-1-marek.vasut+renesas@mailbox.org?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-08-21  2:05 [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes Marek Vasut
  2026-08-21  2:19 ` sashiko-bot
@ 2026-09-01 13:35 ` Manivannan Sadhasivam
  2026-09-03 17:32 ` Bjorn Helgaas
  2 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2026-09-01 13:35 UTC (permalink / raw)
  To: linux-pci, Marek Vasut
  Cc: stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc


On Fri, 21 Aug 2026 04:05:51 +0200, Marek Vasut wrote:
> R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> must not exceed minimum MPS of any element along the packet path.
> Force limit Max_Payload_Size to at most 256 Bytes for each device
> connected to this PCIe controller.
> 
> R-Car Gen4 Reference Manual, chapter 104.4.8 Usage notes for
> MRRS (Max Read Request Size) states:
>   Please set "Max Read Request Size" to 128 bytes or 256 bytes.
>   If "Max Read Request Size" is set to anything other than the
>   above, the transferred data will not match the expected value.
> This limitation also seems the apply to devices issuing MRd TLP.
> This limitation can be triggered by using non-HMB NVMe SSD with
> Max_Read_Request_Size 512 Bytes, for example Crucial P5 Plus.
> Any write into the SSD (MRd TLP issued by the SSD) longer than
> 256 Bytes wraps around at 256 Byte boundary, and the same data
> are written into the SSD starting at offset 0 and at 256 Bytes.
> Force limit Max_Read_Request_Size to at most 256 Bytes for each
> device connected to this PCIe controller to avoid this behavior.
> 
> [...]

Applied, thanks!

[1/1] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
      commit: d1f8ada0815e247b52e5f9d4ff3a8bc9e4080bb9

Best regards,
-- 
மணிவண்ணன் சதாசிவம்



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-08-21  2:05 [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes Marek Vasut
  2026-08-21  2:19 ` sashiko-bot
  2026-09-01 13:35 ` Manivannan Sadhasivam
@ 2026-09-03 17:32 ` Bjorn Helgaas
  2026-09-03 18:51   ` Marek Vasut
  2 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2026-09-03 17:32 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-pci, stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc, Ziyao Li, Rong Zhang,
	Huacai Chen

[+cc Ziyao, Rong, Huacai for similar Loongson MRRS issue]

On Fri, Aug 21, 2026 at 04:05:51AM +0200, Marek Vasut wrote:
> R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> must not exceed minimum MPS of any element along the packet path.
> Force limit Max_Payload_Size to at most 256 Bytes for each device
> connected to this PCIe controller.

IIUC the PCI core already enforces this limit, and what this patch
does is double-check that this limit is observed with this check,
right?

  +	WARN_ON(pcie_get_mps(dev) > 256);

More below.

> R-Car Gen4 Reference Manual, chapter 104.4.8 Usage notes for
> MRRS (Max Read Request Size) states:
>   Please set "Max Read Request Size" to 128 bytes or 256 bytes.
>   If "Max Read Request Size" is set to anything other than the
>   above, the transferred data will not match the expected value.
> This limitation also seems the apply to devices issuing MRd TLP.
> This limitation can be triggered by using non-HMB NVMe SSD with
> Max_Read_Request_Size 512 Bytes, for example Crucial P5 Plus.
> Any write into the SSD (MRd TLP issued by the SSD) longer than
> 256 Bytes wraps around at 256 Byte boundary, and the same data
> are written into the SSD starting at offset 0 and at 256 Bytes.
> Force limit Max_Read_Request_Size to at most 256 Bytes for each
> device connected to this PCIe controller to avoid this behavior.
> 
> An non-HMB (Host Memory Buffer) NVMe SSD can be identified using
> the following command. Affected SSD reports 'hmpre' field as 0:
> "
> $ nvme id-ctrl /dev/nvme0 | grep hmpre
> hmpre     : 0
> "
> 
> The symptom is a read from the SSD which wraps around at 256 Byte
> boundary. The test for this symptom can be implemented by writing
> 512 Byte of random data into the SSD and reading the data back. If
> the read back data repeat after 256 Bytes, the device is affected.
> "
> $ dd if=/dev/urandom of=/tmp/data.bin bs=256 count=2 ; \
>   dd if=/tmp/data.bin of=/dev/nvme0n1 bs=256 count=2 ; \
>   dd if=/dev/nvme0n1 bs=256 count=2 of=/tmp/readback.bin
> "
> 
> Expected data:
> "
> $ hexdump -vC /tmp/data.bin
> 00000000  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca
> 00000010  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b
> ...
> 00000100  7a ce 3c b2 e1 d5 d9 11  88 63 10 59 76 3c dc 32 <-- random
> 00000110  72 32 2a 7d a3 e1 aa 13  7c da 58 a1 7b 21 11 50 <-- data
> "
> 
> Faulty readback, collected without this change in place:
> "
> $ hexdump -vC /tmp/readback.bin
> 00000000  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca <---.
> 00000010  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b <-. |
> ...                                                          | |
> 00000100  97 81 b7 3b 0e 38 2b 4d  a7 d3 e0 47 ff c2 4b ca <-:-+- repeated
> 00000110  c1 85 98 f0 4a ac 03 a0  3b ab f3 19 44 dd 06 8b <-+--- data
>      ^^^
>       |
>       '--- Repeat starts at offset 0x100 = 256 Bytes
> "
> 
> Fixes: 0d0c551011df ("PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Koichiro Den <den@valinux.co.jp>
> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: - Dispose of the reprogramming of MPS/MRRS altogether
>     - Dispose of the entire fixup quirk
>     - Replace both with bridge enable_device hook
>     - Limit MPS/MRRS along the entire packet path to follow
>       MRRS limitation requirement
> V3: - Set no_inc_mrrs to prevent periperhal drivers from increasing MRRS
>       over the limit set by this PCIe controller driver
>     - Warn on MPS > 256 Bytes
>     - Recheck updated documentation and update code comments to match
> ---
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 51 +++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fbe465a29068f..e38cde2257bb4 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -411,6 +411,54 @@ static int rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
>  	return ret;
>  }
>  
> +static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
> +					struct pci_dev *dev)
> +{
> +	/*
> +	 * R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> +	 * Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> +	 * must not exceed minimum MPS of any element along the packet path.
> +	 * The controller reports Max_Payload_Size_Supported (MPSS) 256 Bytes
> +	 * for header type 0 and 128 Bytes for header type 1. The PCIe core
> +	 * will not allow MPS to be set higher than MPSS, warn here in case
> +	 * something went very wrong in the core.
> +	 *
> +	 * For details, refer to chapter "104.1.1 Features" in either of:
> +	 * R-Car S4 R19UH0161EJ0140 Rev.1.40 Jul. 31, 2026 or
> +	 * R-Car V4H R19UH0186EJ0140 Rev.1.40 Aug. 7, 2026 or
> +	 * R-Car V4M R19UH0217EJ0110 Rev.1.10 Jun. 30, 2026.
> +	 */
> +	WARN_ON(pcie_get_mps(dev) > 256);
> +
> +	/*
> +	 * R-Car Gen4 Reference Manual, chapter 104.4.8 Usage notes for
> +	 * MRRS (Max Read Request Size) states:
> +	 *   Please set "Max Read Request Size" to 128 bytes or 256 bytes.
> +	 *   If "Max Read Request Size" is set to anything other than the
> +	 *   above, the transferred data will not match the expected value.
> +	 * This limitation also seems the apply to devices issuing MRd TLP.
> +	 * This limitation can be triggered by using non-HMB NVMe SSD with
> +	 * Max_Read_Request_Size 512 Bytes, for example Crucial P5 Plus.
> +	 * Any write into the SSD (MRd TLP issued by the SSD) longer than
> +	 * 256 Bytes wraps around at 256 Byte boundary, and the same data
> +	 * are written into the SSD starting at offset 0 and at 256 Bytes.
> +	 * Force limit Max_Read_Request_Size to at most 256 Bytes for each
> +	 * device connected to this PCIe controller to avoid this behavior.
> +	 *
> +	 * For details, refer to aforementioned chapter in either of:
> +	 * R-Car S4 R19UH0161EJ0140 Rev.1.40 Jul. 31, 2026 or
> +	 * R-Car V4H R19UH0186EJ0140 Rev.1.40 Aug. 7, 2026 or
> +	 * R-Car V4M R19UH0217EJ0110 Rev.1.10 Jun. 30, 2026.
> +	 */
> +	bridge->no_inc_mrrs = 1;
> +	if (pcie_get_readrq(dev) > 256) {
> +		pci_info(dev, "Limiting MRRS to 256 bytes\n");
> +		pcie_set_readrq(dev, 256);
> +	}

It would be nice if all the platforms that need no_inc_mrrs could
apply it the same way, but I assume you saw loongson_mrrs_quirk() and
loongson_set_min_mrrs_quirk() in the process of finding no_inc_mrrs,
and chose a different implementation strategy for some reason, e.g.,
this way doesn't have to include device IDs for all the Root Ports?

Maybe we should rework no_inc_mrrs in such a way that drivers could
set a max MRRS in the struct pci_host_bridge and make
pcie_write_mrrs() and pcie_set_readrq() pay attention to it?  That
might let us get rid of the FIXUP approach.

> +	return 0;
> +}
> +
>  /* Host mode */
>  static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  {
> @@ -418,6 +466,9 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
>  	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
>  	int ret;
>  
> +	if (pp->bridge)
> +		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
> +
>  	gpiod_set_value_cansleep(dw->pe_rst, 1);
>  
>  	ret = rcar_gen4_pcie_common_init(rcar);
> -- 
> 2.53.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-09-03 17:32 ` Bjorn Helgaas
@ 2026-09-03 18:51   ` Marek Vasut
  2026-09-03 20:27     ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2026-09-03 18:51 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc, Ziyao Li, Rong Zhang,
	Huacai Chen

Hello Bjorn,

On 9/3/26 7:32 PM, Bjorn Helgaas wrote:
> [+cc Ziyao, Rong, Huacai for similar Loongson MRRS issue]
> 
> On Fri, Aug 21, 2026 at 04:05:51AM +0200, Marek Vasut wrote:
>> R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
>> Max_Payload_Size (MPS). PCIe specification indicates that the MPS
>> must not exceed minimum MPS of any element along the packet path.
>> Force limit Max_Payload_Size to at most 256 Bytes for each device
>> connected to this PCIe controller.
> 
> IIUC the PCI core already enforces this limit, and what this patch
> does is double-check that this limit is observed with this check,
> right?

That is correct, this was changed in V3, I missed the commit message 
update, sorry.

Would you like me to respin the patch one more time with an updated 
commit message, or would you be willing to fix it up in tree ?

>    +	WARN_ON(pcie_get_mps(dev) > 256);
> 
> More below.


[...]

>> +	bridge->no_inc_mrrs = 1;
>> +	if (pcie_get_readrq(dev) > 256) {
>> +		pci_info(dev, "Limiting MRRS to 256 bytes\n");
>> +		pcie_set_readrq(dev, 256);
>> +	}
> 
> It would be nice if all the platforms that need no_inc_mrrs could
> apply it the same way, but I assume you saw loongson_mrrs_quirk() and
> loongson_set_min_mrrs_quirk() in the process of finding no_inc_mrrs,
> and chose a different implementation strategy for some reason, e.g.,
> this way doesn't have to include device IDs for all the Root Ports?

The loongson quirk won't work if the PCIe controller driver is built as 
a module, which the R-Car Gen4 PCIe driver can be, and in fact is often 
built as a module, because it depends on firmware which is loaded from 
filesystem.

If the controller driver is built as a module, then 
DECLARE_PCI_FIXUP_ENABLE() is not applied, the 
DECLARE_PCI_FIXUP_ENABLE() is applied only on boot and therefore only 
for built-in drivers.

I got burnt by DECLARE_PCI_FIXUP_ENABLE() in V1 of this patch.

However, there is also another part to this -- the 
rcar_gen4_pcie_enable_device() is called for every device on the bus and 
applies the MRRS limitation to every device on the bus that is 
downstream of the controller, not only the controller. This is necessary 
on this controller variant, else hardware like PCIe SSDs with MRRS 
higher than the controller break.

> Maybe we should rework no_inc_mrrs in such a way that drivers could
> set a max MRRS in the struct pci_host_bridge and make
> pcie_write_mrrs() and pcie_set_readrq() pay attention to it?  That
> might let us get rid of the FIXUP approach.
In light of the last paragraph above, that the MRRS has to be limited 
also on all devices downstream of this particular controller, I would 
like to ask -- does the Loongson controller have the same limitation or 
not ? If not, then I would argue this quirk should be isolated to this 
controller variant ; else, I am happy to start on the core patches.

[...]

Thank you for your help !

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-09-03 18:51   ` Marek Vasut
@ 2026-09-03 20:27     ` Bjorn Helgaas
  2026-09-03 20:43       ` Marek Vasut
  2026-09-04 17:42       ` Rong Zhang
  0 siblings, 2 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2026-09-03 20:27 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-pci, stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc, Ziyao Li, Rong Zhang,
	Huacai Chen

On Thu, Sep 03, 2026 at 08:51:06PM +0200, Marek Vasut wrote:
> Hello Bjorn,
> 
> On 9/3/26 7:32 PM, Bjorn Helgaas wrote:
> > [+cc Ziyao, Rong, Huacai for similar Loongson MRRS issue]
> > 
> > On Fri, Aug 21, 2026 at 04:05:51AM +0200, Marek Vasut wrote:
> > > R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> > > Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> > > must not exceed minimum MPS of any element along the packet path.
> > > Force limit Max_Payload_Size to at most 256 Bytes for each device
> > > connected to this PCIe controller.
> > 
> > IIUC the PCI core already enforces this limit, and what this patch
> > does is double-check that this limit is observed with this check,
> > right?
> 
> That is correct, this was changed in V3, I missed the commit message update,
> sorry.
> 
> Would you like me to respin the patch one more time with an updated commit
> message, or would you be willing to fix it up in tree ?

I fixed the commit log, no problem.

> >    +	WARN_ON(pcie_get_mps(dev) > 256);
> > 
> > More below.
> 
> 
> [...]
> 
> > > +	bridge->no_inc_mrrs = 1;
> > > +	if (pcie_get_readrq(dev) > 256) {
> > > +		pci_info(dev, "Limiting MRRS to 256 bytes\n");
> > > +		pcie_set_readrq(dev, 256);
> > > +	}
> > 
> > It would be nice if all the platforms that need no_inc_mrrs could
> > apply it the same way, but I assume you saw loongson_mrrs_quirk() and
> > loongson_set_min_mrrs_quirk() in the process of finding no_inc_mrrs,
> > and chose a different implementation strategy for some reason, e.g.,
> > this way doesn't have to include device IDs for all the Root Ports?
> 
> The loongson quirk won't work if the PCIe controller driver is built as a
> module, which the R-Car Gen4 PCIe driver can be, and in fact is often built
> as a module, because it depends on firmware which is loaded from filesystem.
> 
> If the controller driver is built as a module, then
> DECLARE_PCI_FIXUP_ENABLE() is not applied, the DECLARE_PCI_FIXUP_ENABLE() is
> applied only on boot and therefore only for built-in drivers.
> 
> I got burnt by DECLARE_PCI_FIXUP_ENABLE() in V1 of this patch.

Ouch, that does hurt.

> However, there is also another part to this -- the
> rcar_gen4_pcie_enable_device() is called for every device on the bus and
> applies the MRRS limitation to every device on the bus that is downstream of
> the controller, not only the controller. This is necessary on this
> controller variant, else hardware like PCIe SSDs with MRRS higher than the
> controller break.
> 
> > Maybe we should rework no_inc_mrrs in such a way that drivers could
> > set a max MRRS in the struct pci_host_bridge and make
> > pcie_write_mrrs() and pcie_set_readrq() pay attention to it?  That
> > might let us get rid of the FIXUP approach.
>
> In light of the last paragraph above, that the MRRS has to be limited also
> on all devices downstream of this particular controller, I would like to ask
> -- does the Loongson controller have the same limitation or not ? If not,
> then I would argue this quirk should be isolated to this controller variant
> ; else, I am happy to start on the core patches.

I don't know if we'll get a real answer for Loongson (there's no
maintainer listed for it, hint hint :)), but my guess is that it does
apply to all devices downstream of the Loongson controller.

I think MRRS is mostly interesting for DMA because MMIO from CPUs is
usually small sizes, far below the 128-byte or larger transfers that
devices may do.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-09-03 20:27     ` Bjorn Helgaas
@ 2026-09-03 20:43       ` Marek Vasut
  2026-09-04 17:42       ` Rong Zhang
  1 sibling, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2026-09-03 20:43 UTC (permalink / raw)
  To: Bjorn Helgaas, Nishanth Menon
  Cc: linux-pci, stable, Krzysztof Wilczyński, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc, Ziyao Li, Rong Zhang,
	Huacai Chen

Hello Bjorn,

On 9/3/26 10:27 PM, Bjorn Helgaas wrote:
> On Thu, Sep 03, 2026 at 08:51:06PM +0200, Marek Vasut wrote:
>> Hello Bjorn,
>>
>> On 9/3/26 7:32 PM, Bjorn Helgaas wrote:
>>> [+cc Ziyao, Rong, Huacai for similar Loongson MRRS issue]
>>>
>>> On Fri, Aug 21, 2026 at 04:05:51AM +0200, Marek Vasut wrote:
>>>> R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
>>>> Max_Payload_Size (MPS). PCIe specification indicates that the MPS
>>>> must not exceed minimum MPS of any element along the packet path.
>>>> Force limit Max_Payload_Size to at most 256 Bytes for each device
>>>> connected to this PCIe controller.
>>>
>>> IIUC the PCI core already enforces this limit, and what this patch
>>> does is double-check that this limit is observed with this check,
>>> right?
>>
>> That is correct, this was changed in V3, I missed the commit message update,
>> sorry.
>>
>> Would you like me to respin the patch one more time with an updated commit
>> message, or would you be willing to fix it up in tree ?
> 
> I fixed the commit log, no problem.

Thank you.

>>>     +	WARN_ON(pcie_get_mps(dev) > 256);
>>>
>>> More below.
>>
>>
>> [...]
>>
>>>> +	bridge->no_inc_mrrs = 1;
>>>> +	if (pcie_get_readrq(dev) > 256) {
>>>> +		pci_info(dev, "Limiting MRRS to 256 bytes\n");
>>>> +		pcie_set_readrq(dev, 256);
>>>> +	}
>>>
>>> It would be nice if all the platforms that need no_inc_mrrs could
>>> apply it the same way, but I assume you saw loongson_mrrs_quirk() and
>>> loongson_set_min_mrrs_quirk() in the process of finding no_inc_mrrs,
>>> and chose a different implementation strategy for some reason, e.g.,
>>> this way doesn't have to include device IDs for all the Root Ports?
>>
>> The loongson quirk won't work if the PCIe controller driver is built as a
>> module, which the R-Car Gen4 PCIe driver can be, and in fact is often built
>> as a module, because it depends on firmware which is loaded from filesystem.
>>
>> If the controller driver is built as a module, then
>> DECLARE_PCI_FIXUP_ENABLE() is not applied, the DECLARE_PCI_FIXUP_ENABLE() is
>> applied only on boot and therefore only for built-in drivers.
>>
>> I got burnt by DECLARE_PCI_FIXUP_ENABLE() in V1 of this patch.
> 
> Ouch, that does hurt.

I also mentioned this to TI a while back, because I think the 
pci-keystone.c has the same (module) issue. ( +CC Nishanth here too )

>> However, there is also another part to this -- the
>> rcar_gen4_pcie_enable_device() is called for every device on the bus and
>> applies the MRRS limitation to every device on the bus that is downstream of
>> the controller, not only the controller. This is necessary on this
>> controller variant, else hardware like PCIe SSDs with MRRS higher than the
>> controller break.
>>
>>> Maybe we should rework no_inc_mrrs in such a way that drivers could
>>> set a max MRRS in the struct pci_host_bridge and make
>>> pcie_write_mrrs() and pcie_set_readrq() pay attention to it?  That
>>> might let us get rid of the FIXUP approach.
>>
>> In light of the last paragraph above, that the MRRS has to be limited also
>> on all devices downstream of this particular controller, I would like to ask
>> -- does the Loongson controller have the same limitation or not ? If not,
>> then I would argue this quirk should be isolated to this controller variant
>> ; else, I am happy to start on the core patches.
> 
> I don't know if we'll get a real answer for Loongson (there's no
> maintainer listed for it, hint hint :)), but my guess is that it does
> apply to all devices downstream of the Loongson controller.
> 
> I think MRRS is mostly interesting for DMA because MMIO from CPUs is
> usually small sizes, far below the 128-byte or larger transfers that
> devices may do.

I agree with that, and DMA is what triggers the fault in my case.

Looking at the TI ks_pcie_quirk() FIXUP, I wonder if that might be a 
third instance of the same behavior. TI uses it to work around errata 
i2037 PCIe: PCI-Express May Corrupt Inbound Data [1] page 19 . But I now 
wonder, whether this behavior might be some common behavior of the DWC 
PCIe controller core ? Is there someone from Synopsys who might comment 
on that ?

[1] https://www.ti.com/lit/er/sprz452i/sprz452i.pdf

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
  2026-09-03 20:27     ` Bjorn Helgaas
  2026-09-03 20:43       ` Marek Vasut
@ 2026-09-04 17:42       ` Rong Zhang
  1 sibling, 0 replies; 8+ messages in thread
From: Rong Zhang @ 2026-09-04 17:42 UTC (permalink / raw)
  To: Bjorn Helgaas, Marek Vasut
  Cc: Jiaxun Yang, linux-pci, stable,
	Krzysztof Wilczy��ski, Bjorn Helgaas,
	Geert Uytterhoeven, Koichiro Den, Lorenzo Pieralisi, Magnus Damm,
	Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda,
	linux-kernel, linux-renesas-soc, Ziyao Li, Huacai Chen

Hi Bjorn, Marek,

Thanks for reaching me.

On Thu, 2026-09-03 at 15:27 -0500, Bjorn Helgaas wrote:
> On Thu, Sep 03, 2026 at 08:51:06PM +0200, Marek Vasut wrote:
> > Hello Bjorn,
> > 
> > On 9/3/26 7:32 PM, Bjorn Helgaas wrote:
> > > [+cc Ziyao, Rong, Huacai for similar Loongson MRRS issue]
> > > 
> > > On Fri, Aug 21, 2026 at 04:05:51AM +0200, Marek Vasut wrote:
> > > > R-Car Gen4 PCIe controller has a hardware limitation of 256 Bytes
> > > > Max_Payload_Size (MPS). PCIe specification indicates that the MPS
> > > > must not exceed minimum MPS of any element along the packet path.
> > > > Force limit Max_Payload_Size to at most 256 Bytes for each device
> > > > connected to this PCIe controller.
> > > 
> > > IIUC the PCI core already enforces this limit, and what this patch
> > > does is double-check that this limit is observed with this check,
> > > right?
> > 
> > That is correct, this was changed in V3, I missed the commit message update,
> > sorry.
> > 
> > Would you like me to respin the patch one more time with an updated commit
> > message, or would you be willing to fix it up in tree ?
> 
> I fixed the commit log, no problem.
> 
> > >    +	WARN_ON(pcie_get_mps(dev) > 256);
> > > 
> > > More below.
> > 
> > 
> > [...]
> > 
> > > > +	bridge->no_inc_mrrs = 1;
> > > > +	if (pcie_get_readrq(dev) > 256) {
> > > > +		pci_info(dev, "Limiting MRRS to 256 bytes\n");
> > > > +		pcie_set_readrq(dev, 256);
> > > > +	}
> > > 
> > > It would be nice if all the platforms that need no_inc_mrrs could
> > > apply it the same way, but I assume you saw loongson_mrrs_quirk() and
> > > loongson_set_min_mrrs_quirk() in the process of finding no_inc_mrrs,
> > > and chose a different implementation strategy for some reason, e.g.,
> > > this way doesn't have to include device IDs for all the Root Ports?
> > 
> > The loongson quirk won't work if the PCIe controller driver is built as a
> > module, which the R-Car Gen4 PCIe driver can be, and in fact is often built
> > as a module, because it depends on firmware which is loaded from filesystem.
> > 
> > If the controller driver is built as a module, then
> > DECLARE_PCI_FIXUP_ENABLE() is not applied, the DECLARE_PCI_FIXUP_ENABLE() is
> > applied only on boot and therefore only for built-in drivers.
> > 
> > I got burnt by DECLARE_PCI_FIXUP_ENABLE() in V1 of this patch.
> 
> Ouch, that does hurt.
> 
> > However, there is also another part to this -- the
> > rcar_gen4_pcie_enable_device() is called for every device on the bus and
> > applies the MRRS limitation to every device on the bus that is downstream of
> > the controller, not only the controller. This is necessary on this
> > controller variant, else hardware like PCIe SSDs with MRRS higher than the
> > controller break.
> > 
> > > Maybe we should rework no_inc_mrrs in such a way that drivers could
> > > set a max MRRS in the struct pci_host_bridge and make
> > > pcie_write_mrrs() and pcie_set_readrq() pay attention to it?  That
> > > might let us get rid of the FIXUP approach.
> > 
> > In light of the last paragraph above, that the MRRS has to be limited also
> > on all devices downstream of this particular controller, I would like to ask
> > -- does the Loongson controller have the same limitation or not ? If not,
> > then I would argue this quirk should be isolated to this controller variant
> > ; else, I am happy to start on the core patches.

ACK. I agreed that it should make our life easier.

> 
> I don't know if we'll get a real answer for Loongson (there's no
> maintainer listed for it, hint hint :)), 
> 

(+CC Jiaxun)

The driver was introduced by Jiaxun without updating MAINTAINERS. I guess
he'd be willing to be listed as a maintainer.

I don't work for Loongson, but I do maintain several MIPS-based Loongson
devices for the Golang community with my colleagues and personally own a
MIPS-based Loongson-LS3A4000-7A1000-NUC-SE mini PC. I do some PCIe
experiments on it from time to time for fun. So I am OK if someone wants
to list me as a maintainer or reviewer :)

> but my guess is that it does
> apply to all devices downstream of the Loongson controller.

I believe this is the case. Maybe Jiaxun can shed a light on it too.

Just checked the kmsg log from April, the firmware seemed to only clamp
MRRS for devices directly connected to the root ports. IOW, it seemed to
only clamp MRRS for the upstream port of a PCIe switch, so
loongson_set_min_mrrs_quirk() had to fix up downstream ports.

If you need more information I can do some more experiments with the PCIe
switch card.

Thanks,
Rong

> 
> I think MRRS is mostly interesting for DMA because MMIO from CPUs is
> usually small sizes, far below the 128-byte or larger transfers that
> devices may do.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-04 17:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21  2:05 [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes Marek Vasut
2026-08-21  2:19 ` sashiko-bot
2026-09-01 13:35 ` Manivannan Sadhasivam
2026-09-03 17:32 ` Bjorn Helgaas
2026-09-03 18:51   ` Marek Vasut
2026-09-03 20:27     ` Bjorn Helgaas
2026-09-03 20:43       ` Marek Vasut
2026-09-04 17:42       ` Rong Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox