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

[Ayan Kumar Halder <ayankuma@amd.com>]

Hi Luca,

On 29/04/2025 16:20, 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@arm.com>
> ---
> v4 changes:
>   - update helper comments
>   - rename XN_EL2_ENABLED to PRBAR_EL2_XN_ENABLED
>   - protected pr_of_xenaddr() with #ifdef Arm64 until Arm32
>     can build with it
> ---
>   xen/arch/arm/include/asm/arm64/mpu.h | 11 +++++
>   xen/arch/arm/include/asm/mpu.h       |  4 ++
>   xen/arch/arm/include/asm/mpu/mm.h    | 10 ++++
>   xen/arch/arm/mpu/mm.c                | 68 ++++++++++++++++++++++++++++
>   4 files changed, 93 insertions(+)
>
> diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h
> index b27fccd77550..39233b43a5aa 100644
> --- a/xen/arch/arm/include/asm/arm64/mpu.h
> +++ b/xen/arch/arm/include/asm/arm64/mpu.h
> @@ -3,6 +3,17 @@
>   #ifndef __ARM_ARM64_MPU_H__
>   #define __ARM_ARM64_MPU_H__
>
> +/*
> + * 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 PRBAR_EL2_XN_ENABLED  0x2
> +
>   #ifndef __ASSEMBLY__
>
>   /* Protection Region Base Address Register */
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 0e0a7f05ade9..7b82f10d336b 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -24,6 +24,10 @@
>   #define NUM_MPU_REGIONS_MASK    (NUM_MPU_REGIONS - 1)
>   #define MAX_MPU_REGION_NR       255
>
> +/* Access permission attributes. */
> +/* Read/Write at EL2, No Access at EL1/EL0. */
> +#define AP_RW_EL2 0x0
> +
>   #ifndef __ASSEMBLY__
>
>   #ifdef CONFIG_ARM_64
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
> index e2235e568e81..296fe74c5d61 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -75,6 +75,16 @@ extern void read_protection_region(pr_t *pr_read, uint8_t sel);
>    */
>   extern void write_protection_region(const pr_t *pr_write, uint8_t sel);
>
> +/*
> + * Creates a pr_t structure describing a protection region.
> + *
> + * @base: base address as base of the protection region.
> + * @limit: exclusive address as limit of the protection region.
> + * @attr: attribute index for the memory type.
> + * @return: pr_t structure describing a protection region.
> + */
> +extern pr_t pr_of_xenaddr(paddr_t base, paddr_t limit, unsigned int attr_idx);
> +
>   #endif /* __ARM_MPU_MM_H__ */
>
>   /*
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 40ccf99adc94..2e0aeb486ff8 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -9,6 +9,7 @@
>   #include <xen/types.h>
>   #include <asm/mpu.h>
>   #include <asm/mpu/mm.h>
> +#include <asm/page.h>
>   #include <asm/sysregs.h>
>
>   struct page_info *frame_table;
> @@ -151,6 +152,73 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
>           BUG(); /* Can't happen */
>       }
>   }
> +
> +pr_t pr_of_xenaddr(paddr_t base, paddr_t limit, unsigned int attr_idx)
> +{
> +    prbar_t prbar;
> +    prlar_t prlar;
> +    pr_t region;
> +
> +    /* Build up value for PRBAR_EL2. */
> +    prbar = (prbar_t) {
> +        .reg = {
> +            .ap = AP_RW_EL2,      /* Read/Write at EL2, no access at EL1/EL0. */
> +            .xn = PRBAR_EL2_XN_ENABLED,   /* No need to execute outside .text */
> +        }};
> +
> +    switch ( attr_idx )
> +    {
> +    case MT_NORMAL_NC:
> +        /*
> +         * ARM ARM: Overlaying the shareability attribute (DDI
> +         * 0406C.b B3-1376 to 1377)
> +         *
> +         * A memory region with a resultant memory type attribute of normal,
> +         * and a resultant cacheability attribute of Inner non-cacheable,
> +         * outer non-cacheable, must have a resultant shareability attribute
> +         * of outer shareable, otherwise shareability is UNPREDICTABLE.
> +         *
> +         * On ARMv8 sharability is ignored and explicitly treated as outer
> +         * shareable for normal inner non-cacheable, outer non-cacheable.
> +         */
> +        prbar.reg.sh = LPAE_SH_OUTER;
> +        break;
> +    case MT_DEVICE_nGnRnE:
> +    case MT_DEVICE_nGnRE:
> +        /*
> +         * Shareability is ignored for non-normal memory, Outer is as
> +         * good as anything.
> +         *
> +         * On ARMv8 sharability is ignored and explicitly treated as outer
> +         * shareable for any device memory type.
> +         */
> +        prbar.reg.sh = LPAE_SH_OUTER;
> +        break;
> +    default:
> +        /* Xen mappings are SMP coherent */
> +        prbar.reg.sh = LPAE_SH_INNER;
> +    }
> +
> +    /* 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 */
> +        }};
> +
> +    /* Build up MPU memory region. */
> +    region = (pr_t) {
> +        .prbar = prbar,
> +        .prlar = prlar,
> +    };
> +
> +    /* Set base address and limit address. */
> +    pr_set_base(&region, base);
> +    pr_set_limit(&region, limit);
> +
> +    return region;
> +}
>   #endif
>
>   void __init setup_mm(void)
> --
> 2.34.1
>
>
- Ayan