Re: [PATCH 3/9] arm/mpu: Introduce MPU memory region map structure

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

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.
>
>
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
>
> Introduce pr_t typedef which is a structure having the prbar
> and prlar members, each being structured as the registers of
> the aarch64 armv8-r architecture.
>
> Introduce the array 'xen_mpumap' that will store a view of
> the content of the MPU regions.
>
> Introduce MAX_MPU_REGIONS macro that uses the value of
> NUM_MPU_REGIONS_MASK just for clarity, because using the
> latter as number of elements of the xen_mpumap array might
> be misleading.
>
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>   xen/arch/arm/Makefile                |  1 +
>   xen/arch/arm/include/asm/arm64/mpu.h | 38 ++++++++++++++++++++++++++++
>   xen/arch/arm/mpu/Makefile            |  1 +
>   xen/arch/arm/mpu/mm.c                | 23 +++++++++++++++++
>   4 files changed, 63 insertions(+)
>   create mode 100644 xen/arch/arm/mpu/Makefile
>   create mode 100644 xen/arch/arm/mpu/mm.c
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 43ab5e8f2550..fb0948f067bd 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -1,6 +1,7 @@
>   obj-$(CONFIG_ARM_32) += arm32/
>   obj-$(CONFIG_ARM_64) += arm64/
>   obj-$(CONFIG_MMU) += mmu/
> +obj-$(CONFIG_MPU) += mpu/
>   obj-$(CONFIG_ACPI) += acpi/
>   obj-$(CONFIG_HAS_PCI) += pci/
>   ifneq ($(CONFIG_NO_PLAT),y)
> diff --git a/xen/arch/arm/include/asm/arm64/mpu.h 
> b/xen/arch/arm/include/asm/arm64/mpu.h
> index f8a029f1a135..38dbf5b2255a 100644
> --- a/xen/arch/arm/include/asm/arm64/mpu.h
> +++ b/xen/arch/arm/include/asm/arm64/mpu.h
> @@ -13,6 +13,44 @@
>   #define NUM_MPU_REGIONS_SHIFT   8
>   #define NUM_MPU_REGIONS         (_AC(1, UL) << NUM_MPU_REGIONS_SHIFT)
>   #define NUM_MPU_REGIONS_MASK    (NUM_MPU_REGIONS - 1)
> +
> +#define MAX_MPU_REGIONS         NUM_MPU_REGIONS_MASK
Not really a comment, but a note. The above needs to be common between 
arm64 and arm32.
I had moved this in


   "[PATCH 3/5] xen/arm: mpu: Move some of the definitions to common
   file" .

Depending on whose patch goes in first, this code needs to be moved in 
the initial patch so that there is less code movement in the latter patch.
> +
> +#ifndef __ASSEMBLY__
> +
> +/* Protection Region Base Address Register */
> +typedef union {
> +    struct __packed {
> +        unsigned long xn:2;       /* Execute-Never */
> +        unsigned long ap:2;       /* Acess Permission */
> +        unsigned long sh:2;       /* Sharebility */
> +        unsigned long base:46;    /* Base Address */
> +        unsigned long pad:12;
> +    } reg;
> +    uint64_t bits;
> +} prbar_t;
> +
> +/* Protection Region Limit Address Register */
> +typedef union {
> +    struct __packed {
> +        unsigned long en:1;     /* Region enable */
> +        unsigned long ai:3;     /* Memory Attribute Index */
> +        unsigned long ns:1;     /* Not-Secure */
> +        unsigned long res:1;    /* Reserved 0 by hardware */
> +        unsigned long limit:46; /* Limit Address */
> +        unsigned long pad:12;
> +    } reg;
> +    uint64_t bits;
> +} prlar_t;
> +
> +/* MPU Protection Region */
> +typedef struct {
> +    prbar_t prbar;
> +    prlar_t prlar;
> +} pr_t;
> +
> +#endif /* __ASSEMBLY__ */
> +
>   #endif /* __ARM64_MPU_H__ */
>
>   /*
> diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile
> new file mode 100644
> index 000000000000..b18cec483671
> --- /dev/null
> +++ b/xen/arch/arm/mpu/Makefile
> @@ -0,0 +1 @@
> +obj-y += mm.o
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> new file mode 100644
> index 000000000000..3ca609ff80cc
> --- /dev/null
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * xen/arch/arm/mpu/mm.c
> + *
> + * MPU-based memory managment code for Armv8-R AArch64.
> + *
> + * Copyright (C) 2023 Arm Ltd.
> + *
> + */
> +
> +#include <asm/arm64/mpu.h>
Can we just include <asm/mpu.h> ?

The arm64/arm32 specific includes will occur in the header file.

> +
> +/* EL2 Xen MPU memory region mapping table. */
> +pr_t xen_mpumap[MAX_MPU_REGIONS];
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.34.1
- Ayan
>