[PATCH v2 0/4] Enable early bootup of Armv8-R AArch32 systems

[PATCH v2 0/4] Enable early bootup of Armv8-R AArch32 systems

[Ayan Kumar Halder]

Enabled early booting of Armv8-R AArch32 based systems.

This patch serie with
https://patchwork.kernel.org/project/xen-devel/list/?series=943154 and
https://gitlab.com/xen-project/people/ayankuma/xen/-/commit/b407e5713c619821f3f5a55da65d4f8f380c3778

will enable build on Xen on Armv8-R AArch32 for MPU.

The current patch serie does *not* conflict with the other serie. Thus, any of
them can be committed in any order.

Ayan Kumar Halder (3):
  xen/arm32: Initialize MM specific registers in enable_mmu
  xen/arm32: mpu: Create boot-time MPU protection regions
  xen/arm32: mpu: Implement a dummy enable_secondary_cpu_mm

Michal Orzel (1):
  xen/arm32: Allow ARM_PA_BITS_40 only if !MPU

 xen/arch/arm/Kconfig                  |   2 +-
 xen/arch/arm/arm32/Makefile           |   1 +
 xen/arch/arm/arm32/head.S             |  14 --
 xen/arch/arm/arm32/mmu/head.S         |  15 +++
 xen/arch/arm/arm32/mpu/Makefile       |   1 +
 xen/arch/arm/arm32/mpu/head.S         | 180 ++++++++++++++++++++++++++
 xen/arch/arm/include/asm/cpregs.h     |   4 +
 xen/arch/arm/include/asm/mpu/cpregs.h |  21 +++
 8 files changed, 223 insertions(+), 15 deletions(-)
 create mode 100644 xen/arch/arm/arm32/mpu/Makefile
 create mode 100644 xen/arch/arm/arm32/mpu/head.S
 create mode 100644 xen/arch/arm/include/asm/mpu/cpregs.h

-- 
2.25.1

[PATCH v2 1/4] xen/arm32: Initialize MM specific registers in enable_mmu

[Ayan Kumar Halder]

All the memory management specific registers are initialized in enable_mmu.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from -

v1 - HTCR and HMAIR{0,1} are not set together with the other memory management
registers in enable_mmu()

Similar changes are to be done in arm64 as well. I prefer to do that in a
separate patch so that all the arm32 changes are kept together in this series.

 xen/arch/arm/arm32/head.S     | 14 --------------
 xen/arch/arm/arm32/mmu/head.S | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 4ff5c220bc..50da179f81 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -218,20 +218,6 @@ cpu_init:
         add   pc, r1, r10                   /* Call paddr(init func) */
 
 cpu_init_done:
-        /* Set up memory attribute type tables */
-        mov_w r0, MAIR0VAL
-        mov_w r1, MAIR1VAL
-        mcr   CP32(r0, HMAIR0)
-        mcr   CP32(r1, HMAIR1)
-
-        /*
-         * Set up the HTCR:
-         * PT walks use Inner-Shareable accesses,
-         * PT walks are write-back, write-allocate in both cache levels,
-         * Full 32-bit address space goes through this table.
-         */
-        mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
-        mcr   CP32(r0, HTCR)
 
         mov_w r0, HSCTLR_SET
         mcr   CP32(r0, HSCTLR)
diff --git a/xen/arch/arm/arm32/mmu/head.S b/xen/arch/arm/arm32/mmu/head.S
index 1e2bbf0c82..8fa74bd556 100644
--- a/xen/arch/arm/arm32/mmu/head.S
+++ b/xen/arch/arm/arm32/mmu/head.S
@@ -279,6 +279,21 @@ ENDPROC(create_page_tables)
 enable_mmu:
         PRINT("- Turning on paging -\r\n")
 
