|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 11/40] xen/mpu: build up start-of-day Xen MPU memory region map
Hi Penny, On 13/01/2023 05:28, Penny Zheng wrote: From: Penny Zheng <penny.zheng@xxxxxxx> The start-of-day Xen MPU memory region layout shall be like as follows: xen_mpumap[0] : Xen text xen_mpumap[1] : Xen read-only data xen_mpumap[2] : Xen read-only after init data xen_mpumap[3] : Xen read-write data xen_mpumap[4] : Xen BSS ...... xen_mpumap[max_xen_mpumap - 2]: Xen init data xen_mpumap[max_xen_mpumap - 1]: Xen init text Can you explain why the init region should be at the end of the MPU? max_xen_mpumap refers to the number of regions supported by the EL2 MPU. The layout shall be compliant with what we describe in xen.lds.S, or the codes need adjustment. As MMU system and MPU system have different functions to create the boot MMU/MPU memory management data, instead of introducing extra #ifdef in main code flow, we introduce a neutral name prepare_early_mappings for both, and also to replace create_page_tables for MMU. Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx> Signed-off-by: Wei Chen <wei.chen@xxxxxxx> --- xen/arch/arm/arm64/Makefile | 2 + xen/arch/arm/arm64/head.S | 17 +- xen/arch/arm/arm64/head_mmu.S | 4 +- xen/arch/arm/arm64/head_mpu.S | 323 +++++++++++++++++++++++ xen/arch/arm/include/asm/arm64/mpu.h | 63 +++++ xen/arch/arm/include/asm/arm64/sysregs.h | 49 ++++ xen/arch/arm/mm_mpu.c | 48 ++++ xen/arch/arm/xen.lds.S | 4 + 8 files changed, 502 insertions(+), 8 deletions(-) create mode 100644 xen/arch/arm/arm64/head_mpu.S create mode 100644 xen/arch/arm/include/asm/arm64/mpu.h create mode 100644 xen/arch/arm/mm_mpu.c diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile index 22da2f54b5..438c9737ad 100644 --- a/xen/arch/arm/arm64/Makefile +++ b/xen/arch/arm/arm64/Makefile @@ -10,6 +10,8 @@ obj-y += entry.o obj-y += head.o ifneq ($(CONFIG_HAS_MPU),y) obj-y += head_mmu.o +else +obj-y += head_mpu.o endif obj-y += insn.o obj-$(CONFIG_LIVEPATCH) += livepatch.o diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 782bd1f94c..145e3d53dc 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -68,9 +68,9 @@ * x24 - * x25 - * x26 - skip_zero_bss (boot cpu only) - * x27 - - * x28 - - * x29 - + * x27 - region selector (mpu only) + * x28 - prbar (mpu only) + * x29 - prlar (mpu only) > * x30 - lr */@@ -82,7 +82,7 @@* --------------------------- * * The requirements are: - * MMU = off, D-cache = off, I-cache = on or off, + * MMU/MPU = off, D-cache = off, I-cache = on or off, * x0 = physical address to the FDT blob. * * This must be the very first address in the loaded image. @@ -252,7 +252,12 @@ real_start_efi:bl check_cpu_mode Any particular reason to not use 'stp'?Also, AFAICT, with data cache disabled. But at least on ARMv8-A, the cache is never really off. So don't need some cache maintainance? FAOD, I know the existing MMU code has the same issue. But I would rather prefer if the new code introduced is compliant to the Arm Arm. +.endm + +/* + * Macro to store the maximum number of regions supported by the EL2 MPU + * in max_xen_mpumap, which is identified by MPUIR_EL2. + * + * Outputs: + * nr_regions: preserve the maximum number of regions supported by the EL2 MPU + * + * Clobbers \tmp1 + * > + */ Are you going to have multiple users? If not, then I would prefer if this is folded in the only caller. +.macro read_max_el2_regions, nr_regions, tmp1 + load_paddr \tmp1, max_xen_mpumap I would rather prefer if we restrict the use of global while the MMU if off (see why above). + mrs \nr_regions, MPUIR_EL2 + isb What's that isb for? + str \nr_regions, [\tmp1] +.endm + +/* + * Macro to prepare and set a MPU memory region + * + * Inputs: + * base: base address symbol (should be page-aligned) + * limit: limit address symbol + * sel: region selector + * prbar: store computed PRBAR_EL2 value + * prlar: store computed PRLAR_EL2 value + * attr_prbar: PRBAR_EL2-related memory attributes. If not specified it will be REGION_DATA_PRBAR + * attr_prlar: PRLAR_EL2-related memory attributes. If not specified it will be REGION_NORMAL_PRLAR + * + * Clobber \tmp1 This macro will also clobber x27, x28, x29. + * + */ +.macro prepare_xen_region, base, limit, sel, prbar, prlar, tmp1, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ + load_paddr \prbar, \base + and \prbar, \prbar, #MPU_REGION_MASK + mov \tmp1, #\attr_prbar + orr \prbar, \prbar, \tmp1 + + /* Prepare value for PRLAR_EL2 reg and preserve it in \prlar.*/ + load_paddr \prlar, \limit + /* Round up limit address to be PAGE_SIZE aligned */ + roundup_section \prlar + /* Limit address should be inclusive */ + sub \prlar, \prlar, #1 + and \prlar, \prlar, #MPU_REGION_MASK + mov \tmp1, #\attr_prlar + orr \prlar, \prlar, \tmp1 + + mov x27, \sel + mov x28, \prbar + mov x29, \prlar + /* + * x27, x28, x29 are special registers designed as + * inputs for function write_pr + */ + bl write_pr +.endm + +.section .text.idmap, "ax", %progbits + +/* + * ENTRY to configure a EL2 MPU memory region + * ARMv8-R AArch64 at most supports 255 MPU protection regions. + * See section G1.3.18 of the reference manual for ARMv8-R AArch64, + * PRBAR<n>_EL2 and PRLAR<n>_EL2 provides access to the EL2 MPU region + * determined by the value of 'n' and PRSELR_EL2.REGION as + * PRSELR_EL2.REGION<7:4>:n.(n = 0, 1, 2, ... , 15) + * For example to access regions from 16 to 31 (0b10000 to 0b11111): + * - Set PRSELR_EL2 to 0b1xxxx + * - Region 16 configuration is accessible through PRBAR0_EL2 and PRLAR0_EL2 + * - Region 17 configuration is accessible through PRBAR1_EL2 and PRLAR1_EL2 + * - Region 18 configuration is accessible through PRBAR2_EL2 and PRLAR2_EL2 + * - ... + * - Region 31 configuration is accessible through PRBAR15_EL2 and PRLAR15_EL2 + * + * Inputs: + * x27: region selector + * x28: preserve value for PRBAR_EL2 + * x29: preserve value for PRLAR_EL2 + * + */ +ENTRY(write_pr) AFAICT, this function would not be necessary if the index for the init sections were hardcoded. So I would like to understand why the index cannot be hardcoded. + msr PRSELR_EL2, x27 + dsb sy What is this 'dsb' for? Also why 'sy'? + and x27, x27, #REGION_SEL_MASK + cmp x27, #0 + bne 1f + msr PRBAR0_EL2, x28 + msr PRLAR0_EL2, x29 + b out +1: + cmp x27, #1 + bne 2f + msr PRBAR1_EL2, x28 + msr PRLAR1_EL2, x29 + b out +2: + cmp x27, #2 + bne 3f + msr PRBAR2_EL2, x28 + msr PRLAR2_EL2, x29 + b out +3: + cmp x27, #3 + bne 4f + msr PRBAR3_EL2, x28 + msr PRLAR3_EL2, x29 + b out +4: + cmp x27, #4 + bne 5f + msr PRBAR4_EL2, x28 + msr PRLAR4_EL2, x29 + b out +5: + cmp x27, #5 + bne 6f + msr PRBAR5_EL2, x28 + msr PRLAR5_EL2, x29 + b out +6: + cmp x27, #6 + bne 7f + msr PRBAR6_EL2, x28 + msr PRLAR6_EL2, x29 + b out +7: + cmp x27, #7 + bne 8f + msr PRBAR7_EL2, x28 + msr PRLAR7_EL2, x29 + b out +8: + cmp x27, #8 + bne 9f + msr PRBAR8_EL2, x28 + msr PRLAR8_EL2, x29 + b out +9: + cmp x27, #9 + bne 10f + msr PRBAR9_EL2, x28 + msr PRLAR9_EL2, x29 + b out +10: + cmp x27, #10 + bne 11f + msr PRBAR10_EL2, x28 + msr PRLAR10_EL2, x29 + b out +11: + cmp x27, #11 + bne 12f + msr PRBAR11_EL2, x28 + msr PRLAR11_EL2, x29 + b out +12: + cmp x27, #12 + bne 13f + msr PRBAR12_EL2, x28 + msr PRLAR12_EL2, x29 + b out +13: + cmp x27, #13 + bne 14f + msr PRBAR13_EL2, x28 + msr PRLAR13_EL2, x29 + b out +14: + cmp x27, #14 + bne 15f + msr PRBAR14_EL2, x28 + msr PRLAR14_EL2, x29 + b out +15: + msr PRBAR15_EL2, x28 + msr PRLAR15_EL2, x29 +out: + isb What is this 'isb' for? + ret +ENDPROC(write_pr) + +/* + * Static start-of-day Xen EL2 MPU memory region layout. + * + * xen_mpumap[0] : Xen text + * xen_mpumap[1] : Xen read-only data + * xen_mpumap[2] : Xen read-only after init data + * xen_mpumap[3] : Xen read-write data + * xen_mpumap[4] : Xen BSS + * ...... + * xen_mpumap[max_xen_mpumap - 2]: Xen init data + * xen_mpumap[max_xen_mpumap - 1]: Xen init text + * + * Clobbers x0 - x6 + * + * It shall be compliant with what describes in xen.lds.S, or the below + * codes need adjustment. + * It shall also follow the rules of putting fixed MPU memory region in + * the front, and the others in the rear, which, here, mainly refers to + * boot-only region, like Xen init text region. + */ +ENTRY(prepare_early_mappings) + /* stack LR as write_pr will be called later like nested function */ + mov x6, lr + + /* x0: region sel */ + mov x0, xzr + /* Xen text section. */ + prepare_xen_region _stext, _etext, x0, x1, x2, x3, attr_prbar=REGION_TEXT_PRBAR + create_mpu_entry xen_mpumap, x0, x1, x2, x3, x4 You always seem to call prepare_xen_region and create_mpu_entry. Can they be combined? Also, will the first parameter of create_mpu_entry always be xen_mpumap? If so, I will remove it from the parameter. [...] Why do you need this ALIGN? I know you are not using alternative instructions yet. But, you should make sure they are included. So I think rather than introduce __init_data_begin, you want to use "_einitext" for the start of the "Init data" section. Spurious? Cheers, -- Julien Grall
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |