[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/7] arm/mpu: Provide access to the MPU region from the C code
Hi Luca,
On 15/04/2025 00:07, Luca Fancellu wrote:
HI Julien,
On 14 Apr 2025, at 12:41, Julien Grall <julien@xxxxxxx> wrote:
Hi Luca,
On 11/04/2025 23:56, Luca Fancellu wrote:
Implement some utility function in order to access the MPU regions
from the C world.
Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
---
v3 changes:
- Moved PRBAR0_EL2/PRLAR0_EL2 to arm64 specific
- Modified prepare_selector() to be easily made a NOP
for Arm32, which can address up to 32 region without
changing selector and it is also its maximum amount
of MPU regions.
---
---
xen/arch/arm/include/asm/arm64/mpu.h | 7 ++
xen/arch/arm/include/asm/mpu.h | 1 +
xen/arch/arm/include/asm/mpu/mm.h | 24 +++++
xen/arch/arm/mpu/mm.c | 125 +++++++++++++++++++++++++++
4 files changed, 157 insertions(+)
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h
b/xen/arch/arm/include/asm/arm64/mpu.h
index 4d2bd7d7877f..b4e1ecdf741d 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -8,6 +8,13 @@
#ifndef __ASSEMBLY__
+/*
+ * The following are needed for the case generators GENERATE_WRITE_PR_REG_CASE
+ * and GENERATE_READ_PR_REG_CASE with num==0
+ */
+#define PRBAR0_EL2 PRBAR_EL2
+#define PRLAR0_EL2 PRLAR_EL2
Rather than aliasing, shouldn't we just rename PR{B,L}AR_EL2 to PR{B,L}AR0_EL2?
This would the code mixing between the two.
PR{B,L}AR0_ELx does not exists really, the PR{B,L}AR<n>_ELx exists for n=1..15, here I’m only
using this “alias” for the generator,
but PR{B,L}AR_EL2 are the real register.
In this case, can PR{B,L}AR0_EL2 defined in mm.c so they are not used
anywhere else?
+
/* Protection Region Base Address Register */
typedef union {
struct __packed {
diff --git a/xen/arch/arm/include/asm/mpu.h b/xen/arch/arm/include/asm/mpu.h
index e148c705b82c..59ff22c804c1 100644
--- a/xen/arch/arm/include/asm/mpu.h
+++ b/xen/arch/arm/include/asm/mpu.h
@@ -13,6 +13,7 @@
#define MPU_REGION_SHIFT 6
#define MPU_REGION_ALIGN (_AC(1, UL) << MPU_REGION_SHIFT)
#define MPU_REGION_MASK (~(MPU_REGION_ALIGN - 1))
+#define MPU_REGION_RES0 (0xFFFULL << 52)
#define NUM_MPU_REGIONS_SHIFT 8
#define NUM_MPU_REGIONS (_AC(1, UL) << NUM_MPU_REGIONS_SHIFT)
diff --git a/xen/arch/arm/include/asm/mpu/mm.h
b/xen/arch/arm/include/asm/mpu/mm.h
index 86f33d9836b7..5cabe9d111ce 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -8,6 +8,7 @@
#include <xen/page-size.h>
#include <xen/types.h>
#include <asm/mm.h>
+#include <asm/mpu.h>
extern struct page_info *frame_table;
@@ -29,6 +30,29 @@ static inline struct page_info *virt_to_page(const void *v)
return mfn_to_page(mfn);
}
+/* Utility function to be used whenever MPU regions are modified */
+static inline void context_sync_mpu(void)
+{
+ /*
+ * ARM DDI 0600B.a, C1.7.1
+ * Writes to MPU registers are only guaranteed to be visible following a
+ * Context synchronization event and DSB operation.
I know we discussed about this before. I find odd that the specification says "context
synchronization event and DSB operation". At least to me, it implies "isb + dsb" not
the other way around. Has this been clarified in newer version of the specification?
unfortunately no, I’m looking into the latest one (Arm® Architecture Reference
Manual Supplement Armv8, for R-profile AArch64 architecture 0600B.a) but it has the same
wording, however
I spoke internally with Cortex-R architects and they told me to use DSB+ISB
So you didn't speak with the ArmV8-R architects? Asking because we are
writing code for ArmV8-R (so not only Cortex-R).
In any case, I still think this is something that needs to be clarified
in the specification. So people that don't have access to the Arm
internal architects know the correct sequence. Is this something you can
follow-up on?
Cheers,
--
Julien Grall
|
