Re: [PATCH v3 4/6] xen/arm: mpu: Create boot-time MPU protection regions

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 10/10/2024 15:03, Ayan Kumar Halder wrote:
> Define enable_boot_cpu_mm() for the AArch64-V8R system.
>
> Like boot-time page table in MMU system, we need a boot-time MPU protection
> region configuration in MPU system so Xen can fetch code and data from normal
> memory.
>
> To do this, Xen maps the following sections of the binary as separate regions
> (with permissions) :-
> 1. Text (Read only at EL2, execution is permitted)
> 2. RO data (Read only at EL2)
> 3. RO after init data and RW data (Read/Write at EL2)
> 4. Init Text (Read only at EL2, execution is permitted)
> 5. Init data and BSS (Read/Write at EL2)
>
> Before creating a region, we check if the count exceeds the number defined in
> MPUIR_EL2. If so, then the boot fails.
>
> Also we check if the region is empty or not. IOW, if the start and end address
> of a section is the same, we skip mapping the region.
>
> To map a region, Xen uses the PRBAR_EL2, PRLAR_EL2 and PRSELR_EL2 registers.
> One can refer to ARM DDI 0600B.a ID062922 G1.3  "General System Control
> Registers", to get the definitions of these registers. Also, refer to G1.2
> "Accessing MPU memory region registers", the following
>
> ```
> The MPU provides two register interfaces to program the MPU regions:
> - Access to any of the MPU regions via PRSELR_ELx, PRBAR<n>_ELx, and
> PRLAR<n>_ELx.
> ```
>
> We use the above mechanism for mapping sections to MPU memory regions.
>
> MPU specific registers are defined in
> xen/arch/arm/include/asm/arm64/mpu/sysregs.h.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Changes from :-
>
> v1 - 1. Instead of mapping a (XEN_START_ADDRESS + 2MB) as a single MPU region,
> we have separate MPU regions for different parts of the Xen binary. The reason
> being different regions will nned different permissions (as mentioned in the
> linker script).
>
> 2. Introduced a label (__init_data_begin) to mark the beginning of the init data
> section.
>
> 3. Moved MPU specific register definitions to mpu/sysregs.h.
>
> 4. Fixed coding style issues.
>
> 5. Included page.h in mpu/head.S as page.h includes sysregs.h.
> I haven't seen sysregs.h included directly from head.S or mmu/head.S.
> (Outstanding comment not addressed).
>
> v2 - 1. Extracted "enable_mpu()" in a separate patch.
>
> 2. Removed alignment for limit address.
>
> 3. Merged some of the sections for preparing the early boot regions.
>
> 4. Checked for the max limit of MPU regions before creating a new region.
>
> 5. Checked for empty regions.
>
>   xen/arch/arm/Makefile                        |   1 +
>   xen/arch/arm/arm64/mpu/Makefile              |   1 +
>   xen/arch/arm/arm64/mpu/head.S                | 130 +++++++++++++++++++
>   xen/arch/arm/include/asm/arm64/mpu/sysregs.h |  27 ++++
>   xen/arch/arm/include/asm/mm.h                |   2 +
>   xen/arch/arm/include/asm/mpu/arm64/mm.h      |  22 ++++
>   xen/arch/arm/include/asm/mpu/mm.h            |  20 +++
>   xen/arch/arm/xen.lds.S                       |   1 +
>   8 files changed, 204 insertions(+)
>   create mode 100644 xen/arch/arm/arm64/mpu/Makefile
>   create mode 100644 xen/arch/arm/arm64/mpu/head.S
>   create mode 100644 xen/arch/arm/include/asm/arm64/mpu/sysregs.h
>   create mode 100644 xen/arch/arm/include/asm/mpu/arm64/mm.h
>   create mode 100644 xen/arch/arm/include/asm/mpu/mm.h
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 7792bff597..aebccec63a 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/
This change is incorrect. The correct change should have been in :-

--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,5 +1,6 @@
 obj-y += lib/
 obj-$(CONFIG_MMU) += mmu/
+obj-$(CONFIG_MPU) += mpu/

 obj-y += cache.o

I will wait for comments from reviewers before re-spinning the patch.

- Ayan