Hi Julien/Luca/all,
On 04/09/2024 19:38, Ayan Kumar Halder wrote:
On 04/09/2024 19:14, Luca Fancellu wrote:
Hi Ayan,
Hi Luca,
Apologies but I can’t do a full review yet,
No worries. :)
+
+/* MPU normal memory attributes. */
+#define PRBAR_NORMAL_MEM 0x30 /* SH=11 AP=00 XN=00 */
+#define PRLAR_NORMAL_MEM 0x0f /* NS=0 ATTR=111 EN=1 */
+
+.macro write_pr, sel, prbar, prlar
+ msr PRSELR_EL2, \sel
+ dsb sy
I am not sure I understand why this is a dsb rather than isb. Can
you clarify?
ISB is not needed here as the memory protection hasn't been
activated yet. The above instruction just selects the memory region
and the below two instructions sets the base address and limit for
that memory region. After the three instructions, we need an ISB so
that the memory protection takes into affect for further instruction
fetches.
However, a DSB is needed here as the below two instructions depend
on this. So, we definitely want this instruction to complete.
Further, refer to the note in ARM DDI 0600A.d ID120821, C1.7.1
"Protection region attributes"
0.
```Writes to MPU registers are only guaranteed to be visible
following a Context synchronization event and DSB operation.```
Thus, I infer that DSB is necessary here.
I think this was a mistake from the author of this patch, in my
opinion there should be an ISB
after setting PRSELR_ELx, to enforce a synchronisation before writing
PR{B,L}AR_ELx which
depends on the value written on PRSELR.
That synchronisation is enforced by DSB.
From
https://developer.arm.com/documentation/dui0489/c/arm-and-thumb-instructions/miscellaneous-instructions/dmb--dsb--and-isb
,
```Data Synchronization Barrier acts as a special kind of memory
barrier. No instruction in program order after this instruction
executes until this instruction completes.
...
Instruction Synchronization Barrier flushes the pipeline in the
processor, ```
Why should we flush the instruction pipeline after setting PRSELR_ELx
? It does not have any impact on instruction fetch.
After the discussion on matrix, I agree that we need an 'isb' here to
ensure that there is no instruction reordering.
Thanks for the explanation.
- Ayan
