Re: [PATCH 3/5] PCI: Improve the mrrs quirk for LS7A

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2021/5/15 11:49, Huacai Chen 写道:
Hi, Krzysztof and Bjorn

I will improve my spelling, and others see below.

On Fri, May 14, 2021 at 11:40 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
On Fri, May 14, 2021 at 04:00:23PM +0800, Huacai Chen wrote:
In new revision of LS7A, some pcie ports support larger value than 256,
but their mrrs values are not dectectable. And moreover, the current
loongson_mrrs_quirk() cannot avoid devices increasing its mrrs after
pci_enable_device(). So the only possible way is configure mrrs of all
devices in BIOS, and add a pci dev flag (PCI_DEV_FLAGS_NO_INCREASE_MRRS)
to stop the increasing mrrs operations.
s/mrrs/MRRS/
s/dectectable/detectable/

This doesn't make sense to me.  MRRS "sets the maximum Read Request
size for the Function as a Requester" (PCIe r5.0, sec 7.5.3.4).

The MRRS in the Device Control register is a 3-bit RW field (or a RO
field with value 000b).  If it's RW, software is allowed to write any
3-bit value to it.  There is no "maximum allowed value" for software
to detect.

The value software writes is only a *limit* on the Read Request size.
The hardware is never obligated to generate Read Requests of that max
size.  If software writes 101b (4096 byte max size), and the hardware
only supports generating 128-byte Read Requests, there's no issue.
It's perfectly fine for the hardware to generate 128-byte requests.

Apparently something bad happens if software writes something "too
large" to MRRS?  What actually happens?

If the problem is that the device generates a large Read Request and
in response, it receives a data TLP that is larger than it can handle,
that sounds like an MPS issue, not an MRRS issue.

Based on the existing loongson_mrrs_quirk(), it looks like this is a
long-standing issue.  I'm sorry I missed this when reviewing the
driver in the first place.  This all needs a much better explanation
of what the real problem is.  The "h/w limitation of 256 bytes maximum
read request size" comment just doesn't make sense from the spec point
of view.

I do know that Linux uses MRRS and MPS in ... highly unusual ways, and
maybe we're tripping over that somehow.  If so, we need to figure out
exactly how so we can make Linux's use of MPS and MRRS better overall.
I have discussed with Shuai Huang (the main designer of LS7A), he said
that some devices (such as Realtek 8169) usually write a large value
to MRRS in its driver. And that usually larger than LS7A bridge can
handle, the quirk in this patch is avoid device driver to increase
MRRS (and BIOS initialize a reasonable value at power on stage).

Based on my experiments on LS2K which have a similar issue, I guess the
problem is Loongson's AXI bus failed to accept reading burst larger then
certain size.

The larger MRRS is legal for PCIe controller but not for upstream bus.
So when you write the value to MRRS register the controller will still
generate oversized TLP and then send illegal response to AXI bus. Thus
we need to limit MRRS in software to avoid such situation.

I'm not a loongson employee so it might be wrong.

Thanks.

- Jiaxun


Huacai




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux