Re: [PATCH] PCI: brcmstb: Avoid downstream access during link training

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

 





On 8/7/2023 12:28 AM, Pali Rohár wrote:
On Monday 07 August 2023 09:13:58 Pali Rohár wrote:
On Sunday 06 August 2023 19:48:59 Florian Fainelli wrote:
On 8/6/2023 2:43 PM, Pali Rohár wrote:
On Sunday 06 August 2023 06:44:50 Lukas Wunner wrote:
The Broadcom Set Top Box PCIe controller signals an Asynchronous SError
Interrupt

This is little incorrect wording. PCIe controller cannot send Async
SError, this is ARMv8 specific thing. In this case PCIe controller is
connected to ARM core via AXI bus and on PCIe transaction timeout it
sends AXI Slave Error, which then ARMv8 core reports to kernel as Async
SError interrupt.

That is indeed a better way to explain the issue. FWIW, on BCM2711 the PCIe
core connects via SCB and then AXI towards the ARMv8 CPU, does not change a
thing about your paragraph.

Ok, it is better describe the issue correctly.


The proper fix is to configure PCIe controller to never send AXI Slave
Error and neither AXI Decode Error (to prevent SErrors at all). For
example Synopsys PCIe controllers have proprietary hidden configuration
bits for enabling/disabling this AXI error reporting behavior.

That does not exist with the version of the block present in BCM2711
unfortunately.

I was expecting such answer. I think that we have been discussing this
issue privately more months ago.


Or second option is to access affected memory from the ARMv8 core via
synchronous operations and map memory as nGnRnE. Then ARMv8 core reports
AXI Slave Error as Synchronous Abort Exception which you can catch,
examine that was caused on PCIe memory region and fabricate all-ones
response. But the second option is not available for some licensed ARMv8
Cortex cores (e.g. A53) as they do not implement nE (non Early Write
Acknowledgement) memory mapping correctly.

BCM2711 uses Cortex-A72 do these cores still not implement nE correctly? Do
you have a reference backing up that claim (not disputing it, just curious).

I remember that I read this in Cortex-A53 documentation. Let me find it.

It is here:
ARM Cortex-A53 MPCore Processor Technical Reference Manual r0p3 - Support for v8 memory types:
https://developer.arm.com/documentation/ddi0500/e/level-1-memory-system/support-for-v8-memory-types

"nGnRnE - Treated the same as nGnRE inside a Cortex-A53 processor"

So really, disabling Early-acknowledge has no effect on A53.


The patch below does not fix the issue at all, instead it opens a new
race condition that if link state is changed _after_ the check and
_before_ accessing config space.

Fair enough, but in the situation you describe there is not much that can be
done anyway so we might as well deal with a narrowed window?

Unless there is hidden/proprietary/undocumented bits for PCIe controller
to disable generating AXI errors, or ability to use only synchronous
access to affected memory from ARM core (e.g. by nE) there is really not
much what can be done here. I can just say that this is badly designed HW.

One more thing, if I remember correctly, ARMv8 specifies that Syndrome
register which delivers SError has some bits reserved for optional
feature - target AXI slave id which sent AXI Error. A53 does not
implement it and I highly doubt that other licensed ARM cores have them.
But maybe you could check A72. But even with that I'm really not sure if
it could be possible to catch SError, detect that it was sent by AXI
slave corresponding to PCIe controller and ignore it.

Thanks for reviewing.
--
Florian

One more idea. Does platform have some DMA controller? If yes, maybe it
could be possible to completely avoid access to that affected memory
from ARM core and instead access its content via DMA controller? DMA
controller could be better and does not signal slave errors as SError to
ARM core.

Looks like I never got back to you on that, yes 2711 has 15 programmable DMA controllers (drivers/dma/bcm2835-dma.c) which could be an option here, though since the PCIe bridge is supposed to be transparent, I am not sure how we could hook it in into memory cycles?

Would not that require faulting (from a MMU perspective) on the PCIe outbound window to then go and kick off the DMA engine to transfer data around?
--
Florian

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[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