+        /* Set up memory attribute type tables */
+        mov_w r0, MAIR0VAL
+        mov_w r1, MAIR1VAL
+        mcr   CP32(r0, HMAIR0)
+        mcr   CP32(r1, HMAIR1)
+
+        /*
+         * Set up the HTCR:
+         * PT walks use Inner-Shareable accesses,
+         * PT walks are write-back, write-allocate in both cache levels,
+         * Full 32-bit address space goes through this table.
+         */
+        mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
+        mcr   CP32(r0, HTCR)
+
         /*
          * The state of the TLBs is unknown before turning on the MMU.
          * Flush them to avoid stale one.
-- 
2.25.1

[PATCH v2 2/4] xen/arm32: mpu: Create boot-time MPU protection regions

[Ayan Kumar Halder]

Define enable_boot_cpu_mm() for the Armv8-R AArch32.

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
are same, we skip mapping the region.

One needs to set up HMAIR0 and HMAIR1 registers in enable_mpu(). The register
configurations are the same as in enable_mmu().

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from

v1 -

1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to what is
being done in enable_mmu(). All the mm related configurations happen in this
function.

2. Fixed some typos.

 xen/arch/arm/arm32/Makefile           |   1 +
 xen/arch/arm/arm32/mpu/Makefile       |   1 +
 xen/arch/arm/arm32/mpu/head.S         | 170 ++++++++++++++++++++++++++
 xen/arch/arm/include/asm/cpregs.h     |   4 +
 xen/arch/arm/include/asm/mpu/cpregs.h |  21 ++++
 5 files changed, 197 insertions(+)
 create mode 100644 xen/arch/arm/arm32/mpu/Makefile
 create mode 100644 xen/arch/arm/arm32/mpu/head.S
 create mode 100644 xen/arch/arm/include/asm/mpu/cpregs.h

diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 40a2b4803f..537969d753 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,5 +1,6 @@
 obj-y += lib/
 obj-$(CONFIG_MMU) += mmu/
+obj-$(CONFIG_MPU) += mpu/
 
 obj-$(CONFIG_EARLY_PRINTK) += debug.o
 obj-y += domctl.o
diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile
new file mode 100644
index 0000000000..3340058c08
--- /dev/null
+++ b/xen/arch/arm/arm32/mpu/Makefile
@@ -0,0 +1 @@
+obj-y += head.o
diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S
new file mode 100644
index 0000000000..40648ce1a8
--- /dev/null
+++ b/xen/arch/arm/arm32/mpu/head.S
@@ -0,0 +1,170 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Start-of-day code for an Armv8-R MPU system.
+ */
+
+#include <asm/arm32/sysregs.h>
+#include <asm/early_printk.h>
+#include <asm/page.h>
+
+/* Backgroud region enable/disable */
+#define SCTLR_ELx_BR    BIT(17, UL)
+
+#define REGION_TEXT_PRBAR       0x18    /* SH=11 AP=10 XN=0 */
+#define REGION_RO_PRBAR         0x1D    /* SH=11 AP=10 XN=1 */
+#define REGION_DATA_PRBAR       0x19    /* SH=11 AP=00 XN=1 */
+#define REGION_DEVICE_PRBAR     0x11    /* SH=10 AP=00 XN=1 */
+
+#define REGION_NORMAL_PRLAR     0x0f    /* NS=0 ATTR=111 EN=1 */
+#define REGION_DEVICE_PRLAR     0x09    /* NS=0 ATTR=100 EN=1 */
+
+/*
+ * Macro to prepare and set a EL2 MPU memory region.
+ * We will also create an according MPU memory region entry, which
+ * is a structure of pr_t,  in table \prmap.
+ *
+ * sel:         region selector
+ * base:        reg storing base address
+ * limit:       reg storing limit address
+ * prbar:       store computed PRBAR_EL2 value
+ * prlar:       store computed PRLAR_EL2 value
+ * maxcount:    maximum number of EL2 regions supported
+ * attr_prbar:  PRBAR_EL2-related memory attributes. If not specified it will be
+ *              REGION_DATA_PRBAR
+ * attr_prlar:  PRLAR_EL2-related memory attributes. If not specified it will be
+ *              REGION_NORMAL_PRLAR
+ *
+ * Preserves \maxcount
+ * Output:
+ *  \sel: Next available region selector index.
+ * Clobbers \base, \limit, \prbar, \prlar
+ *
+ * Note that all parameters using registers should be distinct.
+ */
+.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR
+    /* Check if the region is empty */
+    cmp   \base, \limit
+    beq   1f
+
+    /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */
+    cmp   \sel, \maxcount
+    bge   fail_insufficient_regions
+
+    /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/
+    and   \base, \base, #MPU_REGION_MASK
+    mov   \prbar, #\attr_prbar
+    orr   \prbar, \prbar, \base
+
+    /* Limit address should be inclusive */
+    sub   \limit, \limit, #1
+    and   \limit, \limit, #MPU_REGION_MASK
+    mov   \prlar, #\attr_prlar
+    orr   \prlar, \prlar, \limit
+
+    mcr   CP32(\sel, PRSELR_EL2)
+    isb
+    mcr   CP32(\prbar, PRBAR_EL2)
+    mcr   CP32(\prlar,  PRLAR_EL2)
+    dsb   sy
+    isb
+
+    add   \sel, \sel, #1
+
+1:
+.endm
+
+/*
+ * Failure caused due to insufficient MPU regions.
+ */
+FUNC_LOCAL(fail_insufficient_regions)
+    PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n")
+1:  wfe
+    b   1b
+END(fail_insufficient_regions)
+
+/*
+ * Enable EL2 MPU and data cache
+ * If the Background region is enabled, then the MPU uses the default memory
+ * map as the Background region for generating the memory
+ * attributes when MPU is disabled.
+ * Since the default memory map of the Armv8-R AArch32 architecture is
+ * IMPLEMENTATION DEFINED, we intend to turn off the Background region here.
+ *
+ * Clobbers x0
+ *
+ */
+FUNC_LOCAL(enable_mpu)
+    /* Set up memory attribute type tables */
+    mov_w r0, MAIR0VAL
+    mov_w r1, MAIR1VAL
+    mcr   CP32(r0, HMAIR0)
+    mcr   CP32(r1, HMAIR1)
+
+    mrc   CP32(r0, HSCTLR)
+    bic   r0, r0, #SCTLR_ELx_BR       /* Disable Background region */
+    orr   r0, r0, #SCTLR_Axx_ELx_M    /* Enable MPU */
+    orr   r0, r0, #SCTLR_Axx_ELx_C    /* Enable D-cache */
+    mcr   CP32(r0, HSCTLR)
+    isb
+
+    ret
+END(enable_mpu)
+
+/*
+ * Maps the various sections of Xen (decsribed in xen.lds.S) as different MPU
+ * regions.
+ *
+ * Clobbers r0
+ *
+ */
+
+FUNC(enable_boot_cpu_mm)
+    /* Get the number of regions specified in MPUIR_EL2 */
+    mrc   CP32(r5, MPUIR_EL2)
+    and   r5, r5, #NUM_MPU_REGIONS_MASK
+
+    /* x0: region sel */
+    mov   r0, #0
+
+    /* Xen text section. */
+    ldr   r1, =_stext
+    ldr   r2, =_etext
+    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+
+    /* Xen read-only data section. */
+    ldr   r1, =_srodata
+    ldr   r2, =_erodata
+    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_RO_PRBAR
+
+    /* Xen read-only after init and data section. (RW data) */
+    ldr   r1, =__ro_after_init_start
+    ldr   r2, =__init_begin
+    prepare_xen_region r0, r1, r2, r3, r4, r5
+
+    /* Xen code section. */
+    ldr   r1, =__init_begin
+    ldr   r2, =__init_data_begin
+    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+
+    /* Xen data and BSS section. */
+    ldr   r1, =__init_data_begin
+    ldr   r2, =__bss_end
+    prepare_xen_region r0, r1, r2, r3, r4, r5
+
+#ifdef CONFIG_EARLY_PRINTK
+    /* Xen early UART section. */
+    ldr   r1, =CONFIG_EARLY_UART_BASE_ADDRESS
+    ldr   r2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE)
+    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
+#endif
+
+    b    enable_mpu
+    ret
+END(enable_boot_cpu_mm)
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h
index aec9e8f329..6019a2cbdd 100644
--- a/xen/arch/arm/include/asm/cpregs.h
+++ b/xen/arch/arm/include/asm/cpregs.h
@@ -1,6 +1,10 @@
 #ifndef __ASM_ARM_CPREGS_H
 #define __ASM_ARM_CPREGS_H
 
