On 18/10/2024 23:13, Julien Grall wrote:
Hi Ayan,
Hi Julien,
Just one clarification.
On 10/10/2024 15:03, Ayan Kumar Halder wrote:
diff --git a/xen/arch/arm/arm64/mpu/Makefile
b/xen/arch/arm/arm64/mpu/Makefile
new file mode 100644
index 0000000000..3340058c08
--- /dev/null
+++ b/xen/arch/arm/arm64/mpu/Makefile
@@ -0,0 +1 @@
+obj-y += head.o
diff --git a/xen/arch/arm/arm64/mpu/head.S
b/xen/arch/arm/arm64/mpu/head.S
new file mode 100644
index 0000000000..4a21bc815c
--- /dev/null
+++ b/xen/arch/arm/arm64/mpu/head.S
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Start-of-day code for an Armv8-R MPU system.
+ */
+
+#include <asm/mm.h>
+#include <asm/arm64/mpu/sysregs.h>
+
+#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */
+#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */
+#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */
+
+#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 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.
+ *
+ * Inputs:
+ * sel: region selector
+ * base: reg storing base address (should be page-aligned)
+ * 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
+ */
+.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount,
attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR
+
+ /* Check if the number of regions exceeded the count specified
in MPUIR_EL2 */
+ add \sel, \sel, #1
+ cmp \sel, \maxcount
+ bgt fail
+
+ /* 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
+
+ msr PRSELR_EL2, \sel
+ isb
+ msr PRBAR_EL2, \prbar
+ msr PRLAR_EL2, \prlar
+ dsb sy
+ isb
+.endm
+
+/* Load the physical address of a symbol into xb */
+.macro load_paddr xb, sym
+ ldr \xb, =\sym
+ add \xb, \xb, x20 /* x20 - Phys offset */
Sorry I didn't spot this until now. Xen will be linked to a specific
physical address, so why do we need to add an offset?
Yes, this needs to be removed. x20 contains 0.
+.endm
+
+/*
+ * Maps the various sections of Xen (described in xen.lds.S) as
different MPU
+ * regions.
+ *
+ * Inputs:
+ * lr : Address to return to.
+ *
+ * Clobbers x0 - x5
+ *
+ */
+FUNC(enable_boot_cpu_mm)
+
+ /* Check if the number of regions exceeded the count specified
in MPUIR_EL2 */
AFAIU, this doesn't match what the instruction is doing below.
Sorry, this needs to be removed.
+ mrs x5, MPUIR_EL2
+
+ /* x0: region sel */
+ mov x0, xzr
+ /* Xen text section. */
+ load_paddr x1, _stext
+ load_paddr x2, _etext
+ cmp x1, x2
+ beq 1f
This check seems to be excessive... I can't think of a reason why
there would be no text at all... Same for a lot of the checks below.
Is it ok if we have this excess check ? The downsides are only a small
increase in code size and boot time. Otherwise, I need to justify why we
have this checks in some places, but not in others.
+ prepare_xen_region x0, x1, x2, x3, x4, x5,
attr_prbar=REGION_TEXT_PRBAR
+
+1: /* Xen read-only data section. */
+ load_paddr x1, _srodata
+ load_paddr x2, _erodata
+ cmp x1, x2
+ beq 2f
+ prepare_xen_region x0, x1, x2, x3, x4, x5,
attr_prbar=REGION_RO_PRBAR
+
+2: /* Xen read-only after init and data section. (RW data) */
+ load_paddr x1, __ro_after_init_start
+ load_paddr x2, __init_begin
+ cmp x1, x2
+ beq 3f
+ prepare_xen_region x0, x1, x2, x3, x4, x5
+
+3: /* Xen code section. */
+ load_paddr x1, __init_begin
+ load_paddr x2, __init_data_begin
+ cmp x1, x2
+ beq 4f
+ prepare_xen_region x0, x1, x2, x3, x4, x5,
attr_prbar=REGION_TEXT_PRBAR
+
+4: /* Xen data and BSS section. */
+ load_paddr x1, __init_data_begin
+ load_paddr x2, __bss_end
+ cmp x1, x2
+ beq 5f
+ prepare_xen_region x0, x1, x2, x3, x4, x5
+
+5:
+ ret
+
+fail:
This name is a bit too generic given you use within a macro. Also, I
think it should be its own local function because the macro can be
used anywhere.
Ack. I will convert this to a function.
- Ayan
