Re: [PATCH v3] PCI: rcar-gen4: Limit Max_Read_Request_Size and Max_Payload_Size to 256 Bytes
From: Marek Vasut
Date: Thu Sep 03 2026 - 16:54:37 EST
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