+#ifdef CONFIG_MPU
+#include <asm/mpu/cpregs.h>
+#endif
+
 /*
  * AArch32 Co-processor registers.
  *
diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
new file mode 100644
index 0000000000..bd17a8c75a
--- /dev/null
+++ b/xen/arch/arm/include/asm/mpu/cpregs.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_MPU_CPREGS_H
+#define __ASM_ARM_MPU_CPREGS_H
+
+#define HMPUIR          p15,4,c0,c0,4
+
+/* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */
+#define HPRSELR         p15,4,c6,c2,1
+#define PRBAR_EL2       p15,4,c6,c3,0
+#define PRLAR_EL2       p15,4,c6,c8,1
+
+#define MPUIR_EL2               HMPUIR
+#define PRSELR_EL2              HPRSELR
+
+#endif
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.25.1

[PATCH v2 3/4] xen/arm32: mpu: Implement a dummy enable_secondary_cpu_mm

[Ayan Kumar Halder]

Secondary cpus initialization is not yet supported. Thus, we print an
appropriate message and put the secondary cpus in WFE state.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from :-

v1 - 1. Add R-b.

 xen/arch/arm/arm32/mpu/head.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S
index 40648ce1a8..b6d31701f2 100644
--- a/xen/arch/arm/arm32/mpu/head.S
+++ b/xen/arch/arm/arm32/mpu/head.S
@@ -162,6 +162,16 @@ FUNC(enable_boot_cpu_mm)
     ret
 END(enable_boot_cpu_mm)
 
