On 2/16/22 12:50, Pali Rohár wrote:
Hi,
In case the controller is transitioning to L1 in rcar_pcie_config_access(),
any read/write access to PCIECDR triggers asynchronous external abort. This
is because the transition to L1 link state must be manually finished by the
driver. The PCIe IP can transition back from L1 state to L0 on its own.
The current asynchronous external abort hook implementation restarts
the instruction which finally triggered the fault, which can be a
different instruction than the read/write instruction which started
the faulting access. Usually the instruction which finally triggers
the fault is one which has some data dependency on the result of the
read/write. In case of read, the read value after fixup is undefined,
while a read value of faulting read should be all Fs.
It is possible to enforce the fault using 'isb' instruction placed
right after the read/write instruction which started the faulting
access. Add custom register accessors which perform the read/write
followed immediately by 'isb'.
This way, the fault always happens on the 'isb' and in case of read,
which is located one instruction before the 'isb', it is now possible
to fix up the return value of the read in the asynchronous external
abort hook and make that read return all Fs.
I'm looking at this again and I do not think that this is reliable.
Are you still running into any problems on your hardware with these
patches applied ?
Asynchronous aborts are by definition asynchronous. Placing isb looks
like a hack to decrease probability that asynchronous abort would be
triggered at wrong time.
That is exactly what this patch fixes, the ISB enforces the async
exception at the right moment so it can be fixed up in the fixup handler
(thanks Arnd).
Marek: Cannot you change the code to trigger proper synchronous abort
for this operation? If this is ARMv7 system, what about trying to change
memory mapping where is the accessing address to strongly-ordered?
Writing to strongly-ordered ARMv7 mapping could not report asynchronous
aborts anymore, but I'm not sure.
No, last time I tried tweaking the mapping, that didn't lead to sync aborts.
Marek: Are you sure that also ldr instruction is causing asynchronous
abort? This is strange as normally load from memory mapped config space
could not finish earlier than data from config space are fetched.
Normally these load instructions should cause synchronous abort on data
errors.
I am positive LDR triggers async abort on this hardware, since that's
how this problem was triggered by Geert.
[...]
