Re: [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 11/06/2025 15:35, Ayan Kumar Halder wrote:
> Introduce pr_t typedef which is a structure having the prbar and prlar members,
> each being structured as the registers of the AArch32 Armv8-R architecture.
>
> Also, define MPU_REGION_RES0 to 0 as there are no reserved 0 bits beyond the
> BASE or LIMIT bitfields in prbar or prlar respectively.
>
> In pr_of_addr(), enclose prbar and prlar arm64 specific bitfields with
> appropriate macros. So, that this function can be later reused for arm32 as
> well.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

> ---
> Changes from v1 :-
>
> 1. Preserve pr_t typedef in arch specific files.
>
> 2. Fix typo.
>
> v2 :-
>
> 1. Change CONFIG_ARM64 to CONFIG_ARM_64 to enclose arm64 specific bitfields for
> prbar and prlar registers in pr_of_addr().
>
>   xen/arch/arm/include/asm/arm32/mpu.h | 34 ++++++++++++++++++++++++++--
>   xen/arch/arm/mpu/mm.c                |  4 ++++
>   2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/arm32/mpu.h 
> b/xen/arch/arm/include/asm/arm32/mpu.h
> index f0d4d4055c..0a6930b3a0 100644
> --- a/xen/arch/arm/include/asm/arm32/mpu.h
> +++ b/xen/arch/arm/include/asm/arm32/mpu.h
> @@ -5,10 +5,40 @@
>   
>   #ifndef __ASSEMBLY__
>   
> +/*
> + * Unlike arm64, there are no reserved 0 bits beyond base and limit bitfield in
> + * prbar and prlar registers respectively.
> + */
> +#define MPU_REGION_RES0       0x0
> +
> +/* Hypervisor Protection Region Base Address Register */
> +typedef union {
> +    struct {
> +        unsigned int xn:1;       /* Execute-Never */
> +        unsigned int ap_0:1;     /* Access Permission AP[0] */
> +        unsigned int ro:1;       /* Access Permission AP[1] */
> +        unsigned int sh:2;       /* Shareability */
> +        unsigned int res0:1;
> +        unsigned int base:26;    /* Base Address */
> +    } reg;
> +    uint32_t bits;
> +} prbar_t;
> +
> +/* Hypervisor Protection Region Limit Address Register */
> +typedef union {
> +    struct {
> +        unsigned int en:1;     /* Region enable */
> +        unsigned int ai:3;     /* Memory Attribute Index */
> +        unsigned int res0:2;
> +        unsigned int limit:26; /* Limit Address */
> +    } reg;
> +    uint32_t bits;
> +} prlar_t;
> +
>   /* MPU Protection Region */
>   typedef struct {
> -    uint32_t prbar;
> -    uint32_t prlar;
> +    prbar_t prbar;
> +    prlar_t prlar;
>   } pr_t;
>   
>   #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 86fbe105af..3d37beab57 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -167,7 +167,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int 
> flags)
>       /* Build up value for PRBAR_EL2. */
>       prbar = (prbar_t) {
>           .reg = {
> +#ifdef CONFIG_ARM_64
>               .xn_0 = 0,
> +#endif
>               .xn = PAGE_XN_MASK(flags),
>               .ap_0 = 0,
>               .ro = PAGE_RO_MASK(flags)
> @@ -206,7 +208,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int 
> flags)
>       /* Build up value for PRLAR_EL2. */
>       prlar = (prlar_t) {
>           .reg = {
> +#ifdef CONFIG_ARM_64
>               .ns = 0,        /* Hyp mode is in secure world */
> +#endif
>               .ai = attr_idx,
>               .en = 1,        /* Region enabled */
>           }};
--
Julien Grall