+/*
+ * We don't yet support secondary CPUs bring-up. Implement a dummy helper to
+ * please the common code.
+ */
+ENTRY(enable_secondary_cpu_mm)
+    PRINT("- SMP not enabled yet -\r\n")
+1:  wfe
+    b 1b
+ENDPROC(enable_secondary_cpu_mm)
+
 /*
  * Local variables:
  * mode: ASM
-- 
2.25.1

[PATCH v2 4/4] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU

[Ayan Kumar Halder]

From: Michal Orzel <michal.orzel@amd.com>

ArmV8-R AArch32 does not support LPAE. The reason being PMSAv8-32
supports 32-bit physical address only.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from 

v1 - 1. New patch.

 xen/arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 89c099ff46..b413e8399b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -58,7 +58,7 @@ config ARM_PA_BITS_32
 
 config ARM_PA_BITS_40
 	bool "40-bit"
-	depends on ARM_32
+	depends on ARM_32 && !MPU
 endchoice
 
 config PADDR_BITS
-- 
2.25.1

Re: [PATCH v2 1/4] xen/arm32: Initialize MM specific registers in enable_mmu

[Orzel, Michal]

On 13/03/2025 19:28, Ayan Kumar Halder wrote:
> All the memory management specific registers are initialized in enable_mmu.
The title is a bit misleading (description does not help too). It's a pure code
movement, yet it's not mentioned at all. This can be fixed by changing title:
"Move MM specific registers to enable_mmu"

> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
With the title changed:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v2 2/4] xen/arm32: mpu: Create boot-time MPU protection regions

[Orzel, Michal]

On 13/03/2025 19:28, Ayan Kumar Halder wrote:
> Define enable_boot_cpu_mm() for the Armv8-R AArch32.
> 
> 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
> are same, we skip mapping the region.
> 
> One needs to set up HMAIR0 and HMAIR1 registers in enable_mpu(). The register
> configurations are the same as in enable_mmu().
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from
> 
> v1 -
> 
> 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to what is
> being done in enable_mmu(). All the mm related configurations happen in this
> function.
> 
> 2. Fixed some typos.
> 
>  xen/arch/arm/arm32/Makefile           |   1 +
>  xen/arch/arm/arm32/mpu/Makefile       |   1 +
>  xen/arch/arm/arm32/mpu/head.S         | 170 ++++++++++++++++++++++++++
>  xen/arch/arm/include/asm/cpregs.h     |   4 +
>  xen/arch/arm/include/asm/mpu/cpregs.h |  21 ++++
>  5 files changed, 197 insertions(+)
>  create mode 100644 xen/arch/arm/arm32/mpu/Makefile
>  create mode 100644 xen/arch/arm/arm32/mpu/head.S
>  create mode 100644 xen/arch/arm/include/asm/mpu/cpregs.h
> 
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 40a2b4803f..537969d753 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -1,5 +1,6 @@
>  obj-y += lib/
>  obj-$(CONFIG_MMU) += mmu/
> +obj-$(CONFIG_MPU) += mpu/
>  
>  obj-$(CONFIG_EARLY_PRINTK) += debug.o
>  obj-y += domctl.o
> diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile
> new file mode 100644
> index 0000000000..3340058c08
> --- /dev/null
> +++ b/xen/arch/arm/arm32/mpu/Makefile
> @@ -0,0 +1 @@
> +obj-y += head.o
> diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S
> new file mode 100644
> index 0000000000..40648ce1a8
> --- /dev/null
> +++ b/xen/arch/arm/arm32/mpu/head.S
> @@ -0,0 +1,170 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Start-of-day code for an Armv8-R MPU system.
> + */
> +
> +#include <asm/arm32/sysregs.h>
> +#include <asm/early_printk.h>
> +#include <asm/page.h>
> +
> +/* Backgroud region enable/disable */
> +#define SCTLR_ELx_BR    BIT(17, UL)
> +
> +#define REGION_TEXT_PRBAR       0x18    /* SH=11 AP=10 XN=0 */
> +#define REGION_RO_PRBAR         0x1D    /* SH=11 AP=10 XN=1 */
> +#define REGION_DATA_PRBAR       0x19    /* SH=11 AP=00 XN=1 */
> +#define REGION_DEVICE_PRBAR     0x11    /* SH=10 AP=00 XN=1 */
> +
> +#define REGION_NORMAL_PRLAR     0x0f    /* NS=0 ATTR=111 EN=1 */
> +#define REGION_DEVICE_PRLAR     0x09    /* NS=0 ATTR=100 EN=1 */
> +
> +/*
Lines from here...

> + * Macro to prepare and set a EL2 MPU memory region.
> + * We will also create an according MPU memory region entry, which
> + * is a structure of pr_t,  in table \prmap.
> + *
> + * sel:         region selector
> + * base:        reg storing base address
> + * limit:       reg storing limit address
> + * prbar:       store computed PRBAR_EL2 value
> + * prlar:       store computed PRLAR_EL2 value
> + * maxcount:    maximum number of EL2 regions supported
> + * attr_prbar:  PRBAR_EL2-related memory attributes. If not specified it will be
> + *              REGION_DATA_PRBAR
> + * attr_prlar:  PRLAR_EL2-related memory attributes. If not specified it will be
> + *              REGION_NORMAL_PRLAR
> + *
> + * Preserves \maxcount
> + * Output:
> + *  \sel: Next available region selector index.
> + * Clobbers \base, \limit, \prbar, \prlar
Just a question: I know it was done like that for arm64 but where does this
format of specifying registers in the comment also with leading '\' come from?
At least for me, it reads odd.

> + *
> + * Note that all parameters using registers should be distinct.
> + */
> +.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR
> +    /* Check if the region is empty */
> +    cmp   \base, \limit
> +    beq   1f
> +
> +    /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */
> +    cmp   \sel, \maxcount
> +    bge   fail_insufficient_regions
> +
> +    /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/
> +    and   \base, \base, #MPU_REGION_MASK
> +    mov   \prbar, #\attr_prbar
> +    orr   \prbar, \prbar, \base
> +
> +    /* Limit address should be inclusive */
> +    sub   \limit, \limit, #1
> +    and   \limit, \limit, #MPU_REGION_MASK
> +    mov   \prlar, #\attr_prlar
> +    orr   \prlar, \prlar, \limit
> +
... to here (40+ in total) are exactly the same for arm64 and arm32 MPU. I don't
think it's great to have this code duplication. There are/will be also other
functions that will be the same. Can we have a common MPU head.S or a header
file to avoid duplication?

