* [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support
@ 2025-06-11 14:35 Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi all,
This patch serie enables R52 support based on Luca's series.
"[PATCH v6 0/6] First chunk for Arm R82 and MPU support".
Changes from :-
v1 .. v2 - Changes mentioned in individual patches
v3 - Split "arm/mpu: Provide access to the MPU region from the C code"
into 4 patches.
Ayan Kumar Halder (6):
arm/mpu: Introduce MPU memory region map structure
arm/mpu: Provide and populate MPU C data structures
arm/mpu: Move domain-page.c to arm32 specific dir
arm/mpu: Move the functions to arm64 specific files
arm/mpu: Define arm32 system registers
arm/mpu: Enable read/write to protection regions for arm32
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/asm-offsets.c | 6 +
xen/arch/arm/arm32/cache.S | 43 ++++++
xen/arch/arm/arm32/mpu/head.S | 41 ++++-
xen/arch/arm/include/asm/arm32/mpu.h | 34 ++++-
xen/arch/arm/include/asm/mpu.h | 2 -
xen/arch/arm/include/asm/mpu/cpregs.h | 68 ++++++++-
xen/arch/arm/include/asm/mpu/regions.inc | 2 +-
xen/arch/arm/mpu/Makefile | 3 +-
xen/arch/arm/mpu/arm32/Makefile | 2 +
xen/arch/arm/mpu/{ => arm32}/domain-page.c | 0
xen/arch/arm/mpu/arm32/mm.c | 165 +++++++++++++++++++++
xen/arch/arm/mpu/arm64/Makefile | 1 +
xen/arch/arm/mpu/arm64/mm.c | 130 ++++++++++++++++
xen/arch/arm/mpu/mm.c | 123 +--------------
15 files changed, 487 insertions(+), 134 deletions(-)
create mode 100644 xen/arch/arm/arm32/cache.S
create mode 100644 xen/arch/arm/mpu/arm32/Makefile
rename xen/arch/arm/mpu/{ => arm32}/domain-page.c (100%)
create mode 100644 xen/arch/arm/mpu/arm32/mm.c
create mode 100644 xen/arch/arm/mpu/arm64/Makefile
create mode 100644 xen/arch/arm/mpu/arm64/mm.c
--
2.25.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-13 15:30 ` Luca Fancellu
2025-06-16 8:16 ` Julien Grall
2025-06-11 14:35 ` [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures Ayan Kumar Halder
` (4 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Introduce pr_t typedef which is a structure having the prbar and prlar members,
each being structured as the registers of the AArch32 Armv8-R architecture.
Also, define MPU_REGION_RES0 to 0 as there are no reserved 0 bits beyond the
BASE or LIMIT bitfields in prbar or prlar respectively.
In pr_of_addr(), enclose prbar and prlar arm64 specific bitfields with
appropriate macros. So, that this function can be later reused for arm32 as
well.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from v1 :-
1. Preserve pr_t typedef in arch specific files.
2. Fix typo.
v2 :-
1. Change CONFIG_ARM64 to CONFIG_ARM_64 to enclose arm64 specific bitfields for
prbar and prlar registers in pr_of_addr().
xen/arch/arm/include/asm/arm32/mpu.h | 34 ++++++++++++++++++++++++++--
xen/arch/arm/mpu/mm.c | 4 ++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/include/asm/arm32/mpu.h b/xen/arch/arm/include/asm/arm32/mpu.h
index f0d4d4055c..0a6930b3a0 100644
--- a/xen/arch/arm/include/asm/arm32/mpu.h
+++ b/xen/arch/arm/include/asm/arm32/mpu.h
@@ -5,10 +5,40 @@
#ifndef __ASSEMBLY__
+/*
+ * Unlike arm64, there are no reserved 0 bits beyond base and limit bitfield in
+ * prbar and prlar registers respectively.
+ */
+#define MPU_REGION_RES0 0x0
+
+/* Hypervisor Protection Region Base Address Register */
+typedef union {
+ struct {
+ unsigned int xn:1; /* Execute-Never */
+ unsigned int ap_0:1; /* Access Permission AP[0] */
+ unsigned int ro:1; /* Access Permission AP[1] */
+ unsigned int sh:2; /* Shareability */
+ unsigned int res0:1;
+ unsigned int base:26; /* Base Address */
+ } reg;
+ uint32_t bits;
+} prbar_t;
+
+/* Hypervisor Protection Region Limit Address Register */
+typedef union {
+ struct {
+ unsigned int en:1; /* Region enable */
+ unsigned int ai:3; /* Memory Attribute Index */
+ unsigned int res0:2;
+ unsigned int limit:26; /* Limit Address */
+ } reg;
+ uint32_t bits;
+} prlar_t;
+
/* MPU Protection Region */
typedef struct {
- uint32_t prbar;
- uint32_t prlar;
+ prbar_t prbar;
+ prlar_t prlar;
} pr_t;
#endif /* __ASSEMBLY__ */
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 86fbe105af..3d37beab57 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -167,7 +167,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
/* Build up value for PRBAR_EL2. */
prbar = (prbar_t) {
.reg = {
+#ifdef CONFIG_ARM_64
.xn_0 = 0,
+#endif
.xn = PAGE_XN_MASK(flags),
.ap_0 = 0,
.ro = PAGE_RO_MASK(flags)
@@ -206,7 +208,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
/* 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 */
}};
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-13 15:39 ` Luca Fancellu
2025-06-16 8:25 ` Julien Grall
2025-06-11 14:35 ` [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir Ayan Kumar Halder
` (3 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Modify Arm32 assembly boot code to reset any unused MPU region, initialise
'max_mpu_regions' with the number of supported MPU regions and set/clear the
bitmap 'xen_mpumap_mask' used to track the enabled regions.
Introduce cache.S to hold arm32 cache related functions.
Use the macro definition for "dcache_line_size" from linux.
Change the order of registers in prepare_xen_region() as 'strd' instruction
is used to store {prbar, prlar} in arm32. Thus, 'prbar' has to be a even
numbered register and 'prlar' is the consecutively ordered register.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from v1 :-
1. Introduce cache.S to hold arm32 cache initialization instructions.
2. Use dcache_line_size macro definition from linux.
3. Use mov_w instead of ldr.
4. Use a single stm instruction for 'store_pair' macro definition.
v2 :-
1. Use strd instead of stm.
2. Fix some coding style issues.
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/asm-offsets.c | 6 ++++
xen/arch/arm/arm32/cache.S | 43 ++++++++++++++++++++++++
xen/arch/arm/arm32/mpu/head.S | 41 +++++++++++++++++-----
xen/arch/arm/include/asm/mpu/regions.inc | 2 +-
5 files changed, 84 insertions(+), 9 deletions(-)
create mode 100644 xen/arch/arm/arm32/cache.S
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 537969d753..531168f58a 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -2,6 +2,7 @@ obj-y += lib/
obj-$(CONFIG_MMU) += mmu/
obj-$(CONFIG_MPU) += mpu/
+obj-y += cache.o
obj-$(CONFIG_EARLY_PRINTK) += debug.o
obj-y += domctl.o
obj-y += domain.o
diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
index 8bbb0f938e..c203ce269d 100644
--- a/xen/arch/arm/arm32/asm-offsets.c
+++ b/xen/arch/arm/arm32/asm-offsets.c
@@ -75,6 +75,12 @@ void __dummy__(void)
OFFSET(INITINFO_stack, struct init_info, stack);
BLANK();
+
+#ifdef CONFIG_MPU
+ DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
+ DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
+ BLANK();
+#endif
}
/*
diff --git a/xen/arch/arm/arm32/cache.S b/xen/arch/arm/arm32/cache.S
new file mode 100644
index 0000000000..b21bc66793
--- /dev/null
+++ b/xen/arch/arm/arm32/cache.S
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Cache maintenance */
+
+#include <asm/arm32/sysregs.h>
+
+/* dcache_line_size - get the minimum D-cache line size from the CTR register */
+ .macro dcache_line_size, reg, tmp
+ mrc CP32(\tmp, CTR) /* read ctr */
+ lsr \tmp, \tmp, #16
+ and \tmp, \tmp, #0xf /* cache line size encoding */
+ mov \reg, #4 /* bytes per word */
+ mov \reg, \reg, lsl \tmp /* actual cache line size */
+ .endm
+
+/*
+ * __invalidate_dcache_area(addr, size)
+ *
+ * Ensure that the data held in the cache for the buffer is invalidated.
+ *
+ * - addr - start address of the buffer
+ * - size - size of the buffer
+ *
+ * Clobbers r0 - r3
+ */
+FUNC(__invalidate_dcache_area)
+ dcache_line_size r2, r3
+ add r1, r0, r1
+ sub r3, r2, #1
+ bic r0, r0, r3
+1: mcr CP32(r0, DCIMVAC) /* invalidate D line / unified line */
+ add r0, r0, r2
+ cmp r0, r1
+ blo 1b
+ dsb sy
+ ret
+END(__invalidate_dcache_area)
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S
index b2c5245e51..6a631626a7 100644
--- a/xen/arch/arm/arm32/mpu/head.S
+++ b/xen/arch/arm/arm32/mpu/head.S
@@ -46,43 +46,68 @@ END(enable_mpu)
*/
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
+ mrc CP32(r3, MPUIR_EL2)
+ and r3, r3, #NUM_MPU_REGIONS_MASK
+
+ mov_w r0, max_mpu_regions
+ str r3, [r0]
+ mcr CP32(r0, DCIMVAC) /* Invalidate cache for max_mpu_regions addr */
/* x0: region sel */
mov r0, #0
/* Xen text section. */
mov_w r1, _stext
mov_w r2, _etext
- prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+ prepare_xen_region r0, r1, r2, r4, r5, r3, attr_prbar=REGION_TEXT_PRBAR
/* Xen read-only data section. */
mov_w r1, _srodata
mov_w r2, _erodata
- prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_RO_PRBAR
+ prepare_xen_region r0, r1, r2, r4, r5, r3, attr_prbar=REGION_RO_PRBAR
/* Xen read-only after init and data section. (RW data) */
mov_w r1, __ro_after_init_start
mov_w r2, __init_begin
- prepare_xen_region r0, r1, r2, r3, r4, r5
+ prepare_xen_region r0, r1, r2, r4, r5, r3
/* Xen code section. */
mov_w r1, __init_begin
mov_w r2, __init_data_begin
- prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+ prepare_xen_region r0, r1, r2, r4, r5, r3, attr_prbar=REGION_TEXT_PRBAR
/* Xen data and BSS section. */
mov_w r1, __init_data_begin
mov_w r2, __bss_end
- prepare_xen_region r0, r1, r2, r3, r4, r5
+ prepare_xen_region r0, r1, r2, r4, r5, r3
#ifdef CONFIG_EARLY_PRINTK
/* Xen early UART section. */
mov_w r1, CONFIG_EARLY_UART_BASE_ADDRESS
mov_w 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
+ prepare_xen_region r0, r1, r2, r4, r5, r3, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
#endif
+zero_mpu:
+ /* Reset remaining MPU regions */
+ cmp r0, r3
+ beq out_zero_mpu
+ mov r1, #0
+ mov r2, #1
+ prepare_xen_region r0, r1, r2, r4, r5, r3, attr_prlar=REGION_DISABLED_PRLAR
+ b zero_mpu
+
+out_zero_mpu:
+ /* Invalidate data cache for MPU data structures */
+ mov r4, lr
+ mov_w r0, xen_mpumap_mask
+ mov r1, #XEN_MPUMAP_MASK_sizeof
+ bl __invalidate_dcache_area
+
+ ldr r0, =xen_mpumap
+ mov r1, #XEN_MPUMAP_sizeof
+ bl __invalidate_dcache_area
+ mov lr, r4
+
b enable_mpu
END(enable_boot_cpu_mm)
diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/include/asm/mpu/regions.inc
index 6b8c233e6c..23fead3b21 100644
--- a/xen/arch/arm/include/asm/mpu/regions.inc
+++ b/xen/arch/arm/include/asm/mpu/regions.inc
@@ -24,7 +24,7 @@
#define XEN_MPUMAP_ENTRY_SHIFT 0x3 /* 8 byte structure */
.macro store_pair reg1, reg2, dst
- .word 0xe7f000f0 /* unimplemented */
+ strd \reg1, \reg2, [\dst]
.endm
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-11 19:46 ` Luca Fancellu
2025-06-11 14:35 ` [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files Ayan Kumar Halder
` (2 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Create xen/arch/arm/mpu/arm32 to hold arm32 specific bits.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from :-
v1..v2 - New patch in v3.
xen/arch/arm/mpu/Makefile | 2 +-
xen/arch/arm/mpu/arm32/Makefile | 1 +
xen/arch/arm/mpu/{ => arm32}/domain-page.c | 0
3 files changed, 2 insertions(+), 1 deletion(-)
create mode 100644 xen/arch/arm/mpu/arm32/Makefile
rename xen/arch/arm/mpu/{ => arm32}/domain-page.c (100%)
diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile
index 808e3e2cb3..9359d79332 100644
--- a/xen/arch/arm/mpu/Makefile
+++ b/xen/arch/arm/mpu/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_ARM_32) += domain-page.o
+obj-$(CONFIG_ARM_32) += arm32/
obj-y += mm.o
obj-y += p2m.o
obj-y += setup.init.o
diff --git a/xen/arch/arm/mpu/arm32/Makefile b/xen/arch/arm/mpu/arm32/Makefile
new file mode 100644
index 0000000000..e15ce2f7be
--- /dev/null
+++ b/xen/arch/arm/mpu/arm32/Makefile
@@ -0,0 +1 @@
+obj-y += domain-page.o
diff --git a/xen/arch/arm/mpu/domain-page.c b/xen/arch/arm/mpu/arm32/domain-page.c
similarity index 100%
rename from xen/arch/arm/mpu/domain-page.c
rename to xen/arch/arm/mpu/arm32/domain-page.c
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
` (2 preceding siblings ...)
2025-06-11 14:35 ` [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-13 15:08 ` Luca Fancellu
2025-06-11 14:35 ` [PATCH v3 5/6] arm/mpu: Define arm32 system registers Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
5 siblings, 1 reply; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
prepare_selector(), read_protection_region() and write_protection_region()
differ significantly between arm32 and arm64. Thus, move these functions
to their specific folders.
GENERATE_{WRITE/READ}_PR_REG_CASE are duplicated for arm32 and arm64 so
as to improve the code readability.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from -
v1..v2 - New patch introduced in v3.
xen/arch/arm/mpu/Makefile | 1 +
xen/arch/arm/mpu/arm64/Makefile | 1 +
xen/arch/arm/mpu/arm64/mm.c | 130 ++++++++++++++++++++++++++++++++
xen/arch/arm/mpu/mm.c | 117 ----------------------------
4 files changed, 132 insertions(+), 117 deletions(-)
create mode 100644 xen/arch/arm/mpu/arm64/Makefile
create mode 100644 xen/arch/arm/mpu/arm64/mm.c
diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile
index 9359d79332..4963c8b550 100644
--- a/xen/arch/arm/mpu/Makefile
+++ b/xen/arch/arm/mpu/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_ARM_32) += arm32/
+obj-$(CONFIG_ARM_64) += arm64/
obj-y += mm.o
obj-y += p2m.o
obj-y += setup.init.o
diff --git a/xen/arch/arm/mpu/arm64/Makefile b/xen/arch/arm/mpu/arm64/Makefile
new file mode 100644
index 0000000000..b18cec4836
--- /dev/null
+++ b/xen/arch/arm/mpu/arm64/Makefile
@@ -0,0 +1 @@
+obj-y += mm.o
diff --git a/xen/arch/arm/mpu/arm64/mm.c b/xen/arch/arm/mpu/arm64/mm.c
new file mode 100644
index 0000000000..a978c1fc6e
--- /dev/null
+++ b/xen/arch/arm/mpu/arm64/mm.c
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/bug.h>
+#include <xen/types.h>
+#include <asm/mpu.h>
+#include <asm/sysregs.h>
+#include <asm/system.h>
+
+/*
+ * The following are needed for the cases: GENERATE_WRITE_PR_REG_CASE
+ * and GENERATE_READ_PR_REG_CASE with num==0
+ */
+#define PRBAR0_EL2 PRBAR_EL2
+#define PRLAR0_EL2 PRLAR_EL2
+
+#define PRBAR_EL2_(n) PRBAR##n##_EL2
+#define PRLAR_EL2_(n) PRLAR##n##_EL2
+
+#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
+ case num: \
+ { \
+ WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
+ WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
+ break; \
+ }
+
+#define GENERATE_READ_PR_REG_CASE(num, pr) \
+ case num: \
+ { \
+ pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
+ pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
+ break; \
+ }
+
+/*
+ * Armv8-R supports direct access and indirect access to the MPU regions through
+ * registers:
+ * - indirect access involves changing the MPU region selector, issuing an isb
+ * barrier and accessing the selected region through specific registers
+ * - direct access involves accessing specific registers that point to
+ * specific MPU regions, without changing the selector, avoiding the use of
+ * a barrier.
+ * For Arm64 the PR{B,L}AR_ELx (for n=0) and PR{B,L}AR<n>_ELx (for n=1..15) are
+ * used for the direct access to the regions selected by
+ * PRSELR_EL2.REGION<7:4>:n, so 16 regions can be directly accessed when the
+ * selector is a multiple of 16, giving access to all the supported memory
+ * regions.
+ */
+static void prepare_selector(uint8_t *sel)
+{
+ uint8_t cur_sel = *sel;
+
+ /*
+ * {read,write}_protection_region works using the direct access to the 0..15
+ * regions, so in order to save the isb() overhead, change the PRSELR_EL2
+ * only when needed, so when the upper 4 bits of the selector will change.
+ */
+ cur_sel &= 0xF0U;
+ if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
+ {
+ WRITE_SYSREG(cur_sel, PRSELR_EL2);
+ isb();
+ }
+ *sel = *sel & 0xFU;
+}
+
+void read_protection_region(pr_t *pr_read, uint8_t sel)
+{
+ prepare_selector(&sel);
+
+ switch ( sel )
+ {
+ GENERATE_READ_PR_REG_CASE(0, pr_read);
+ GENERATE_READ_PR_REG_CASE(1, pr_read);
+ GENERATE_READ_PR_REG_CASE(2, pr_read);
+ GENERATE_READ_PR_REG_CASE(3, pr_read);
+ GENERATE_READ_PR_REG_CASE(4, pr_read);
+ GENERATE_READ_PR_REG_CASE(5, pr_read);
+ GENERATE_READ_PR_REG_CASE(6, pr_read);
+ GENERATE_READ_PR_REG_CASE(7, pr_read);
+ GENERATE_READ_PR_REG_CASE(8, pr_read);
+ GENERATE_READ_PR_REG_CASE(9, pr_read);
+ GENERATE_READ_PR_REG_CASE(10, pr_read);
+ GENERATE_READ_PR_REG_CASE(11, pr_read);
+ GENERATE_READ_PR_REG_CASE(12, pr_read);
+ GENERATE_READ_PR_REG_CASE(13, pr_read);
+ GENERATE_READ_PR_REG_CASE(14, pr_read);
+ GENERATE_READ_PR_REG_CASE(15, pr_read);
+ default:
+ BUG(); /* Can't happen */
+ break;
+ }
+}
+
+void write_protection_region(const pr_t *pr_write, uint8_t sel)
+{
+ prepare_selector(&sel);
+
+ switch ( sel )
+ {
+ GENERATE_WRITE_PR_REG_CASE(0, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(1, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(2, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(3, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(4, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(5, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(6, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(7, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(8, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(9, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(10, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(11, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(12, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(13, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(14, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(15, pr_write);
+ default:
+ BUG(); /* Can't happen */
+ break;
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 3d37beab57..7ab68fc8c7 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -29,35 +29,6 @@ DECLARE_BITMAP(xen_mpumap_mask, MAX_MPU_REGION_NR) \
/* EL2 Xen MPU memory region mapping table. */
pr_t __cacheline_aligned __section(".data") xen_mpumap[MAX_MPU_REGION_NR];
-#ifdef CONFIG_ARM_64
-/*
- * The following are needed for the cases: GENERATE_WRITE_PR_REG_CASE
- * and GENERATE_READ_PR_REG_CASE with num==0
- */
-#define PRBAR0_EL2 PRBAR_EL2
-#define PRLAR0_EL2 PRLAR_EL2
-
-#define PRBAR_EL2_(n) PRBAR##n##_EL2
-#define PRLAR_EL2_(n) PRLAR##n##_EL2
-
-#endif /* CONFIG_ARM_64 */
-
-#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
- case num: \
- { \
- WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
- WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
- break; \
- }
-
-#define GENERATE_READ_PR_REG_CASE(num, pr) \
- case num: \
- { \
- pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
- pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
- break; \
- }
-
static void __init __maybe_unused build_assertions(void)
{
/*
@@ -69,94 +40,6 @@ static void __init __maybe_unused build_assertions(void)
}
#ifdef CONFIG_ARM_64
-/*
- * Armv8-R supports direct access and indirect access to the MPU regions through
- * registers:
- * - indirect access involves changing the MPU region selector, issuing an isb
- * barrier and accessing the selected region through specific registers
- * - direct access involves accessing specific registers that point to
- * specific MPU regions, without changing the selector, avoiding the use of
- * a barrier.
- * For Arm64 the PR{B,L}AR_ELx (for n=0) and PR{B,L}AR<n>_ELx (for n=1..15) are
- * used for the direct access to the regions selected by
- * PRSELR_EL2.REGION<7:4>:n, so 16 regions can be directly accessed when the
- * selector is a multiple of 16, giving access to all the supported memory
- * regions.
- */
-static void prepare_selector(uint8_t *sel)
-{
- uint8_t cur_sel = *sel;
-
- /*
- * {read,write}_protection_region works using the direct access to the 0..15
- * regions, so in order to save the isb() overhead, change the PRSELR_EL2
- * only when needed, so when the upper 4 bits of the selector will change.
- */
- cur_sel &= 0xF0U;
- if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
- {
- WRITE_SYSREG(cur_sel, PRSELR_EL2);
- isb();
- }
- *sel &= 0xFU;
-}
-
-void read_protection_region(pr_t *pr_read, uint8_t sel)
-{
- prepare_selector(&sel);
-
- switch ( sel )
- {
- GENERATE_READ_PR_REG_CASE(0, pr_read);
- GENERATE_READ_PR_REG_CASE(1, pr_read);
- GENERATE_READ_PR_REG_CASE(2, pr_read);
- GENERATE_READ_PR_REG_CASE(3, pr_read);
- GENERATE_READ_PR_REG_CASE(4, pr_read);
- GENERATE_READ_PR_REG_CASE(5, pr_read);
- GENERATE_READ_PR_REG_CASE(6, pr_read);
- GENERATE_READ_PR_REG_CASE(7, pr_read);
- GENERATE_READ_PR_REG_CASE(8, pr_read);
- GENERATE_READ_PR_REG_CASE(9, pr_read);
- GENERATE_READ_PR_REG_CASE(10, pr_read);
- GENERATE_READ_PR_REG_CASE(11, pr_read);
- GENERATE_READ_PR_REG_CASE(12, pr_read);
- GENERATE_READ_PR_REG_CASE(13, pr_read);
- GENERATE_READ_PR_REG_CASE(14, pr_read);
- GENERATE_READ_PR_REG_CASE(15, pr_read);
- default:
- BUG(); /* Can't happen */
- break;
- }
-}
-
-void write_protection_region(const pr_t *pr_write, uint8_t sel)
-{
- prepare_selector(&sel);
-
- switch ( sel )
- {
- GENERATE_WRITE_PR_REG_CASE(0, pr_write);
- GENERATE_WRITE_PR_REG_CASE(1, pr_write);
- GENERATE_WRITE_PR_REG_CASE(2, pr_write);
- GENERATE_WRITE_PR_REG_CASE(3, pr_write);
- GENERATE_WRITE_PR_REG_CASE(4, pr_write);
- GENERATE_WRITE_PR_REG_CASE(5, pr_write);
- GENERATE_WRITE_PR_REG_CASE(6, pr_write);
- GENERATE_WRITE_PR_REG_CASE(7, pr_write);
- GENERATE_WRITE_PR_REG_CASE(8, pr_write);
- GENERATE_WRITE_PR_REG_CASE(9, pr_write);
- GENERATE_WRITE_PR_REG_CASE(10, pr_write);
- GENERATE_WRITE_PR_REG_CASE(11, pr_write);
- GENERATE_WRITE_PR_REG_CASE(12, pr_write);
- GENERATE_WRITE_PR_REG_CASE(13, pr_write);
- GENERATE_WRITE_PR_REG_CASE(14, pr_write);
- GENERATE_WRITE_PR_REG_CASE(15, pr_write);
- default:
- BUG(); /* Can't happen */
- break;
- }
-}
-
pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
{
unsigned int attr_idx = PAGE_AI_MASK(flags);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 5/6] arm/mpu: Define arm32 system registers
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
` (3 preceding siblings ...)
2025-06-11 14:35 ` [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-16 10:38 ` Hari Limaye
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
5 siblings, 1 reply; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Fix the definition for HPRLAR.
Define the base/limit address registers to access the first 32 protection
regions.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from :-
v1 - v1 - New patch introduced in v3 (Extracted from
"arm/mpu: Provide access to the MPU region from the C code").
xen/arch/arm/include/asm/mpu/cpregs.h | 68 ++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
index d5cd0e04d5..bb15e02df6 100644
--- a/xen/arch/arm/include/asm/mpu/cpregs.h
+++ b/xen/arch/arm/include/asm/mpu/cpregs.h
@@ -9,7 +9,73 @@
/* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */
#define HPRSELR p15,4,c6,c2,1
#define HPRBAR p15,4,c6,c3,0
-#define HPRLAR p15,4,c6,c8,1
+#define HPRLAR p15,4,c6,c3,1
+
+/* CP15 CR6: MPU Protection Region Base/Limit Address Register */
+#define HPRBAR0 p15,4,c6,c8,0
+#define HPRLAR0 p15,4,c6,c8,1
+#define HPRBAR1 p15,4,c6,c8,4
+#define HPRLAR1 p15,4,c6,c8,5
+#define HPRBAR2 p15,4,c6,c9,0
+#define HPRLAR2 p15,4,c6,c9,1
+#define HPRBAR3 p15,4,c6,c9,4
+#define HPRLAR3 p15,4,c6,c9,5
+#define HPRBAR4 p15,4,c6,c10,0
+#define HPRLAR4 p15,4,c6,c10,1
+#define HPRBAR5 p15,4,c6,c10,4
+#define HPRLAR5 p15,4,c6,c10,5
+#define HPRBAR6 p15,4,c6,c11,0
+#define HPRLAR6 p15,4,c6,c11,1
+#define HPRBAR7 p15,4,c6,c11,4
+#define HPRLAR7 p15,4,c6,c11,5
+#define HPRBAR8 p15,4,c6,c12,0
+#define HPRLAR8 p15,4,c6,c12,1
+#define HPRBAR9 p15,4,c6,c12,4
+#define HPRLAR9 p15,4,c6,c12,5
+#define HPRBAR10 p15,4,c6,c13,0
+#define HPRLAR10 p15,4,c6,c13,1
+#define HPRBAR11 p15,4,c6,c13,4
+#define HPRLAR11 p15,4,c6,c13,5
+#define HPRBAR12 p15,4,c6,c14,0
+#define HPRLAR12 p15,4,c6,c14,1
+#define HPRBAR13 p15,4,c6,c14,4
+#define HPRLAR13 p15,4,c6,c14,5
+#define HPRBAR14 p15,4,c6,c15,0
+#define HPRLAR14 p15,4,c6,c15,1
+#define HPRBAR15 p15,4,c6,c15,4
+#define HPRLAR15 p15,4,c6,c15,5
+#define HPRBAR16 p15,5,c6,c8,0
+#define HPRLAR16 p15,5,c6,c8,1
+#define HPRBAR17 p15,5,c6,c8,4
+#define HPRLAR17 p15,5,c6,c8,5
+#define HPRBAR18 p15,5,c6,c9,0
+#define HPRLAR18 p15,5,c6,c9,1
+#define HPRBAR19 p15,5,c6,c9,4
+#define HPRLAR19 p15,5,c6,c9,5
+#define HPRBAR20 p15,5,c6,c10,0
+#define HPRLAR20 p15,5,c6,c10,1
+#define HPRBAR21 p15,5,c6,c10,4
+#define HPRLAR21 p15,5,c6,c10,5
+#define HPRBAR22 p15,5,c6,c11,0
+#define HPRLAR22 p15,5,c6,c11,1
+#define HPRBAR23 p15,5,c6,c11,4
+#define HPRLAR23 p15,5,c6,c11,5
+#define HPRBAR24 p15,5,c6,c12,0
+#define HPRLAR24 p15,5,c6,c12,1
+#define HPRBAR25 p15,5,c6,c12,4
+#define HPRLAR25 p15,5,c6,c12,5
+#define HPRBAR26 p15,5,c6,c13,0
+#define HPRLAR26 p15,5,c6,c13,1
+#define HPRBAR27 p15,5,c6,c13,4
+#define HPRLAR27 p15,5,c6,c13,5
+#define HPRBAR28 p15,5,c6,c14,0
+#define HPRLAR28 p15,5,c6,c14,1
+#define HPRBAR29 p15,5,c6,c14,4
+#define HPRLAR29 p15,5,c6,c14,5
+#define HPRBAR30 p15,5,c6,c15,0
+#define HPRLAR30 p15,5,c6,c15,1
+#define HPRBAR31 p15,5,c6,c15,4
+#define HPRLAR31 p15,5,c6,c15,5
/* Aliases of AArch64 names for use in common code */
#ifdef CONFIG_ARM_32
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
` (4 preceding siblings ...)
2025-06-11 14:35 ` [PATCH v3 5/6] arm/mpu: Define arm32 system registers Ayan Kumar Halder
@ 2025-06-11 14:35 ` Ayan Kumar Halder
2025-06-12 9:35 ` Luca Fancellu
` (3 more replies)
5 siblings, 4 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-11 14:35 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Define prepare_selector(), read_protection_region() and
write_protection_region() for arm32. Also, define
GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
Enable pr_of_addr() for arm32.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from :-
v1 - 1. Enable write_protection_region() for aarch32.
v2 - 1. Enable access to protection regions from 0 - 255.
xen/arch/arm/include/asm/mpu.h | 2 -
xen/arch/arm/mpu/arm32/Makefile | 1 +
xen/arch/arm/mpu/arm32/mm.c | 165 ++++++++++++++++++++++++++++++++
xen/arch/arm/mpu/mm.c | 2 -
4 files changed, 166 insertions(+), 4 deletions(-)
create mode 100644 xen/arch/arm/mpu/arm32/mm.c
diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
index 8f06ddac0f..63560c613b 100644
--- a/xen/arch/arm/include/asm/mpu.h
+++ b/xen/arch/arm/include/asm/mpu.h
@@ -25,7 +25,6 @@
#ifndef __ASSEMBLY__
-#ifdef CONFIG_ARM_64
/*
* Set base address of MPU protection region.
*
@@ -85,7 +84,6 @@ static inline bool region_is_valid(const pr_t *pr)
{
return pr->prlar.reg.en;
}
-#endif /* CONFIG_ARM_64 */
#endif /* __ASSEMBLY__ */
diff --git a/xen/arch/arm/mpu/arm32/Makefile b/xen/arch/arm/mpu/arm32/Makefile
index e15ce2f7be..3da872322e 100644
--- a/xen/arch/arm/mpu/arm32/Makefile
+++ b/xen/arch/arm/mpu/arm32/Makefile
@@ -1 +1,2 @@
obj-y += domain-page.o
+obj-y += mm.o
diff --git a/xen/arch/arm/mpu/arm32/mm.c b/xen/arch/arm/mpu/arm32/mm.c
new file mode 100644
index 0000000000..5d3cb6dff7
--- /dev/null
+++ b/xen/arch/arm/mpu/arm32/mm.c
@@ -0,0 +1,165 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/bug.h>
+#include <xen/types.h>
+#include <asm/mpu.h>
+#include <asm/sysregs.h>
+#include <asm/system.h>
+
+#define PRBAR_EL2_(n) HPRBAR##n
+#define PRLAR_EL2_(n) HPRLAR##n
+
+#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
+ case num: \
+ { \
+ WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
+ WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
+ break; \
+ }
+
+#define GENERATE_WRITE_PR_REG_OTHERS(num, pr) \
+ case num: \
+ { \
+ WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, HPRBAR); \
+ WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, HPRLAR); \
+ break; \
+ }
+
+#define GENERATE_READ_PR_REG_CASE(num, pr) \
+ case num: \
+ { \
+ pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
+ pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
+ break; \
+ }
+
+#define GENERATE_READ_PR_REG_OTHERS(num, pr) \
+ case num: \
+ { \
+ pr->prbar.bits = READ_SYSREG(HPRBAR); \
+ pr->prlar.bits = READ_SYSREG(HPRLAR); \
+ break; \
+ }
+
+/*
+ * Armv8-R supports direct access and indirect access to the MPU regions through
+ * registers:
+ * - indirect access involves changing the MPU region selector, issuing an isb
+ * barrier and accessing the selected region through specific registers
+ * - direct access involves accessing specific registers that point to
+ * specific MPU regions, without changing the selector, avoiding the use of
+ * a barrier.
+ * For Arm32 the PR{B,L}AR<n>_ELx (for n=0..31) are used for direct access to the
+ * first 32 MPU regions.
+ * For MPU region numbered 32..255, one need to set the region number in PRSELR_ELx,
+ * followed by configuring PR{B,L}AR_ELx.
+ */
+inline void prepare_selector(uint8_t *sel)
+{
+ uint8_t cur_sel = *sel;
+
+ if ( cur_sel > 0x1FU )
+ {
+ WRITE_SYSREG(cur_sel, PRSELR_EL2);
+ isb();
+ }
+}
+
+void read_protection_region(pr_t *pr_read, uint8_t sel)
+{
+ prepare_selector(&sel);
+
+ switch ( sel )
+ {
+ GENERATE_READ_PR_REG_CASE(0, pr_read);
+ GENERATE_READ_PR_REG_CASE(1, pr_read);
+ GENERATE_READ_PR_REG_CASE(2, pr_read);
+ GENERATE_READ_PR_REG_CASE(3, pr_read);
+ GENERATE_READ_PR_REG_CASE(4, pr_read);
+ GENERATE_READ_PR_REG_CASE(5, pr_read);
+ GENERATE_READ_PR_REG_CASE(6, pr_read);
+ GENERATE_READ_PR_REG_CASE(7, pr_read);
+ GENERATE_READ_PR_REG_CASE(8, pr_read);
+ GENERATE_READ_PR_REG_CASE(9, pr_read);
+ GENERATE_READ_PR_REG_CASE(10, pr_read);
+ GENERATE_READ_PR_REG_CASE(11, pr_read);
+ GENERATE_READ_PR_REG_CASE(12, pr_read);
+ GENERATE_READ_PR_REG_CASE(13, pr_read);
+ GENERATE_READ_PR_REG_CASE(14, pr_read);
+ GENERATE_READ_PR_REG_CASE(15, pr_read);
+ GENERATE_READ_PR_REG_CASE(16, pr_read);
+ GENERATE_READ_PR_REG_CASE(17, pr_read);
+ GENERATE_READ_PR_REG_CASE(18, pr_read);
+ GENERATE_READ_PR_REG_CASE(19, pr_read);
+ GENERATE_READ_PR_REG_CASE(20, pr_read);
+ GENERATE_READ_PR_REG_CASE(21, pr_read);
+ GENERATE_READ_PR_REG_CASE(22, pr_read);
+ GENERATE_READ_PR_REG_CASE(23, pr_read);
+ GENERATE_READ_PR_REG_CASE(24, pr_read);
+ GENERATE_READ_PR_REG_CASE(25, pr_read);
+ GENERATE_READ_PR_REG_CASE(26, pr_read);
+ GENERATE_READ_PR_REG_CASE(27, pr_read);
+ GENERATE_READ_PR_REG_CASE(28, pr_read);
+ GENERATE_READ_PR_REG_CASE(29, pr_read);
+ GENERATE_READ_PR_REG_CASE(30, pr_read);
+ GENERATE_READ_PR_REG_CASE(31, pr_read);
+ GENERATE_READ_PR_REG_OTHERS(32 ... 255, pr_read);
+ default:
+ BUG(); /* Can't happen */
+ break;
+ }
+}
+
+void write_protection_region(const pr_t *pr_write, uint8_t sel)
+{
+ prepare_selector(&sel);
+
+ switch ( sel )
+ {
+ GENERATE_WRITE_PR_REG_CASE(0, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(1, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(2, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(3, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(4, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(5, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(6, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(7, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(8, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(9, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(10, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(11, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(12, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(13, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(14, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(15, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(16, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(17, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(18, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(19, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(20, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(21, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(22, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(23, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(24, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(25, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(26, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(27, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(28, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(29, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(30, pr_write);
+ GENERATE_WRITE_PR_REG_CASE(31, pr_write);
+ GENERATE_WRITE_PR_REG_OTHERS(32 ... 255, pr_write);
+ default:
+ BUG(); /* Can't happen */
+ break;
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 7ab68fc8c7..ccfb37a67b 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -39,7 +39,6 @@ static void __init __maybe_unused build_assertions(void)
BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
}
-#ifdef CONFIG_ARM_64
pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
{
unsigned int attr_idx = PAGE_AI_MASK(flags);
@@ -110,7 +109,6 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
return region;
}
-#endif /* CONFIG_ARM_64 */
void __init setup_mm(void)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir
2025-06-11 14:35 ` [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir Ayan Kumar Halder
@ 2025-06-11 19:46 ` Luca Fancellu
2025-06-12 7:48 ` Ayan Kumar Halder
0 siblings, 1 reply; 23+ messages in thread
From: Luca Fancellu @ 2025-06-11 19:46 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Create xen/arch/arm/mpu/arm32 to hold arm32 specific bits.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from :-
>
> v1..v2 - New patch in v3.
>
> xen/arch/arm/mpu/Makefile | 2 +-
> xen/arch/arm/mpu/arm32/Makefile | 1 +
> xen/arch/arm/mpu/{ => arm32}/domain-page.c | 0
> 3 files changed, 2 insertions(+), 1 deletion(-)
> create mode 100644 xen/arch/arm/mpu/arm32/Makefile
> rename xen/arch/arm/mpu/{ => arm32}/domain-page.c (100%)
Uhm, why?
Arm64 is using domain-page.c:
https://gitlab.com/xen-project/people/lucafancellu/xen/-/commit/b28198d00078991d4a6502e94c8779d84fec0120
Did I miss something?
Cheers,
Luca
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir
2025-06-11 19:46 ` Luca Fancellu
@ 2025-06-12 7:48 ` Ayan Kumar Halder
0 siblings, 0 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-12 7:48 UTC (permalink / raw)
To: Luca Fancellu, Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
On 11/06/2025 20:46, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
>
>> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> Create xen/arch/arm/mpu/arm32 to hold arm32 specific bits.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>> Changes from :-
>>
>> v1..v2 - New patch in v3.
>>
>> xen/arch/arm/mpu/Makefile | 2 +-
>> xen/arch/arm/mpu/arm32/Makefile | 1 +
>> xen/arch/arm/mpu/{ => arm32}/domain-page.c | 0
>> 3 files changed, 2 insertions(+), 1 deletion(-)
>> create mode 100644 xen/arch/arm/mpu/arm32/Makefile
>> rename xen/arch/arm/mpu/{ => arm32}/domain-page.c (100%)
> Uhm, why?
>
> Arm64 is using domain-page.c:
> https://gitlab.com/xen-project/people/lucafancellu/xen/-/commit/b28198d00078991d4a6502e94c8779d84fec0120
>
> Did I miss something?
Oh, I did not look at the future patches. I can drop this patch in my
next series once you and Michal/Julien reviews the other patches.
I feel it is cleaner to have separate implementations for
prepare_selector(), read_protection_region(), write_protection_region()
between arm32 and arm64. (Refer patch 4 and 6)
Let me know how it looks like.
- Ayan
>
> Cheers,
> Luca
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
@ 2025-06-12 9:35 ` Luca Fancellu
2025-06-12 10:37 ` Ayan Kumar Halder
2025-06-13 9:30 ` Ayan Kumar Halder
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Luca Fancellu @ 2025-06-12 9:35 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Define prepare_selector(), read_protection_region() and
> write_protection_region() for arm32. Also, define
> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>
> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
> Enable pr_of_addr() for arm32.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
Based on your v2 (https://patchwork.kernel.org/project/xen-devel/patch/20250606164854.1551148-4-ayan.kumar.halder@amd.com/) I was imaging something like this:
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 74e96ca57137..5d324b2d4ca5 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -87,20 +87,28 @@ static void __init __maybe_unused build_assertions(void)
*/
static void prepare_selector(uint8_t *sel)
{
-#ifdef CONFIG_ARM_64
uint8_t cur_sel = *sel;
+#ifdef CONFIG_ARM_64
/*
- * {read,write}_protection_region works using the direct access to the 0..15
- * regions, so in order to save the isb() overhead, change the PRSELR_EL2
- * only when needed, so when the upper 4 bits of the selector will change.
+ * {read,write}_protection_region works using the Arm64 direct access to the
+ * 0..15 regions, so in order to save the isb() overhead, change the
+ * PRSELR_EL2 only when needed, so when the upper 4 bits of the selector
+ * will change.
*/
cur_sel &= 0xF0U;
+#else
+ /* Arm32 MPU can use direct access for 0-31 */
+ if ( cur_sel > 31 )
+ cur_sel = 0;
+#endif
if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
{
WRITE_SYSREG(cur_sel, PRSELR_EL2);
isb();
}
+
+#ifdef CONFIG_ARM_64
*sel = *sel & 0xFU;
#endif
}
@@ -144,6 +152,12 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
GENERATE_READ_PR_REG_CASE(29, pr_read);
GENERATE_READ_PR_REG_CASE(30, pr_read);
GENERATE_READ_PR_REG_CASE(31, pr_read);
+ case 32 ... 255:
+ {
+ pr->prbar.bits = READ_SYSREG(PRBAR_EL2);
+ pr->prlar.bits = READ_SYSREG(PRLAR_EL2);
+ break;
+ }
#endif
default:
BUG(); /* Can't happen */
@@ -190,6 +204,12 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
GENERATE_WRITE_PR_REG_CASE(29, pr_write);
GENERATE_WRITE_PR_REG_CASE(30, pr_write);
GENERATE_WRITE_PR_REG_CASE(31, pr_write);
+ case 32 ... 255:
+ {
+ WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2);
+ WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2);
+ break;
+ }
#endif
default:
BUG(); /* Can't happen */
Is it using too ifdefs in your opinion that would benefit the split you do in v3?
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-12 9:35 ` Luca Fancellu
@ 2025-06-12 10:37 ` Ayan Kumar Halder
2025-06-13 7:00 ` Orzel, Michal
0 siblings, 1 reply; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-12 10:37 UTC (permalink / raw)
To: Luca Fancellu, Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
On 12/06/2025 10:35, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
>
>> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> Define prepare_selector(), read_protection_region() and
>> write_protection_region() for arm32. Also, define
>> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>>
>> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
>> Enable pr_of_addr() for arm32.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
> Based on your v2 (https://patchwork.kernel.org/project/xen-devel/patch/20250606164854.1551148-4-ayan.kumar.halder@amd.com/) I was imaging something like this:
>
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 74e96ca57137..5d324b2d4ca5 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -87,20 +87,28 @@ static void __init __maybe_unused build_assertions(void)
> */
> static void prepare_selector(uint8_t *sel)
> {
> -#ifdef CONFIG_ARM_64
> uint8_t cur_sel = *sel;
>
> +#ifdef CONFIG_ARM_64
> /*
> - * {read,write}_protection_region works using the direct access to the 0..15
> - * regions, so in order to save the isb() overhead, change the PRSELR_EL2
> - * only when needed, so when the upper 4 bits of the selector will change.
> + * {read,write}_protection_region works using the Arm64 direct access to the
> + * 0..15 regions, so in order to save the isb() overhead, change the
> + * PRSELR_EL2 only when needed, so when the upper 4 bits of the selector
> + * will change.
> */
> cur_sel &= 0xF0U;
> +#else
> + /* Arm32 MPU can use direct access for 0-31 */
> + if ( cur_sel > 31 )
> + cur_sel = 0;
> +#endif
> if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
> {
> WRITE_SYSREG(cur_sel, PRSELR_EL2);
> isb();
> }
> +
> +#ifdef CONFIG_ARM_64
> *sel = *sel & 0xFU;
> #endif
> }
> @@ -144,6 +152,12 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
> GENERATE_READ_PR_REG_CASE(29, pr_read);
> GENERATE_READ_PR_REG_CASE(30, pr_read);
> GENERATE_READ_PR_REG_CASE(31, pr_read);
> + case 32 ... 255:
> + {
> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2);
> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2);
> + break;
> + }
> #endif
> default:
> BUG(); /* Can't happen */
> @@ -190,6 +204,12 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
> GENERATE_WRITE_PR_REG_CASE(29, pr_write);
> GENERATE_WRITE_PR_REG_CASE(30, pr_write);
> GENERATE_WRITE_PR_REG_CASE(31, pr_write);
> + case 32 ... 255:
> + {
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2);
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2);
> + break;
> + }
> #endif
> default:
> BUG(); /* Can't happen */
>
>
> Is it using too ifdefs in your opinion that would benefit the split you do in v3?
Yes. However, I understand that this is subjective. I need your and
Michal/Julien to have an opinion here whether to go with the split
(which means some amount of code duplication) or introduce if-defs. I
will be happy to proceed as per your opinions.
- Ayan
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-12 10:37 ` Ayan Kumar Halder
@ 2025-06-13 7:00 ` Orzel, Michal
0 siblings, 0 replies; 23+ messages in thread
From: Orzel, Michal @ 2025-06-13 7:00 UTC (permalink / raw)
To: Ayan Kumar Halder, Luca Fancellu, Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk
On 12/06/2025 12:37, Ayan Kumar Halder wrote:
>
> On 12/06/2025 10:35, Luca Fancellu wrote:
>> Hi Ayan,
> Hi Luca,
>>
>>> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>>
>>> Define prepare_selector(), read_protection_region() and
>>> write_protection_region() for arm32. Also, define
>>> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>>>
>>> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
>>> Enable pr_of_addr() for arm32.
>>>
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> ---
>> Based on your v2 (https://patchwork.kernel.org/project/xen-devel/patch/20250606164854.1551148-4-ayan.kumar.halder@amd.com/) I was imaging something like this:
>>
>> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
>> index 74e96ca57137..5d324b2d4ca5 100644
>> --- a/xen/arch/arm/mpu/mm.c
>> +++ b/xen/arch/arm/mpu/mm.c
>> @@ -87,20 +87,28 @@ static void __init __maybe_unused build_assertions(void)
>> */
>> static void prepare_selector(uint8_t *sel)
>> {
>> -#ifdef CONFIG_ARM_64
>> uint8_t cur_sel = *sel;
>>
>> +#ifdef CONFIG_ARM_64
>> /*
>> - * {read,write}_protection_region works using the direct access to the 0..15
>> - * regions, so in order to save the isb() overhead, change the PRSELR_EL2
>> - * only when needed, so when the upper 4 bits of the selector will change.
>> + * {read,write}_protection_region works using the Arm64 direct access to the
>> + * 0..15 regions, so in order to save the isb() overhead, change the
>> + * PRSELR_EL2 only when needed, so when the upper 4 bits of the selector
>> + * will change.
>> */
>> cur_sel &= 0xF0U;
>> +#else
>> + /* Arm32 MPU can use direct access for 0-31 */
>> + if ( cur_sel > 31 )
>> + cur_sel = 0;
>> +#endif
>> if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
>> {
>> WRITE_SYSREG(cur_sel, PRSELR_EL2);
>> isb();
>> }
>> +
>> +#ifdef CONFIG_ARM_64
>> *sel = *sel & 0xFU;
>> #endif
>> }
>> @@ -144,6 +152,12 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
>> GENERATE_READ_PR_REG_CASE(29, pr_read);
>> GENERATE_READ_PR_REG_CASE(30, pr_read);
>> GENERATE_READ_PR_REG_CASE(31, pr_read);
>> + case 32 ... 255:
>> + {
>> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2);
>> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2);
>> + break;
>> + }
>> #endif
>> default:
>> BUG(); /* Can't happen */
>> @@ -190,6 +204,12 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
>> GENERATE_WRITE_PR_REG_CASE(29, pr_write);
>> GENERATE_WRITE_PR_REG_CASE(30, pr_write);
>> GENERATE_WRITE_PR_REG_CASE(31, pr_write);
>> + case 32 ... 255:
>> + {
>> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2);
>> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2);
>> + break;
>> + }
>> #endif
>> default:
>> BUG(); /* Can't happen */
>>
>>
>> Is it using too ifdefs in your opinion that would benefit the split you do in v3?
>
> Yes. However, I understand that this is subjective. I need your and
> Michal/Julien to have an opinion here whether to go with the split
> (which means some amount of code duplication) or introduce if-defs. I
> will be happy to proceed as per your opinions.
I don't have a strong opinion here. Maybe I slightly prefer the split to avoid
ifdefery.
~Michal
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
2025-06-12 9:35 ` Luca Fancellu
@ 2025-06-13 9:30 ` Ayan Kumar Halder
2025-06-13 16:02 ` Luca Fancellu
2025-06-16 5:57 ` Luca Fancellu
3 siblings, 0 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-13 9:30 UTC (permalink / raw)
To: Ayan Kumar Halder, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
On 11/06/2025 15:35, Ayan Kumar Halder wrote:
> Define prepare_selector(), read_protection_region() and
> write_protection_region() for arm32. Also, define
> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>
> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
> Enable pr_of_addr() for arm32.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from :-
>
> v1 - 1. Enable write_protection_region() for aarch32.
>
> v2 - 1. Enable access to protection regions from 0 - 255.
>
> xen/arch/arm/include/asm/mpu.h | 2 -
> xen/arch/arm/mpu/arm32/Makefile | 1 +
> xen/arch/arm/mpu/arm32/mm.c | 165 ++++++++++++++++++++++++++++++++
> xen/arch/arm/mpu/mm.c | 2 -
> 4 files changed, 166 insertions(+), 4 deletions(-)
> create mode 100644 xen/arch/arm/mpu/arm32/mm.c
>
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 8f06ddac0f..63560c613b 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -25,7 +25,6 @@
>
> #ifndef __ASSEMBLY__
>
> -#ifdef CONFIG_ARM_64
> /*
> * Set base address of MPU protection region.
> *
> @@ -85,7 +84,6 @@ static inline bool region_is_valid(const pr_t *pr)
> {
> return pr->prlar.reg.en;
> }
> -#endif /* CONFIG_ARM_64 */
>
> #endif /* __ASSEMBLY__ */
>
> diff --git a/xen/arch/arm/mpu/arm32/Makefile b/xen/arch/arm/mpu/arm32/Makefile
> index e15ce2f7be..3da872322e 100644
> --- a/xen/arch/arm/mpu/arm32/Makefile
> +++ b/xen/arch/arm/mpu/arm32/Makefile
> @@ -1 +1,2 @@
> obj-y += domain-page.o
> +obj-y += mm.o
> diff --git a/xen/arch/arm/mpu/arm32/mm.c b/xen/arch/arm/mpu/arm32/mm.c
> new file mode 100644
> index 0000000000..5d3cb6dff7
> --- /dev/null
> +++ b/xen/arch/arm/mpu/arm32/mm.c
> @@ -0,0 +1,165 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bug.h>
> +#include <xen/types.h>
> +#include <asm/mpu.h>
> +#include <asm/sysregs.h>
> +#include <asm/system.h>
> +
> +#define PRBAR_EL2_(n) HPRBAR##n
> +#define PRLAR_EL2_(n) HPRLAR##n
> +
> +#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
> + break; \
> + }
> +
> +#define GENERATE_WRITE_PR_REG_OTHERS(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, HPRBAR); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, HPRLAR); \
> + break; \
> + }
> +
> +#define GENERATE_READ_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
> + break; \
> + }
> +
> +#define GENERATE_READ_PR_REG_OTHERS(num, pr) \
> + case num: \
> + { \
> + pr->prbar.bits = READ_SYSREG(HPRBAR); \
> + pr->prlar.bits = READ_SYSREG(HPRLAR); \
> + break; \
> + }
> +
> +/*
> + * Armv8-R supports direct access and indirect access to the MPU regions through
> + * registers:
> + * - indirect access involves changing the MPU region selector, issuing an isb
> + * barrier and accessing the selected region through specific registers
> + * - direct access involves accessing specific registers that point to
> + * specific MPU regions, without changing the selector, avoiding the use of
> + * a barrier.
> + * For Arm32 the PR{B,L}AR<n>_ELx (for n=0..31) are used for direct access to the
> + * first 32 MPU regions.
> + * For MPU region numbered 32..255, one need to set the region number in PRSELR_ELx,
> + * followed by configuring PR{B,L}AR_ELx.
> + */
> +inline void prepare_selector(uint8_t *sel)
> +{
> + uint8_t cur_sel = *sel;
> +
> + if ( cur_sel > 0x1FU )
> + {
> + WRITE_SYSREG(cur_sel, PRSELR_EL2);
> + isb();
> + }
> +}
> +
> +void read_protection_region(pr_t *pr_read, uint8_t sel)
> +{
> + prepare_selector(&sel);
> +
> + switch ( sel )
> + {
> + GENERATE_READ_PR_REG_CASE(0, pr_read);
> + GENERATE_READ_PR_REG_CASE(1, pr_read);
> + GENERATE_READ_PR_REG_CASE(2, pr_read);
> + GENERATE_READ_PR_REG_CASE(3, pr_read);
> + GENERATE_READ_PR_REG_CASE(4, pr_read);
> + GENERATE_READ_PR_REG_CASE(5, pr_read);
> + GENERATE_READ_PR_REG_CASE(6, pr_read);
> + GENERATE_READ_PR_REG_CASE(7, pr_read);
> + GENERATE_READ_PR_REG_CASE(8, pr_read);
> + GENERATE_READ_PR_REG_CASE(9, pr_read);
> + GENERATE_READ_PR_REG_CASE(10, pr_read);
> + GENERATE_READ_PR_REG_CASE(11, pr_read);
> + GENERATE_READ_PR_REG_CASE(12, pr_read);
> + GENERATE_READ_PR_REG_CASE(13, pr_read);
> + GENERATE_READ_PR_REG_CASE(14, pr_read);
> + GENERATE_READ_PR_REG_CASE(15, pr_read);
> + GENERATE_READ_PR_REG_CASE(16, pr_read);
> + GENERATE_READ_PR_REG_CASE(17, pr_read);
> + GENERATE_READ_PR_REG_CASE(18, pr_read);
> + GENERATE_READ_PR_REG_CASE(19, pr_read);
> + GENERATE_READ_PR_REG_CASE(20, pr_read);
> + GENERATE_READ_PR_REG_CASE(21, pr_read);
> + GENERATE_READ_PR_REG_CASE(22, pr_read);
> + GENERATE_READ_PR_REG_CASE(23, pr_read);
> + GENERATE_READ_PR_REG_CASE(24, pr_read);
> + GENERATE_READ_PR_REG_CASE(25, pr_read);
> + GENERATE_READ_PR_REG_CASE(26, pr_read);
> + GENERATE_READ_PR_REG_CASE(27, pr_read);
> + GENERATE_READ_PR_REG_CASE(28, pr_read);
> + GENERATE_READ_PR_REG_CASE(29, pr_read);
> + GENERATE_READ_PR_REG_CASE(30, pr_read);
> + GENERATE_READ_PR_REG_CASE(31, pr_read);
> + GENERATE_READ_PR_REG_OTHERS(32 ... 255, pr_read);
This should be 32 ... 254 . Thanks Luca for pointing this out.
The max number of regions supported is 255 (not 256). It is the maximum
value of HMPUIR.
- Ayan
> + default:
> + BUG(); /* Can't happen */
> + break;
> + }
> +}
> +
> +void write_protection_region(const pr_t *pr_write, uint8_t sel)
> +{
> + prepare_selector(&sel);
> +
> + switch ( sel )
> + {
> + GENERATE_WRITE_PR_REG_CASE(0, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(1, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(2, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(3, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(4, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(5, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(6, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(7, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(8, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(9, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(10, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(11, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(12, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(13, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(14, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(15, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(16, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(17, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(18, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(19, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(20, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(21, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(22, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(23, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(24, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(25, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(26, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(27, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(28, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(29, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(30, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(31, pr_write);
> + GENERATE_WRITE_PR_REG_OTHERS(32 ... 255, pr_write);
> + default:
> + BUG(); /* Can't happen */
> + break;
> + }
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 7ab68fc8c7..ccfb37a67b 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -39,7 +39,6 @@ static void __init __maybe_unused build_assertions(void)
> BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
> }
>
> -#ifdef CONFIG_ARM_64
> pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
> {
> unsigned int attr_idx = PAGE_AI_MASK(flags);
> @@ -110,7 +109,6 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
>
> return region;
> }
> -#endif /* CONFIG_ARM_64 */
>
> void __init setup_mm(void)
> {
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files
2025-06-11 14:35 ` [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files Ayan Kumar Halder
@ 2025-06-13 15:08 ` Luca Fancellu
2025-06-16 17:04 ` Ayan Kumar Halder
0 siblings, 1 reply; 23+ messages in thread
From: Luca Fancellu @ 2025-06-13 15:08 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> prepare_selector(), read_protection_region() and write_protection_region()
> differ significantly between arm32 and arm64. Thus, move these functions
> to their specific folders.
^— NIT: “to sub-arch specific folder”? What do you think?
>
> GENERATE_{WRITE/READ}_PR_REG_CASE are duplicated for arm32 and arm64 so
> as to improve the code readability.
It reads a bit hard in this way, what about:
“Also the macro GENERATE_{WRITE/READ}_PR_REG_CASE are moved, in order to
keep them in the same file of their usage and improve readability"
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from -
>
> v1..v2 - New patch introduced in v3.
>
> xen/arch/arm/mpu/Makefile | 1 +
> xen/arch/arm/mpu/arm64/Makefile | 1 +
> xen/arch/arm/mpu/arm64/mm.c | 130 ++++++++++++++++++++++++++++++++
> xen/arch/arm/mpu/mm.c | 117 ----------------------------
> 4 files changed, 132 insertions(+), 117 deletions(-)
> create mode 100644 xen/arch/arm/mpu/arm64/Makefile
> create mode 100644 xen/arch/arm/mpu/arm64/mm.c
>
> diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile
> index 9359d79332..4963c8b550 100644
> --- a/xen/arch/arm/mpu/Makefile
> +++ b/xen/arch/arm/mpu/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_ARM_32) += arm32/
> +obj-$(CONFIG_ARM_64) += arm64/
> obj-y += mm.o
> obj-y += p2m.o
> obj-y += setup.init.o
> diff --git a/xen/arch/arm/mpu/arm64/Makefile b/xen/arch/arm/mpu/arm64/Makefile
> new file mode 100644
> index 0000000000..b18cec4836
> --- /dev/null
> +++ b/xen/arch/arm/mpu/arm64/Makefile
> @@ -0,0 +1 @@
> +obj-y += mm.o
> diff --git a/xen/arch/arm/mpu/arm64/mm.c b/xen/arch/arm/mpu/arm64/mm.c
> new file mode 100644
> index 0000000000..a978c1fc6e
> --- /dev/null
> +++ b/xen/arch/arm/mpu/arm64/mm.c
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bug.h>
> +#include <xen/types.h>
> +#include <asm/mpu.h>
> +#include <asm/sysregs.h>
> +#include <asm/system.h>
> +
> +/*
> + * The following are needed for the cases: GENERATE_WRITE_PR_REG_CASE
> + * and GENERATE_READ_PR_REG_CASE with num==0
> + */
> +#define PRBAR0_EL2 PRBAR_EL2
> +#define PRLAR0_EL2 PRLAR_EL2
> +
> +#define PRBAR_EL2_(n) PRBAR##n##_EL2
> +#define PRLAR_EL2_(n) PRLAR##n##_EL2
> +
> +#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
> + break; \
> + }
> +
> +#define GENERATE_READ_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
> + break; \
> + }
> +
> +/*
> + * Armv8-R supports direct access and indirect access to the MPU regions through
> + * registers:
> + * - indirect access involves changing the MPU region selector, issuing an isb
> + * barrier and accessing the selected region through specific registers
> + * - direct access involves accessing specific registers that point to
> + * specific MPU regions, without changing the selector, avoiding the use of
> + * a barrier.
> + * For Arm64 the PR{B,L}AR_ELx (for n=0) and PR{B,L}AR<n>_ELx (for n=1..15) are
> + * used for the direct access to the regions selected by
> + * PRSELR_EL2.REGION<7:4>:n, so 16 regions can be directly accessed when the
> + * selector is a multiple of 16, giving access to all the supported memory
> + * regions.
> + */
> +static void prepare_selector(uint8_t *sel)
> +{
> + uint8_t cur_sel = *sel;
> +
> + /*
> + * {read,write}_protection_region works using the direct access to the 0..15
> + * regions, so in order to save the isb() overhead, change the PRSELR_EL2
> + * only when needed, so when the upper 4 bits of the selector will change.
> + */
> + cur_sel &= 0xF0U;
> + if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
> + {
> + WRITE_SYSREG(cur_sel, PRSELR_EL2);
> + isb();
> + }
> + *sel = *sel & 0xFU;
This one is different in the original file (*sel &= 0xFU;)
The rest looks good to me!
With the above fixed:
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Cheers,
Luca
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
@ 2025-06-13 15:30 ` Luca Fancellu
2025-06-16 8:16 ` Julien Grall
1 sibling, 0 replies; 23+ messages in thread
From: Luca Fancellu @ 2025-06-13 15:30 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Introduce pr_t typedef which is a structure having the prbar and prlar members,
> each being structured as the registers of the AArch32 Armv8-R architecture.
>
> Also, define MPU_REGION_RES0 to 0 as there are no reserved 0 bits beyond the
> BASE or LIMIT bitfields in prbar or prlar respectively.
>
> In pr_of_addr(), enclose prbar and prlar arm64 specific bitfields with
> appropriate macros. So, that this function can be later reused for arm32 as
> well.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
LGTM! I’ve also built for Arm32 and Arm64.
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures
2025-06-11 14:35 ` [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures Ayan Kumar Halder
@ 2025-06-13 15:39 ` Luca Fancellu
2025-06-16 8:25 ` Julien Grall
1 sibling, 0 replies; 23+ messages in thread
From: Luca Fancellu @ 2025-06-13 15:39 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Modify Arm32 assembly boot code to reset any unused MPU region, initialise
> 'max_mpu_regions' with the number of supported MPU regions and set/clear the
> bitmap 'xen_mpumap_mask' used to track the enabled regions.
>
> Introduce cache.S to hold arm32 cache related functions.
>
> Use the macro definition for "dcache_line_size" from linux.
>
> Change the order of registers in prepare_xen_region() as 'strd' instruction
> is used to store {prbar, prlar} in arm32. Thus, 'prbar' has to be a even
> numbered register and 'prlar' is the consecutively ordered register.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
This LGTM, I’ve also built for Arm64 and Arm32.
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
2025-06-12 9:35 ` Luca Fancellu
2025-06-13 9:30 ` Ayan Kumar Halder
@ 2025-06-13 16:02 ` Luca Fancellu
2025-06-16 5:57 ` Luca Fancellu
3 siblings, 0 replies; 23+ messages in thread
From: Luca Fancellu @ 2025-06-13 16:02 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Define prepare_selector(), read_protection_region() and
> write_protection_region() for arm32. Also, define
> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>
> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
> Enable pr_of_addr() for arm32.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from :-
>
> v1 - 1. Enable write_protection_region() for aarch32.
>
> v2 - 1. Enable access to protection regions from 0 - 255.
>
> xen/arch/arm/include/asm/mpu.h | 2 -
> xen/arch/arm/mpu/arm32/Makefile | 1 +
> xen/arch/arm/mpu/arm32/mm.c | 165 ++++++++++++++++++++++++++++++++
> xen/arch/arm/mpu/mm.c | 2 -
> 4 files changed, 166 insertions(+), 4 deletions(-)
> create mode 100644 xen/arch/arm/mpu/arm32/mm.c
>
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 8f06ddac0f..63560c613b 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -25,7 +25,6 @@
>
> #ifndef __ASSEMBLY__
>
> -#ifdef CONFIG_ARM_64
> /*
> * Set base address of MPU protection region.
> *
> @@ -85,7 +84,6 @@ static inline bool region_is_valid(const pr_t *pr)
> {
> return pr->prlar.reg.en;
> }
> -#endif /* CONFIG_ARM_64 */
>
> #endif /* __ASSEMBLY__ */
>
> diff --git a/xen/arch/arm/mpu/arm32/Makefile b/xen/arch/arm/mpu/arm32/Makefile
> index e15ce2f7be..3da872322e 100644
> --- a/xen/arch/arm/mpu/arm32/Makefile
> +++ b/xen/arch/arm/mpu/arm32/Makefile
> @@ -1 +1,2 @@
> obj-y += domain-page.o
> +obj-y += mm.o
> diff --git a/xen/arch/arm/mpu/arm32/mm.c b/xen/arch/arm/mpu/arm32/mm.c
> new file mode 100644
> index 0000000000..5d3cb6dff7
> --- /dev/null
> +++ b/xen/arch/arm/mpu/arm32/mm.c
> @@ -0,0 +1,165 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bug.h>
> +#include <xen/types.h>
> +#include <asm/mpu.h>
> +#include <asm/sysregs.h>
> +#include <asm/system.h>
> +
> +#define PRBAR_EL2_(n) HPRBAR##n
> +#define PRLAR_EL2_(n) HPRLAR##n
> +
> +#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
Maybe you don’t need '& ~MPU_REGION_RES0’ since your MPU_REGION_RES0 is zero, here and below
> + break; \
> + }
> +
> +#define GENERATE_WRITE_PR_REG_OTHERS(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, HPRBAR); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, HPRLAR); \
> + break; \
> + }
> +
> +#define GENERATE_READ_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
> + break; \
> + }
> +
> +#define GENERATE_READ_PR_REG_OTHERS(num, pr) \
> + case num: \
> + { \
> + pr->prbar.bits = READ_SYSREG(HPRBAR); \
> + pr->prlar.bits = READ_SYSREG(HPRLAR); \
> + break; \
> + }
> +
> +/*
> + * Armv8-R supports direct access and indirect access to the MPU regions through
> + * registers:
> + * - indirect access involves changing the MPU region selector, issuing an isb
> + * barrier and accessing the selected region through specific registers
> + * - direct access involves accessing specific registers that point to
> + * specific MPU regions, without changing the selector, avoiding the use of
> + * a barrier.
> + * For Arm32 the PR{B,L}AR<n>_ELx (for n=0..31) are used for direct access to the
Arm32 have PR{B,L}AR but it’s not EL2, you mean HPRBAR here and below
> + * first 32 MPU regions.
> + * For MPU region numbered 32..255, one need to set the region number in PRSELR_ELx,
32..254
and also maybe you can use HPRSELR instead of PRSELR_ELx
> + * followed by configuring PR{B,L}AR_ELx.
> + */
> +inline void prepare_selector(uint8_t *sel)
> +{
> + uint8_t cur_sel = *sel;
> +
> + if ( cur_sel > 0x1FU )
can we use 31 here? instead of the hex? It would be quicker to be read by a developer.
> + {
> + WRITE_SYSREG(cur_sel, PRSELR_EL2);
> + isb();
> + }
> +}
> +
> +void read_protection_region(pr_t *pr_read, uint8_t sel)
> +{
> + prepare_selector(&sel);
> +
> + switch ( sel )
> + {
> + GENERATE_READ_PR_REG_CASE(0, pr_read);
> + GENERATE_READ_PR_REG_CASE(1, pr_read);
> + GENERATE_READ_PR_REG_CASE(2, pr_read);
> + GENERATE_READ_PR_REG_CASE(3, pr_read);
> + GENERATE_READ_PR_REG_CASE(4, pr_read);
> + GENERATE_READ_PR_REG_CASE(5, pr_read);
> + GENERATE_READ_PR_REG_CASE(6, pr_read);
> + GENERATE_READ_PR_REG_CASE(7, pr_read);
> + GENERATE_READ_PR_REG_CASE(8, pr_read);
> + GENERATE_READ_PR_REG_CASE(9, pr_read);
> + GENERATE_READ_PR_REG_CASE(10, pr_read);
> + GENERATE_READ_PR_REG_CASE(11, pr_read);
> + GENERATE_READ_PR_REG_CASE(12, pr_read);
> + GENERATE_READ_PR_REG_CASE(13, pr_read);
> + GENERATE_READ_PR_REG_CASE(14, pr_read);
> + GENERATE_READ_PR_REG_CASE(15, pr_read);
> + GENERATE_READ_PR_REG_CASE(16, pr_read);
> + GENERATE_READ_PR_REG_CASE(17, pr_read);
> + GENERATE_READ_PR_REG_CASE(18, pr_read);
> + GENERATE_READ_PR_REG_CASE(19, pr_read);
> + GENERATE_READ_PR_REG_CASE(20, pr_read);
> + GENERATE_READ_PR_REG_CASE(21, pr_read);
> + GENERATE_READ_PR_REG_CASE(22, pr_read);
> + GENERATE_READ_PR_REG_CASE(23, pr_read);
> + GENERATE_READ_PR_REG_CASE(24, pr_read);
> + GENERATE_READ_PR_REG_CASE(25, pr_read);
> + GENERATE_READ_PR_REG_CASE(26, pr_read);
> + GENERATE_READ_PR_REG_CASE(27, pr_read);
> + GENERATE_READ_PR_REG_CASE(28, pr_read);
> + GENERATE_READ_PR_REG_CASE(29, pr_read);
> + GENERATE_READ_PR_REG_CASE(30, pr_read);
> + GENERATE_READ_PR_REG_CASE(31, pr_read);
> + GENERATE_READ_PR_REG_OTHERS(32 ... 255, pr_read);
32 … 254 here and in the below.
> + default:
> + BUG(); /* Can't happen */
> + break;
> + }
> +}
> +
> +void write_protection_region(const pr_t *pr_write, uint8_t sel)
> +{
> + prepare_selector(&sel);
> +
> + switch ( sel )
> + {
> + GENERATE_WRITE_PR_REG_CASE(0, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(1, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(2, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(3, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(4, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(5, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(6, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(7, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(8, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(9, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(10, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(11, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(12, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(13, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(14, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(15, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(16, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(17, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(18, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(19, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(20, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(21, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(22, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(23, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(24, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(25, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(26, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(27, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(28, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(29, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(30, pr_write);
> + GENERATE_WRITE_PR_REG_CASE(31, pr_write);
> + GENERATE_WRITE_PR_REG_OTHERS(32 ... 255, pr_write);
> + default:
> + BUG(); /* Can't happen */
> + break;
> + }
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 7ab68fc8c7..ccfb37a67b 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -39,7 +39,6 @@ static void __init __maybe_unused build_assertions(void)
> BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
> }
>
> -#ifdef CONFIG_ARM_64
> pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
> {
> unsigned int attr_idx = PAGE_AI_MASK(flags);
> @@ -110,7 +109,6 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
>
> return region;
> }
> -#endif /* CONFIG_ARM_64 */
>
> void __init setup_mm(void)
> {
> --
> 2.25.1
>
>
Cheers,
Luca
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
` (2 preceding siblings ...)
2025-06-13 16:02 ` Luca Fancellu
@ 2025-06-16 5:57 ` Luca Fancellu
3 siblings, 0 replies; 23+ messages in thread
From: Luca Fancellu @ 2025-06-16 5:57 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Hi Ayan,
> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>
> Define prepare_selector(), read_protection_region() and
> write_protection_region() for arm32. Also, define
> GENERATE_{READ/WRITE}_PR_REG_OTHERS to access MPU regions from 32 to 255.
>
> Enable pr_{get/set}_{base/limit}(), region_is_valid() for arm32.
> Enable pr_of_addr() for arm32.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> Changes from :-
>
> v1 - 1. Enable write_protection_region() for aarch32.
>
> v2 - 1. Enable access to protection regions from 0 - 255.
>
> xen/arch/arm/include/asm/mpu.h | 2 -
> xen/arch/arm/mpu/arm32/Makefile | 1 +
> xen/arch/arm/mpu/arm32/mm.c | 165 ++++++++++++++++++++++++++++++++
> xen/arch/arm/mpu/mm.c | 2 -
> 4 files changed, 166 insertions(+), 4 deletions(-)
> create mode 100644 xen/arch/arm/mpu/arm32/mm.c
>
> diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
> index 8f06ddac0f..63560c613b 100644
> --- a/xen/arch/arm/include/asm/mpu.h
> +++ b/xen/arch/arm/include/asm/mpu.h
> @@ -25,7 +25,6 @@
>
> #ifndef __ASSEMBLY__
>
> -#ifdef CONFIG_ARM_64
> /*
> * Set base address of MPU protection region.
> *
> @@ -85,7 +84,6 @@ static inline bool region_is_valid(const pr_t *pr)
> {
> return pr->prlar.reg.en;
> }
> -#endif /* CONFIG_ARM_64 */
>
> #endif /* __ASSEMBLY__ */
>
> diff --git a/xen/arch/arm/mpu/arm32/Makefile b/xen/arch/arm/mpu/arm32/Makefile
> index e15ce2f7be..3da872322e 100644
> --- a/xen/arch/arm/mpu/arm32/Makefile
> +++ b/xen/arch/arm/mpu/arm32/Makefile
> @@ -1 +1,2 @@
> obj-y += domain-page.o
> +obj-y += mm.o
> diff --git a/xen/arch/arm/mpu/arm32/mm.c b/xen/arch/arm/mpu/arm32/mm.c
> new file mode 100644
> index 0000000000..5d3cb6dff7
> --- /dev/null
> +++ b/xen/arch/arm/mpu/arm32/mm.c
> @@ -0,0 +1,165 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bug.h>
> +#include <xen/types.h>
> +#include <asm/mpu.h>
> +#include <asm/sysregs.h>
> +#include <asm/system.h>
> +
> +#define PRBAR_EL2_(n) HPRBAR##n
> +#define PRLAR_EL2_(n) HPRLAR##n
> +
> +#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
> + case num: \
> + { \
> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
I was also thinking that in this file now you can use directly HPR{B,L}AR<N> instead of PR{B,L}AR<N>_EL2
Cheers,
Luca
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
2025-06-13 15:30 ` Luca Fancellu
@ 2025-06-16 8:16 ` Julien Grall
1 sibling, 0 replies; 23+ messages in thread
From: Julien Grall @ 2025-06-16 8:16 UTC (permalink / raw)
To: Ayan Kumar Halder, xen-devel
Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
Hi Ayan,
On 11/06/2025 15:35, Ayan Kumar Halder wrote:
> Introduce pr_t typedef which is a structure having the prbar and prlar members,
> each being structured as the registers of the AArch32 Armv8-R architecture.
>
> Also, define MPU_REGION_RES0 to 0 as there are no reserved 0 bits beyond the
> BASE or LIMIT bitfields in prbar or prlar respectively.
>
> In pr_of_addr(), enclose prbar and prlar arm64 specific bitfields with
> appropriate macros. So, that this function can be later reused for arm32 as
> well.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
> ---
> Changes from v1 :-
>
> 1. Preserve pr_t typedef in arch specific files.
>
> 2. Fix typo.
>
> v2 :-
>
> 1. Change CONFIG_ARM64 to CONFIG_ARM_64 to enclose arm64 specific bitfields for
> prbar and prlar registers in pr_of_addr().
>
> xen/arch/arm/include/asm/arm32/mpu.h | 34 ++++++++++++++++++++++++++--
> xen/arch/arm/mpu/mm.c | 4 ++++
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/arm32/mpu.h b/xen/arch/arm/include/asm/arm32/mpu.h
> index f0d4d4055c..0a6930b3a0 100644
> --- a/xen/arch/arm/include/asm/arm32/mpu.h
> +++ b/xen/arch/arm/include/asm/arm32/mpu.h
> @@ -5,10 +5,40 @@
>
> #ifndef __ASSEMBLY__
>
> +/*
> + * Unlike arm64, there are no reserved 0 bits beyond base and limit bitfield in
> + * prbar and prlar registers respectively.
> + */
> +#define MPU_REGION_RES0 0x0
> +
> +/* Hypervisor Protection Region Base Address Register */
> +typedef union {
> + struct {
> + unsigned int xn:1; /* Execute-Never */
> + unsigned int ap_0:1; /* Access Permission AP[0] */
> + unsigned int ro:1; /* Access Permission AP[1] */
> + unsigned int sh:2; /* Shareability */
> + unsigned int res0:1;
> + unsigned int base:26; /* Base Address */
> + } reg;
> + uint32_t bits;
> +} prbar_t;
> +
> +/* Hypervisor Protection Region Limit Address Register */
> +typedef union {
> + struct {
> + unsigned int en:1; /* Region enable */
> + unsigned int ai:3; /* Memory Attribute Index */
> + unsigned int res0:2;
> + unsigned int limit:26; /* Limit Address */
> + } reg;
> + uint32_t bits;
> +} prlar_t;
> +
> /* MPU Protection Region */
> typedef struct {
> - uint32_t prbar;
> - uint32_t prlar;
> + prbar_t prbar;
> + prlar_t prlar;
> } pr_t;
>
> #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 86fbe105af..3d37beab57 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -167,7 +167,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
> /* Build up value for PRBAR_EL2. */
> prbar = (prbar_t) {
> .reg = {
> +#ifdef CONFIG_ARM_64
> .xn_0 = 0,
> +#endif
> .xn = PAGE_XN_MASK(flags),
> .ap_0 = 0,
> .ro = PAGE_RO_MASK(flags)
> @@ -206,7 +208,9 @@ pr_t pr_of_addr(paddr_t base, paddr_t limit, unsigned int flags)
> /* 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 */
> }};
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures
2025-06-11 14:35 ` [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures Ayan Kumar Halder
2025-06-13 15:39 ` Luca Fancellu
@ 2025-06-16 8:25 ` Julien Grall
1 sibling, 0 replies; 23+ messages in thread
From: Julien Grall @ 2025-06-16 8:25 UTC (permalink / raw)
To: Ayan Kumar Halder, xen-devel
Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk
Hi Ayan,
On 11/06/2025 15:35, Ayan Kumar Halder wrote:
> Modify Arm32 assembly boot code to reset any unused MPU region, initialise
> 'max_mpu_regions' with the number of supported MPU regions and set/clear the
> bitmap 'xen_mpumap_mask' used to track the enabled regions.
>
> Introduce cache.S to hold arm32 cache related functions.
>
> Use the macro definition for "dcache_line_size" from linux.
>
> Change the order of registers in prepare_xen_region() as 'strd' instruction
> is used to store {prbar, prlar} in arm32. Thus, 'prbar' has to be a even
> numbered register and 'prlar' is the consecutively ordered register.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/6] arm/mpu: Define arm32 system registers
2025-06-11 14:35 ` [PATCH v3 5/6] arm/mpu: Define arm32 system registers Ayan Kumar Halder
@ 2025-06-16 10:38 ` Hari Limaye
2025-06-16 17:06 ` Ayan Kumar Halder
0 siblings, 1 reply; 23+ messages in thread
From: Hari Limaye @ 2025-06-16 10:38 UTC (permalink / raw)
To: Ayan Kumar Halder
Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
Michal Orzel, Volodymyr Babchuk
Hi Ayan,
I checked the register definitions for HPR{B,L}AR<n> against the Arm
Architecture Reference Manual Supplement for the Armv8-R AArch32
architecture profile (ARM DDI 0568A.c), specifically sections E2.2.3 and
E2.2.6, and everything looks correct to me.
On Wed, Jun 11, 2025 at 03:35:43PM +0000, Ayan Kumar Halder wrote:
> Fix the definition for HPRLAR.
> Define the base/limit address registers to access the first 32 protection
> regions.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
Reviewed-by: Hari Limaye <hari.limaye@arm.com>
Many thanks,
Hari
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files
2025-06-13 15:08 ` Luca Fancellu
@ 2025-06-16 17:04 ` Ayan Kumar Halder
0 siblings, 0 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-16 17:04 UTC (permalink / raw)
To: Luca Fancellu, Ayan Kumar Halder
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
On 13/06/2025 16:08, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
>
>> On 11 Jun 2025, at 15:35, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> prepare_selector(), read_protection_region() and write_protection_region()
>> differ significantly between arm32 and arm64. Thus, move these functions
>> to their specific folders.
> ^— NIT: “to sub-arch specific folder”? What do you think?
yes
>
>> GENERATE_{WRITE/READ}_PR_REG_CASE are duplicated for arm32 and arm64 so
>> as to improve the code readability.
> It reads a bit hard in this way, what about:
>
> “Also the macro GENERATE_{WRITE/READ}_PR_REG_CASE are moved, in order to
> keep them in the same file of their usage and improve readability"
yes, this reads better.
>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>> Changes from -
>>
>> v1..v2 - New patch introduced in v3.
>>
>> xen/arch/arm/mpu/Makefile | 1 +
>> xen/arch/arm/mpu/arm64/Makefile | 1 +
>> xen/arch/arm/mpu/arm64/mm.c | 130 ++++++++++++++++++++++++++++++++
>> xen/arch/arm/mpu/mm.c | 117 ----------------------------
>> 4 files changed, 132 insertions(+), 117 deletions(-)
>> create mode 100644 xen/arch/arm/mpu/arm64/Makefile
>> create mode 100644 xen/arch/arm/mpu/arm64/mm.c
>>
>> diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile
>> index 9359d79332..4963c8b550 100644
>> --- a/xen/arch/arm/mpu/Makefile
>> +++ b/xen/arch/arm/mpu/Makefile
>> @@ -1,4 +1,5 @@
>> obj-$(CONFIG_ARM_32) += arm32/
>> +obj-$(CONFIG_ARM_64) += arm64/
>> obj-y += mm.o
>> obj-y += p2m.o
>> obj-y += setup.init.o
>> diff --git a/xen/arch/arm/mpu/arm64/Makefile b/xen/arch/arm/mpu/arm64/Makefile
>> new file mode 100644
>> index 0000000000..b18cec4836
>> --- /dev/null
>> +++ b/xen/arch/arm/mpu/arm64/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += mm.o
>> diff --git a/xen/arch/arm/mpu/arm64/mm.c b/xen/arch/arm/mpu/arm64/mm.c
>> new file mode 100644
>> index 0000000000..a978c1fc6e
>> --- /dev/null
>> +++ b/xen/arch/arm/mpu/arm64/mm.c
>> @@ -0,0 +1,130 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/bug.h>
>> +#include <xen/types.h>
>> +#include <asm/mpu.h>
>> +#include <asm/sysregs.h>
>> +#include <asm/system.h>
>> +
>> +/*
>> + * The following are needed for the cases: GENERATE_WRITE_PR_REG_CASE
>> + * and GENERATE_READ_PR_REG_CASE with num==0
>> + */
>> +#define PRBAR0_EL2 PRBAR_EL2
>> +#define PRLAR0_EL2 PRLAR_EL2
>> +
>> +#define PRBAR_EL2_(n) PRBAR##n##_EL2
>> +#define PRLAR_EL2_(n) PRLAR##n##_EL2
>> +
>> +#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
>> + case num: \
>> + { \
>> + WRITE_SYSREG(pr->prbar.bits & ~MPU_REGION_RES0, PRBAR_EL2_(num)); \
>> + WRITE_SYSREG(pr->prlar.bits & ~MPU_REGION_RES0, PRLAR_EL2_(num)); \
>> + break; \
>> + }
>> +
>> +#define GENERATE_READ_PR_REG_CASE(num, pr) \
>> + case num: \
>> + { \
>> + pr->prbar.bits = READ_SYSREG(PRBAR_EL2_(num)); \
>> + pr->prlar.bits = READ_SYSREG(PRLAR_EL2_(num)); \
>> + break; \
>> + }
>> +
>> +/*
>> + * Armv8-R supports direct access and indirect access to the MPU regions through
>> + * registers:
>> + * - indirect access involves changing the MPU region selector, issuing an isb
>> + * barrier and accessing the selected region through specific registers
>> + * - direct access involves accessing specific registers that point to
>> + * specific MPU regions, without changing the selector, avoiding the use of
>> + * a barrier.
>> + * For Arm64 the PR{B,L}AR_ELx (for n=0) and PR{B,L}AR<n>_ELx (for n=1..15) are
>> + * used for the direct access to the regions selected by
>> + * PRSELR_EL2.REGION<7:4>:n, so 16 regions can be directly accessed when the
>> + * selector is a multiple of 16, giving access to all the supported memory
>> + * regions.
>> + */
>> +static void prepare_selector(uint8_t *sel)
>> +{
>> + uint8_t cur_sel = *sel;
>> +
>> + /*
>> + * {read,write}_protection_region works using the direct access to the 0..15
>> + * regions, so in order to save the isb() overhead, change the PRSELR_EL2
>> + * only when needed, so when the upper 4 bits of the selector will change.
>> + */
>> + cur_sel &= 0xF0U;
>> + if ( READ_SYSREG(PRSELR_EL2) != cur_sel )
>> + {
>> + WRITE_SYSREG(cur_sel, PRSELR_EL2);
>> + isb();
>> + }
>> + *sel = *sel & 0xFU;
> This one is different in the original file (*sel &= 0xFU;)
Agree with all of the suggested changes.
>
> The rest looks good to me!
> With the above fixed:
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
- Ayan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/6] arm/mpu: Define arm32 system registers
2025-06-16 10:38 ` Hari Limaye
@ 2025-06-16 17:06 ` Ayan Kumar Halder
0 siblings, 0 replies; 23+ messages in thread
From: Ayan Kumar Halder @ 2025-06-16 17:06 UTC (permalink / raw)
To: Hari Limaye, Ayan Kumar Halder
Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
Michal Orzel, Volodymyr Babchuk
On 16/06/2025 11:38, Hari Limaye wrote:
> Hi Ayan,
Hi Hari,
>
> I checked the register definitions for HPR{B,L}AR<n> against the Arm
> Architecture Reference Manual Supplement for the Armv8-R AArch32
> architecture profile (ARM DDI 0568A.c), specifically sections E2.2.3 and
> E2.2.6, and everything looks correct to me.
>
> On Wed, Jun 11, 2025 at 03:35:43PM +0000, Ayan Kumar Halder wrote:
>> Fix the definition for HPRLAR.
>> Define the base/limit address registers to access the first 32 protection
>> regions.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
> Reviewed-by: Hari Limaye <hari.limaye@arm.com>
Thanks and welcome to xen-devel. :)
- Ayan
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-06-16 17:06 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 14:35 [PATCH v3 0/6] Enable R52 support for the first chunk of MPU support Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 1/6] arm/mpu: Introduce MPU memory region map structure Ayan Kumar Halder
2025-06-13 15:30 ` Luca Fancellu
2025-06-16 8:16 ` Julien Grall
2025-06-11 14:35 ` [PATCH v3 2/6] arm/mpu: Provide and populate MPU C data structures Ayan Kumar Halder
2025-06-13 15:39 ` Luca Fancellu
2025-06-16 8:25 ` Julien Grall
2025-06-11 14:35 ` [PATCH v3 3/6] arm/mpu: Move domain-page.c to arm32 specific dir Ayan Kumar Halder
2025-06-11 19:46 ` Luca Fancellu
2025-06-12 7:48 ` Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 4/6] arm/mpu: Move the functions to arm64 specific files Ayan Kumar Halder
2025-06-13 15:08 ` Luca Fancellu
2025-06-16 17:04 ` Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 5/6] arm/mpu: Define arm32 system registers Ayan Kumar Halder
2025-06-16 10:38 ` Hari Limaye
2025-06-16 17:06 ` Ayan Kumar Halder
2025-06-11 14:35 ` [PATCH v3 6/6] arm/mpu: Enable read/write to protection regions for arm32 Ayan Kumar Halder
2025-06-12 9:35 ` Luca Fancellu
2025-06-12 10:37 ` Ayan Kumar Halder
2025-06-13 7:00 ` Orzel, Michal
2025-06-13 9:30 ` Ayan Kumar Halder
2025-06-13 16:02 ` Luca Fancellu
2025-06-16 5:57 ` Luca Fancellu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.