-----Original Message-----
From: Julien Grall <julien@xxxxxxx>
Sent: Monday, February 6, 2023 6:11 PM
To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini
<sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
Subject: Re: [PATCH v2 20/40] xen/mpu: plump early_fdt_map in MPU
systems
Hi,
A few more remarks.
On 13/01/2023 05:28, Penny Zheng wrote:
In MPU system, device tree binary can be packed with Xen image through
CONFIG_DTB_FILE, or provided by bootloader through x0.
In MPU system, each section in xen.lds.S is PAGE_SIZE aligned.
So in order to not overlap with the previous BSS section, dtb section
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 at
rear with REGION_HYPERVISOR_BOOT.
Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
xen/arch/arm/include/asm/arm64/mpu.h | 5 +++
xen/arch/arm/mm_mpu.c | 63 +++++++++++++++++++++++++---
xen/arch/arm/xen.lds.S | 5 ++-
3 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h
b/xen/arch/arm/include/asm/arm64/mpu.h
index fcde6ad0db..b85e420a90 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -45,18 +45,22 @@
* [3:4] Execute Never
* [5:6] Access Permission
* [7] Region Present
+ * [8] Boot-only Region
*/
#define _REGION_AI_BIT 0
#define _REGION_XN_BIT 3
#define _REGION_AP_BIT 5
#define _REGION_PRESENT_BIT 7
+#define _REGION_BOOTONLY_BIT 8
#define _REGION_XN (2U << _REGION_XN_BIT)
#define _REGION_RO (2U << _REGION_AP_BIT)
#define _REGION_PRESENT (1U << _REGION_PRESENT_BIT)
+#define _REGION_BOOTONLY (1U << _REGION_BOOTONLY_BIT)
#define REGION_AI_MASK(x) (((x) >> _REGION_AI_BIT) & 0x7U)
#define REGION_XN_MASK(x) (((x) >> _REGION_XN_BIT) & 0x3U)
#define REGION_AP_MASK(x) (((x) >> _REGION_AP_BIT) & 0x3U)
#define REGION_RO_MASK(x) (((x) >> _REGION_AP_BIT) & 0x2U)
+#define REGION_BOOTONLY_MASK(x) (((x) >> _REGION_BOOTONLY_BIT)
& 0x1U)
/*
* _REGION_NORMAL is convenience define. It is not meant to be used
@@ -68,6 +72,7 @@
#define REGION_HYPERVISOR_RO
(_REGION_NORMAL|_REGION_XN|_REGION_RO)
#define REGION_HYPERVISOR REGION_HYPERVISOR_RW
+#define REGION_HYPERVISOR_BOOT
(REGION_HYPERVISOR_RW|_REGION_BOOTONLY)
#define INVALID_REGION (~0UL)
diff --git a/xen/arch/arm/mm_mpu.c b/xen/arch/arm/mm_mpu.c index
08720a7c19..b34dbf4515 100644
--- a/xen/arch/arm/mm_mpu.c
+++ b/xen/arch/arm/mm_mpu.c
@@ -20,11 +20,16 @@
*/
#include <xen/init.h>
+#include <xen/libfdt/libfdt.h>
#include <xen/mm.h>
#include <xen/page-size.h>
+#include <xen/pfn.h>
+#include <xen/sizes.h>
#include <xen/spinlock.h>
#include <asm/arm64/mpu.h>
+#include <asm/early_printk.h>
#include <asm/page.h>
+#include <asm/setup.h>
#ifdef NDEBUG
static inline void
@@ -62,6 +67,8 @@ uint64_t __ro_after_init max_xen_mpumap;
static DEFINE_SPINLOCK(xen_mpumap_lock);
+static paddr_t dtb_paddr;
+
/* Write a MPU protection region */
#define WRITE_PROTECTION_REGION(sel, pr, prbar_el2, prlar_el2) ({ \
uint64_t _sel = sel; \
@@ -403,7 +410,16 @@ static int xen_mpumap_update_entry(paddr_t
base,
paddr_t limit,
/* During boot time, the default index is next_fixed_region_idx. */
if ( system_state <= SYS_STATE_active )
- idx = next_fixed_region_idx;
+ {
+ /*
+ * If it is a boot-only region (i.e. region for early FDT),
+ * it shall be added from the tail for late init re-organizing
+ */
+ if ( REGION_BOOTONLY_MASK(flags) )
+ idx = next_transient_region_idx;
+ else
+ idx = next_fixed_region_idx;
+ }
xen_mpumap[idx] = pr_of_xenaddr(base, limit - 1,
REGION_AI_MASK(flags));
/* Set permission */
@@ -465,14 +481,51 @@ int map_pages_to_xen(unsigned long virt,
mfn_to_maddr(mfn_add(mfn, nr_mfns)), flags);
}
-/* TODO: Implementation on the first usage */ -void
dump_hyp_walk(vaddr_t addr)
+void * __init early_fdt_map(paddr_t fdt_paddr)
{
+ void *fdt_virt;
+ uint32_t size;
+
+ /*
+ * Check whether the physical FDT address is set and meets the
minimum
+ * alignment requirement. Since we are relying on MIN_FDT_ALIGN to
be at
+ * least 8 bytes so that we always access the magic and size fields
+ * of the FDT header after mapping the first chunk, double check if
+ * that is indeed the case.
+ */
+ BUILD_BUG_ON(MIN_FDT_ALIGN < 8);
+ if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN )
+ return NULL;
+
+ dtb_paddr = fdt_paddr;
+ /*
+ * In MPU system, device tree binary can be packed with Xen image
+ * through CONFIG_DTB_FILE, or provided by bootloader through x0.
The behavior you describe is not specific to the MPU system. I also don't
quite understand how describing the method to pass the DT actually matters
here.
+ * Map FDT with a transient MPU memory region of MAX_FDT_SIZE.
+ * After that, we can do some magic check.
+ */
+ if ( map_pages_to_xen(round_pgdown(fdt_paddr),
I haven't looked at the rest of the series. But from here, it seems a bit
strange
to use map_pages_to_xen() because the virt and the phys should be the
same.