> +    mcr   CP32(\sel, PRSELR_EL2)
> +    isb
> +    mcr   CP32(\prbar, PRBAR_EL2)
> +    mcr   CP32(\prlar,  PRLAR_EL2)
> +    dsb   sy
> +    isb
> +
> +    add   \sel, \sel, #1
> +
> +1:
> +.endm
> +
> +/*
> + * Failure caused due to insufficient MPU regions.
> + */
No need for multi-line comment for a single sentence.

> +FUNC_LOCAL(fail_insufficient_regions)
> +    PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n")
> +1:  wfe
> +    b   1b
> +END(fail_insufficient_regions)
> +
> +/*
> + * Enable EL2 MPU and data cache
> + * If the Background region is enabled, then the MPU uses the default memory
> + * map as the Background region for generating the memory
> + * attributes when MPU is disabled.
> + * Since the default memory map of the Armv8-R AArch32 architecture is
> + * IMPLEMENTATION DEFINED, we intend to turn off the Background region here.
> + *
> + * Clobbers x0
> + *
No need for this extra line in comment

> + */
> +FUNC_LOCAL(enable_mpu)
> +    /* Set up memory attribute type tables */
> +    mov_w r0, MAIR0VAL
> +    mov_w r1, MAIR1VAL
> +    mcr   CP32(r0, HMAIR0)
> +    mcr   CP32(r1, HMAIR1)
> +
> +    mrc   CP32(r0, HSCTLR)
> +    bic   r0, r0, #SCTLR_ELx_BR       /* Disable Background region */
> +    orr   r0, r0, #SCTLR_Axx_ELx_M    /* Enable MPU */
> +    orr   r0, r0, #SCTLR_Axx_ELx_C    /* Enable D-cache */
> +    mcr   CP32(r0, HSCTLR)
> +    isb
> +
> +    ret
> +END(enable_mpu)
> +
> +/*
> + * Maps the various sections of Xen (decsribed in xen.lds.S) as different MPU
> + * regions.
> + *
> + * Clobbers r0
> + *
No need for this extra line in comment

> + */
> +
No empty line here

