Hi,
On 2023/6/30 01:22, Ayan Kumar Halder wrote:
On 26/06/2023 04:34, Penny Zheng wrote:
CAUTION: This message has originated from an External Source. Please
use proper judgment and caution when opening attachments, clicking
links, or responding to this email.
In MPU system, MPU memory region is always mapped PAGE_ALIGN, so in
order to
not access unexpected memory area, dtb section in xen.lds.S should be
made
page-aligned too.
We add . = ALIGN(PAGE_SIZE); in the head of dtb section to make it
happen.
In this commit, we map early FDT with a transient MPU memory region, as
it will be relocated into heap and unmapped at the end of boot.
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
Signed-off-by: Wei Chen <wei.c...@arm.com>
---
v3:
- map the first 2MB. Check the size and then re-map with an extra 2MB
if needed
---
xen/arch/arm/include/asm/arm64/mpu.h | 3 ++-
xen/arch/arm/include/asm/page.h | 5 +++++
xen/arch/arm/mm.c | 26 ++++++++++++++++++++------
xen/arch/arm/mpu/mm.c | 1 +
xen/arch/arm/xen.lds.S | 5 ++++-
5 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h
b/xen/arch/arm/include/asm/arm64/mpu.h
index a6b07bab02..715ea69884 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -72,7 +72,8 @@ typedef union {
unsigned long ns:1; /* Not-Secure */
unsigned long res:1; /* Reserved 0 by hardware */
unsigned long limit:42; /* Limit Address */
- unsigned long pad:16;
+ unsigned long pad:15;
+ unsigned long tran:1; /* Transient region */
} reg;
uint64_t bits;
} prlar_t;
diff --git a/xen/arch/arm/include/asm/page.h
b/xen/arch/arm/include/asm/page.h
index 85ecd5e4de..a434e2205a 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -97,19 +97,24 @@
* [3:4] Execute Never
* [5:6] Access Permission
* [7] Region Present
+ * [8] Transient Region, e.g. MPU memory region is temproraily
+ * mapped for a short time
*/
#define _PAGE_AI_BIT 0
#define _PAGE_XN_BIT 3
#define _PAGE_AP_BIT 5
#define _PAGE_PRESENT_BIT 7
+#define _PAGE_TRANSIENT_BIT 8
I don't think this is related to MPU. At least when I look at the bit
representation of PRBAR_EL1/2,
This set of _PAGE_xxx flags aren't compliant with PRBAR_EL1/2 register map.
It is a flag passed to function map_pages_to_xen() to indicate memory
attributes and permission.
This bit _PAGE_TRANSIENT is to tell whether the new adding MPU region is
a fixed one, or a temporary one.
The MPU region created for early FDT is a temporary one, as it will be
unmapped at the end of boot.
bits [47:6] are for the base address.
If my understanding is correct, then please take it out of this patch
and put it in a separate MMU related patch.
#define _PAGE_AI (7U << _PAGE_AI_BIT)
#define _PAGE_XN (2U << _PAGE_XN_BIT)
#define _PAGE_RO (2U << _PAGE_AP_BIT)
#define _PAGE_PRESENT (1U << _PAGE_PRESENT_BIT)
+#define _PAGE_TRANSIENT (1U << _PAGE_TRANSIENT_BIT)
#define PAGE_AI_MASK(x) (((x) >> _PAGE_AI_BIT) & 0x7U)
#define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x3U)
#define PAGE_AP_MASK(x) (((x) >> _PAGE_AP_BIT) & 0x3U)
#define PAGE_RO_MASK(x) (((x) >> _PAGE_AP_BIT) & 0x2U)
+#define PAGE_TRANSIENT_MASK(x) (((x) >> _PAGE_TRANSIENT_BIT) & 0x1U)
#endif /* CONFIG_HAS_MPU */
/*
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d35e7e280f..8625066256 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -61,8 +61,17 @@ void flush_page_to_ram(unsigned long mfn, bool
sync_icache)
void * __init early_fdt_map(paddr_t fdt_paddr)
{
+#ifndef CONFIG_HAS_MPU
/* We are using 2MB superpage for mapping the FDT */
paddr_t base_paddr = fdt_paddr & SECOND_MASK;
+ unsigned int flags = PAGE_HYPERVISOR_RO | _PAGE_BLOCK;
+ unsigned long base_virt = BOOT_FDT_VIRT_START;
+#else
+ /* MPU region must be PAGE aligned */
+ paddr_t base_paddr = fdt_paddr & PAGE_MASK;
+ unsigned int flags = PAGE_HYPERVISOR_RO | _PAGE_TRANSIENT;
+ unsigned long base_virt = ~0UL;
+#endif
paddr_t offset;
void *fdt_virt;
uint32_t size;
@@ -79,18 +88,24 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN )
return NULL;
+#ifndef CONFIG_HAS_MPU
/* The FDT is mapped using 2MB superpage */
BUILD_BUG_ON(BOOT_FDT_VIRT_START % SZ_2M);
+#endif
- rc = map_pages_to_xen(BOOT_FDT_VIRT_START, maddr_to_mfn(base_paddr),
- SZ_2M >> PAGE_SHIFT,
- PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+ rc = map_pages_to_xen(base_virt, maddr_to_mfn(base_paddr),
+ SZ_2M >> PAGE_SHIFT, flags);
if ( rc )
panic("Unable to map the device-tree.\n");
+#ifndef CONFIG_HAS_MPU
offset = fdt_paddr % SECOND_SIZE;
fdt_virt = (void *)BOOT_FDT_VIRT_START + offset;
+#else
+ offset = fdt_paddr % PAGE_SIZE;
+ fdt_virt = (void *)fdt_paddr;
+#endif
if ( fdt_magic(fdt_virt) != FDT_MAGIC )
return NULL;
@@ -101,10 +116,9 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
if ( (offset + size) > SZ_2M )
{
- rc = map_pages_to_xen(BOOT_FDT_VIRT_START + SZ_2M,
+ rc = map_pages_to_xen(base_virt + SZ_2M,
maddr_to_mfn(base_paddr + SZ_2M),
- SZ_2M >> PAGE_SHIFT,
- PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+ SZ_2M >> PAGE_SHIFT, flags);
if ( rc )
panic("Unable to map the device-tree\n");
}
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 14a1309ca1..f4ce19d36a 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -448,6 +448,7 @@ static int xen_mpumap_update_entry(paddr_t base,
paddr_t limit,
/* Set permission */
xen_mpumap[idx].prbar.reg.ap = PAGE_AP_MASK(flags);
xen_mpumap[idx].prbar.reg.xn = PAGE_XN_MASK(flags);
+ xen_mpumap[idx].prlar.reg.tran = PAGE_TRANSIENT_MASK(flags);
ReferARM DDI 0600A.dID120821 , G1.3.23, PRLAR_EL2, Protection Region
Limit Address Register (EL2), I don't seethis bit described anywhere. Do
we really need this bit for MPU ?
Yes, It is introduced for software implementation.
You could see commit [PATCH v3 36/52] xen/mpu: implememt ioremap_xxx in
MPU and commit [PATCH v3 38/52] xen/mpu: map domain page in MPU system
for better understanding.
- Ayan
write_protection_region((const pr_t*)(&xen_mpumap[idx]), idx);
}
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 4f7daa7dca..f2715d7cb7 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -216,7 +216,10 @@ SECTIONS
_end = . ;
/* Section for the device tree blob (if any). */
- .dtb : { *(.dtb) } :text
+ .dtb : {
+ . = ALIGN(PAGE_SIZE);
+ *(.dtb)
+ } :text
DWARF2_DEBUG_SECTIONS
--
2.25.1