Re: ECRC and Max Read Request Size

[Sinan Kaya <okaya@codeaurora.org>]

On 11/9/2015 2:47 PM, Bjorn Helgaas wrote:
> On Fri, Nov 06, 2015 at 10:39:51PM -0500, Sinan Kaya wrote:
>> On 11/6/2015 7:11 PM, Bjorn Helgaas wrote:
>>> That should definitely be fixed.  Do we enable ECRC unconditionally,
>>> or only when we boot with "pci=ecrc=on"?  The doc
>>> (Documentation/kernel-parameters.txt) suggests the latter.
>>>
>>> It would be ideal if we could try to turn on ECRC all the time by
>>> default for paths that support it.  I suppose there's some risk that
>>> we'd trip over broken hardware.  If that's an issue, we could consider
>>> turning it on all the time on machines newer than X (that's easy on
>>> x86, where we have a DMI BIOS date, but maybe not so easy on other
>>> arches).
>>
>> ecrc=on kernel option works like "force on" rather than just a simple on.
>
> What do you mean by "force on" as opposed to "simple on"?
>
>> I agree we should enable it by default.
>
> Part of the consideration is that enabling ECRC generation does
> increase the size of every TLP by 4 bytes, which reduces the link
> throughput slightly, so I'm not sure we want to blindly enable it.
>
> If I understand the current Linux code correctly, by default (booting
> with no parameter at all, or with "pci=ecrc=bios"), we don't touch
> ECRC at all -- we don't even look to see whether it's supported, and
> we don't enable or disable anything.
>
> With "pci=ecrc=on", we turn on ECRC generation (if supported) and ECRC
> checking (if supported) for every device, including those hot-added
> after boot.
>
> I do like the idea of letting platform firmware decide whether ECRC
> should be enabled by default, because then we don't have to decide on
> a policy in Linux.  Following the lead of the firmware lets OEMs
> decide whether ECRC is a feature they want to market and support.
>
> I think the current strategy could be improved for hot-added devices.
> In the default ("pci=ecrc=bios") case, we leave them alone, which
> means they won't check ECRC because the enable bits will be zero after
> power-up.  If the platform firmware enabled ECRC generation in the
> upstream Root Port, we're paying the cost of lower throughput without
> getting any benefit for it.
>
> It doesn't look like ECRC has any dependencies or ordering
> requirements, so we could also consider adding sysfs knobs to
> enable/disable ECRC at run-time.  I can imagine systems where we want
> ECRC on some paths, e.g., to a disk or network, but not on others,
> e.g., to a display.  Sysfs would be a way to allow that, although it
> *is* an administrative burden.
>

I had to go back and re-read the spec. My understanding was that you can 
always enable ECRC checking but you need to have every device ECRC check 
enabled in order to generate the ECRC downstream direction.

Now, by looking at the spec section you mentioned; this turns out to be 
not true. ECRC checking and generation can be enabled all the time.

I'll enable ECRC and run it against multiple cards to confirm. I was 
using PCIe Keysight Exerciser when I was testing ECRC. Since it is a 
PCIE exerciser, I might have forgotten to enable something in the GUI. 
I'll double check.

BTW, ECRC has negligible performance overhead.


> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project