在2022年6月1日六月 上午3:22，Bjorn Helgaas写道：
> On Sat, Apr 30, 2022 at 04:48:44PM +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).
>
> This seems essentially similar to ks_pcie_quirk() [1]. Why are they
> different, and why do you need no_inc_mrrs, when keystone doesn't?
>
> Or *does* keystone need it and we just haven't figured that out yet?
> Are all callers of pcie_set_readrq() vulnerable to issues there?
Yes actually keystone may need to set this flag as well.
I think Huacai missed a point in his commit message about why he removed
the process of walking through the bus and set proper MRRS. That’s
because Loongson’s firmware will set proper MRRS and the only thing
that Kernel needs to do is leave it as is. no_inc_mrrs is introduced for
this purpose.
In keystone’s case it’s likely that their firmware won’t do such thing, so
their workaround shouldn’t be removed.
And no_inc_mrrs should be set for them to prevent device drivers modifying
MRRS afterwards.
Thanks
- Jiaxun
>
> Whatever we do should be as uniform as possible across host
> controllers.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pci-keystone.c?id=v5.18#n528
>
--
- Jiaxun
