Enable the helper functions defined in mpu/mm.c and asm/mpu.h for ARM32.
Define the register definitions for HPRBAR{0..31} and HPRLAR{0..31}.
One can directly access the first 32 MPU regions using the above registers
without the use of PRSELR.
Also fix the register definition for HPRLAR.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Change from :-
v1 - 1. Enable write_protection_region() for aarch32.
xen/arch/arm/include/asm/mpu.h | 2 -
xen/arch/arm/include/asm/mpu/cpregs.h | 72 ++++++++++++++++++++++++++-
xen/arch/arm/mpu/mm.c | 49 ++++++++++++++++--
3 files changed, 116 insertions(+), 7 deletions(-)
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/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h
index d5cd0e04d5..9f3b32acd7 100644
--- a/xen/arch/arm/include/asm/mpu/cpregs.h
+++ b/xen/arch/arm/include/asm/mpu/cpregs.h
@@ -6,16 +6,86 @@
/* CP15 CR0: MPU Type Register */
#define HMPUIR p15,4,c0,c0,4
+/* CP15 CR6: Protection Region Enable Register */
+#define HPRENR p15,4,c6,c1,1
+
/* 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
/* Alphabetically... */
#define MPUIR_EL2 HMPUIR
#define PRBAR_EL2 HPRBAR
+#define PRENR_EL2 HPRENR
#define PRLAR_EL2 HPRLAR
#define PRSELR_EL2 HPRSELR
#endif /* CONFIG_ARM_32 */
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 2fb6b822c6..74e96ca571 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -40,7 +40,10 @@ pr_t __cacheline_aligned __section(".data") xen_mpumap[MAX_MPU_REGION_NR];
#define PRBAR_EL2_(n) PRBAR##n##_EL2
#define PRLAR_EL2_(n) PRLAR##n##_EL2
-#endif /* CONFIG_ARM_64 */
+#else /* CONFIG_ARM_64 */
+#define PRBAR_EL2_(n) HPRBAR##n
+#define PRLAR_EL2_(n) HPRLAR##n
+#endif /* !CONFIG_ARM_64 */
#define GENERATE_WRITE_PR_REG_CASE(num, pr) \
case num: \
@@ -68,7 +71,6 @@ static void __init __maybe_unused build_assertions(void)
BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
}
-#ifdef CONFIG_ARM_64
/*
* Armv8-R supports direct access and indirect access to the MPU regions through
* registers:
@@ -85,6 +87,7 @@ static void __init __maybe_unused build_assertions(void)
*/
static void prepare_selector(uint8_t *sel)
{
+#ifdef CONFIG_ARM_64
uint8_t cur_sel = *sel;
/*
@@ -98,7 +101,8 @@ static void prepare_selector(uint8_t *sel)
WRITE_SYSREG(cur_sel, PRSELR_EL2);
isb();
}
- *sel &= 0xFU;
+ *sel = *sel & 0xFU;
+#endif
}
void read_protection_region(pr_t *pr_read, uint8_t sel)
@@ -123,6 +127,24 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
GENERATE_READ_PR_REG_CASE(13, pr_read);
GENERATE_READ_PR_REG_CASE(14, pr_read);
GENERATE_READ_PR_REG_CASE(15, pr_read);
+#ifdef CONFIG_ARM_32
+ 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);
+#endif
default:
BUG(); /* Can't happen */
break;
@@ -151,6 +173,24 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
GENERATE_WRITE_PR_REG_CASE(13, pr_write);
GENERATE_WRITE_PR_REG_CASE(14, pr_write);
GENERATE_WRITE_PR_REG_CASE(15, pr_write);
+#ifdef CONFIG_ARM_32
+ 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);
+#endif
default:
BUG(); /* Can't happen */
break;
@@ -208,7 +248,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 */
}};
@@ -225,7 +267,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.1Hi Ayan,
If I understand correctly Armv8-R AArch32 supports up to 255 regions, so I would expect ...
> /*
> * Armv8-R supports direct access and indirect access to the MPU regions through
> * registers:
> @@ -85,6 +87,7 @@ static void __init __maybe_unused build_assertions(void)
> */
> static void prepare_selector(uint8_t *sel)
> {
> +#ifdef CONFIG_ARM_64
> uint8_t cur_sel = *sel;
>
> /*
> @@ -98,7 +101,8 @@ static void prepare_selector(uint8_t *sel)
> WRITE_SYSREG(cur_sel, PRSELR_EL2);
> isb();
> }
> - *sel &= 0xFU;
> + *sel = *sel & 0xFU;
> +#endif
something here to check if the selector is 0-31 or not and:
- set the selector to 0 if set is 0-31
- set the selector to 32-255 if sel > 32
And ...
> }
>
> void read_protection_region(pr_t *pr_read, uint8_t sel)
> @@ -123,6 +127,24 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
> GENERATE_READ_PR_REG_CASE(13, pr_read);
> GENERATE_READ_PR_REG_CASE(14, pr_read);
> GENERATE_READ_PR_REG_CASE(15, pr_read);
> +#ifdef CONFIG_ARM_32
> + 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);
> +#endif
> default:
have something here for Arm32 to access the regions 32-255
> BUG(); /* Can't happen */
> break;
> @@ -151,6 +173,24 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
> GENERATE_WRITE_PR_REG_CASE(13, pr_write);
> GENERATE_WRITE_PR_REG_CASE(14, pr_write);
> GENERATE_WRITE_PR_REG_CASE(15, pr_write);
> +#ifdef CONFIG_ARM_32
> + 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);
> +#endif
> default:
also here have something for Arm32 to access the regions 32-255
> BUG(); /* Can't happen */
> break;
Please let me know your thoughts.
Cheers,
Luca
On 09/06/2025 09:37, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
>
> If I understand correctly Armv8-R AArch32 supports up to 255 regions, so I would expect ...
>
>> /*
>> * Armv8-R supports direct access and indirect access to the MPU regions through
>> * registers:
>> @@ -85,6 +87,7 @@ static void __init __maybe_unused build_assertions(void)
>> */
>> static void prepare_selector(uint8_t *sel)
>> {
>> +#ifdef CONFIG_ARM_64
>> uint8_t cur_sel = *sel;
>>
>> /*
>> @@ -98,7 +101,8 @@ static void prepare_selector(uint8_t *sel)
>> WRITE_SYSREG(cur_sel, PRSELR_EL2);
>> isb();
>> }
>> - *sel &= 0xFU;
>> + *sel = *sel & 0xFU;
>> +#endif
> something here to check if the selector is 0-31 or not and:
>
> - set the selector to 0 if set is 0-31
> - set the selector to 32-255 if sel > 32
yes, good catch. I was initially thinking of supporting only the first
32 regions for arm32. So, it would BUG() for region numbered 32 onwards.
I can expand the patch to support all the 255 regions.
>
> And ...
>
>
>> }
>>
>> void read_protection_region(pr_t *pr_read, uint8_t sel)
>> @@ -123,6 +127,24 @@ void read_protection_region(pr_t *pr_read, uint8_t sel)
>> GENERATE_READ_PR_REG_CASE(13, pr_read);
>> GENERATE_READ_PR_REG_CASE(14, pr_read);
>> GENERATE_READ_PR_REG_CASE(15, pr_read);
>> +#ifdef CONFIG_ARM_32
>> + 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);
>> +#endif
>> default:
> have something here for Arm32 to access the regions 32-255
>
>
>> BUG(); /* Can't happen */
>> break;
>> @@ -151,6 +173,24 @@ void write_protection_region(const pr_t *pr_write, uint8_t sel)
>> GENERATE_WRITE_PR_REG_CASE(13, pr_write);
>> GENERATE_WRITE_PR_REG_CASE(14, pr_write);
>> GENERATE_WRITE_PR_REG_CASE(15, pr_write);
>> +#ifdef CONFIG_ARM_32
>> + 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);
>> +#endif
>> default:
> also here have something for Arm32 to access the regions 32-255
>
>> BUG(); /* Can't happen */
>> break;
>
> Please let me know your thoughts.
Ack
- Ayan
>
> Cheers,
> Luca
>
>
Oh sorry forgot one thing ...
>
>> /*
>> * Armv8-R supports direct access and indirect access to the MPU regions through
>> * registers:
>> @@ -85,6 +87,7 @@ static void __init __maybe_unused build_assertions(void)
>> */
>> static void prepare_selector(uint8_t *sel)
>> {
>> +#ifdef CONFIG_ARM_64
>> uint8_t cur_sel = *sel;
>>
>> /*
>> @@ -98,7 +101,8 @@ static void prepare_selector(uint8_t *sel)
>> WRITE_SYSREG(cur_sel, PRSELR_EL2);
>> isb();
>> }
>> - *sel &= 0xFU;
>> + *sel = *sel & 0xFU;
this line is not part of this commit, maybe rebase clash?
Cheers,
Luca
On 06/06/2025 18:48, Ayan Kumar Halder wrote:
> Enable the helper functions defined in mpu/mm.c and asm/mpu.h for ARM32.
> Define the register definitions for HPRBAR{0..31} and HPRLAR{0..31}.
> One can directly access the first 32 MPU regions using the above registers
> without the use of PRSELR.
>
> Also fix the register definition for HPRLAR.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
It looks good apart from ...
[snip]
> @@ -98,7 +101,8 @@ static void prepare_selector(uint8_t *sel)
> WRITE_SYSREG(cur_sel, PRSELR_EL2);
> isb();
> }
> - *sel &= 0xFU;
> + *sel = *sel & 0xFU;
this change. Why?
Apart from that:
Acked-by: Michal Orzel <michal.orzel@amd.com>
~Michal
© 2016 - 2025 Red Hat, Inc.