> +FUNC(enable_boot_cpu_mm)
This entire function, except for reading sysreg and register prefix (x,r) is the
same between arm64 and arm32. This could also be made common.

> +    /* Get the number of regions specified in MPUIR_EL2 */
> +    mrc   CP32(r5, MPUIR_EL2)
> +    and   r5, r5, #NUM_MPU_REGIONS_MASK
> +
> +    /* x0: region sel */
> +    mov   r0, #0
> +
> +    /* Xen text section. */
> +    ldr   r1, =_stext
> +    ldr   r2, =_etext
> +    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
> +
> +    /* Xen read-only data section. */
> +    ldr   r1, =_srodata
> +    ldr   r2, =_erodata
> +    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_RO_PRBAR
> +
> +    /* Xen read-only after init and data section. (RW data) */
> +    ldr   r1, =__ro_after_init_start
> +    ldr   r2, =__init_begin
> +    prepare_xen_region r0, r1, r2, r3, r4, r5
> +
> +    /* Xen code section. */
> +    ldr   r1, =__init_begin
> +    ldr   r2, =__init_data_begin
> +    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
> +
> +    /* Xen data and BSS section. */
> +    ldr   r1, =__init_data_begin
> +    ldr   r2, =__bss_end
> +    prepare_xen_region r0, r1, r2, r3, r4, r5
> +
> +#ifdef CONFIG_EARLY_PRINTK
> +    /* Xen early UART section. */
> +    ldr   r1, =CONFIG_EARLY_UART_BASE_ADDRESS
> +    ldr   r2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE)
> +    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
> +#endif
> +
> +    b    enable_mpu
> +    ret
> +END(enable_boot_cpu_mm)
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h
> index aec9e8f329..6019a2cbdd 100644
> --- a/xen/arch/arm/include/asm/cpregs.h
> +++ b/xen/arch/arm/include/asm/cpregs.h
> @@ -1,6 +1,10 @@
>  #ifndef __ASM_ARM_CPREGS_H
>  #define __ASM_ARM_CPREGS_H
>  
> +#ifdef CONFIG_MPU
> +#include <asm/mpu/cpregs.h>
> +#endif
> +
>  /*
>   * AArch32 Co-processor registers.
>   *
> diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
> new file mode 100644
> index 0000000000..bd17a8c75a
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/mpu/cpregs.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM_MPU_CPREGS_H
Missing SPDX specifier?

> +#define __ASM_ARM_MPU_CPREGS_H
> +
> +#define HMPUIR          p15,4,c0,c0,4
> +
> +/* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */
> +#define HPRSELR         p15,4,c6,c2,1
> +#define PRBAR_EL2       p15,4,c6,c3,0
> +#define PRLAR_EL2       p15,4,c6,c8,1
> +
> +#define MPUIR_EL2               HMPUIR
> +#define PRSELR_EL2              HPRSELR
> +
> +#endif
Please provide comment /* __ASM_ARM_MPU_CPREGS_H */

> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */

~Michal

Re: [PATCH v2 1/4] xen/arm32: Initialize MM specific registers in enable_mmu

[Ayan Kumar Halder]

Hi Michal,

On 17/03/2025 11:40, Orzel, Michal wrote:
>
> On 13/03/2025 19:28, Ayan Kumar Halder wrote:
>> All the memory management specific registers are initialized in enable_mmu.
> The title is a bit misleading (description does not help too). It's a pure code
> movement, yet it's not mentioned at all. This can be fixed by changing title:
> "Move MM specific registers to enable_mmu"
I will fix this in v3.
>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> With the title changed:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

thnx

- Ayan

>
> ~Michal
>