Hi Ayan,
Apologies but I can’t do a full review yet,
>>> +
>>> +/* 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.
Cheers,
Luca
