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.
- Ayan
