Re: [PATCH 6/9] arm/mpu: Provide a constructor for pr_t type

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 4 Mar 2025, at 11:45, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 28/02/2025 16:18, Luca Fancellu wrote:
>> CAUTION: This message has originated from an External Source. Please use 
>> proper judgment and caution when opening attachments, clicking links, or 
>> responding to this email.
>> 
>> 
>> Provide a function that creates a pr_t object from a memory
>> range and some attributes.
>> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>> ---
>>  xen/arch/arm/include/asm/arm64/mpu.h | 15 ++++++
>>  xen/arch/arm/include/asm/mpu/mm.h    |  3 ++
>>  xen/arch/arm/mpu/mm.c                | 73 ++++++++++++++++++++++++++++
>>  3 files changed, 91 insertions(+)
>> 
>> diff --git a/xen/arch/arm/include/asm/arm64/mpu.h 
>> b/xen/arch/arm/include/asm/arm64/mpu.h
>> index 3a09339818a0..dce77da60110 100644
>> --- a/xen/arch/arm/include/asm/arm64/mpu.h
>> +++ b/xen/arch/arm/include/asm/arm64/mpu.h
>> @@ -17,6 +17,21 @@
>> 
>>  #define MAX_MPU_REGIONS         NUM_MPU_REGIONS_MASK
>> 
>> +/* Access permission attributes. */
>> +/* Read/Write at EL2, No Access at EL1/EL0. */
>> +#define AP_RW_EL2 0x0
> This is common as well between arm64 and arm32.
>> +
>> +/*
>> + * Excute never.
>> + * Stage 1 EL2 translation regime.
>> + * XN[1] determines whether execution of the instruction fetched from the 
>> MPU
>> + * memory region is permitted.
>> + * Stage 2 EL1/EL0 translation regime.
>> + * XN[0] determines whether execution of the instruction fetched from the 
>> MPU
>> + * memory region is permitted.
>> + */
>> +#define XN_ENABLED     0x2
> 
> This seems incorrect.
> 
> As per ARM DDI 0600A.d ID120821, G1.3.19 PRBAR<n>_EL2 (armv8 R64 supplement)
> 
> 0b00 Execution of instructions fetched from the region is permitted.
> 
> 0b01 Execution of instructions fetched from the region is not permitted.
> 
> This holds true for 32-bit as well (except for the fact that XN is denoted by 
> 1-bit).
> 
> So the correct definition is
> 
> #define XN_ENABLED     0x0
> 
> And this is common between arm32/64 , thus it can be moved to common file as 
> well.

Ok maybe my understanding is wrong, but from the specifications:

XN, bits [1:0]

Execute Never. For
• Stage 1 EL2 translation regime and
• Stage 2 EL1&0 translation regime when FEAT_XNX is not implemented

XN[1] determines whether execution of the instructions fetched from the MPU 
memory region is
permitted. In this case, XN[0] is RES0

For stage 2 EL1&0 translation regime when FEAT_XNX is implemented, the behavior 
of XN[1:0]
is same as that defined by VMSAv8-64 for EL1&0 stage 2 translation table 
XN[1:0],bits[54:53]
field in Armv8-A architecture.
0b00 Execution of instructions fetched from the region is permitted.
0b01 Execution of instructions fetched from the region is not permitted.
The reset behavior of this field is:
• On a Warm reset, this field resets to an architecturally UNKNOWN value.

So my understanding is that Stage 1 EL2 regime XN=1 means execution of 
instructions fetched
from the region is not permitted, and when that bit is 1, the XN[0] is RES0

Cheers,
Luca