* [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems
@ 2025-03-30 18:03 Ayan Kumar Halder
2025-03-30 18:03 ` [PATCH v3 1/5] xen/arm32: Move MM specific registers to enable_mmu Ayan Kumar Halder
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw)
To: xen-devel
Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk
Enabled early booting of Armv8-R AArch32 based systems.
This patch serie with
https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@arm.com/
will enable build on Xen on Armv8-R AArch32 for MPU.
Patches 1-4 can be applied independently.
Patch 5 depends on Luca's series.
Changes from :-
V1, v2 - Added "xen/arm32: mpu: Stubs to build MPU for arm32" to build Xen for
Armv8-R AArch32 on top of Luca's series.
Ayan Kumar Halder (4):
xen/arm32: Move MM specific registers to enable_mmu
xen/arm: Move some of the functions to common file
xen/arm32: Create the same boot-time MPU regions as arm64
xen/arm32: mpu: Stubs to build MPU for arm32
Michal Orzel (1):
xen/arm32: Allow ARM_PA_BITS_40 only if !MPU
xen/arch/arm/Kconfig | 4 +-
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/head.S | 14 --
xen/arch/arm/arm32/mmu/head.S | 15 ++
xen/arch/arm/arm32/mpu/Makefile | 3 +
xen/arch/arm/arm32/mpu/head.S | 52 +++++++
xen/arch/arm/arm32/mpu/p2m.c | 18 +++
xen/arch/arm/arm32/mpu/smpboot.c | 23 +++
xen/arch/arm/arm64/mpu/head.S | 132 +-----------------
xen/arch/arm/include/asm/arm32/sysregs.h | 11 ++
xen/arch/arm/include/asm/arm64/sysregs.h | 15 ++
xen/arch/arm/include/asm/cpregs.h | 4 +
xen/arch/arm/include/asm/mm.h | 5 +
xen/arch/arm/include/asm/mpu/cpregs.h | 23 +++
.../include/asm/mpu/prepare_xen_region.inc | 128 +++++++++++++++++
15 files changed, 305 insertions(+), 143 deletions(-)
create mode 100644 xen/arch/arm/arm32/mpu/Makefile
create mode 100644 xen/arch/arm/arm32/mpu/head.S
create mode 100644 xen/arch/arm/arm32/mpu/p2m.c
create mode 100644 xen/arch/arm/arm32/mpu/smpboot.c
create mode 100644 xen/arch/arm/include/asm/mpu/cpregs.h
create mode 100644 xen/arch/arm/include/asm/mpu/prepare_xen_region.inc
--
2.25.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v3 1/5] xen/arm32: Move MM specific registers to enable_mmu 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder @ 2025-03-30 18:03 ` Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 2/5] xen/arm: Move some of the functions to common file Ayan Kumar Halder ` (4 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw) To: xen-devel Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk All the memory management specific registers are initialized in enable_mmu. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> --- Changes from - v1 - HTCR and HMAIR{0,1} are not set together with the other memory management registers in enable_mmu() Similar changes are to be done in arm64 as well. I prefer to do that in a separate patch so that all the arm32 changes are kept together in this series. v2 - Added Michal's R-b. xen/arch/arm/arm32/head.S | 14 -------------- xen/arch/arm/arm32/mmu/head.S | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 4ff5c220bc..50da179f81 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -218,20 +218,6 @@ cpu_init: add pc, r1, r10 /* Call paddr(init func) */ cpu_init_done: - /* Set up memory attribute type tables */ - mov_w r0, MAIR0VAL - mov_w r1, MAIR1VAL - mcr CP32(r0, HMAIR0) - mcr CP32(r1, HMAIR1) - - /* - * Set up the HTCR: - * PT walks use Inner-Shareable accesses, - * PT walks are write-back, write-allocate in both cache levels, - * Full 32-bit address space goes through this table. - */ - mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0)) - mcr CP32(r0, HTCR) mov_w r0, HSCTLR_SET mcr CP32(r0, HSCTLR) diff --git a/xen/arch/arm/arm32/mmu/head.S b/xen/arch/arm/arm32/mmu/head.S index 1e2bbf0c82..8fa74bd556 100644 --- a/xen/arch/arm/arm32/mmu/head.S +++ b/xen/arch/arm/arm32/mmu/head.S @@ -279,6 +279,21 @@ ENDPROC(create_page_tables) enable_mmu: PRINT("- Turning on paging -\r\n") + /* Set up memory attribute type tables */ + mov_w r0, MAIR0VAL + mov_w r1, MAIR1VAL + mcr CP32(r0, HMAIR0) + mcr CP32(r1, HMAIR1) + + /* + * Set up the HTCR: + * PT walks use Inner-Shareable accesses, + * PT walks are write-back, write-allocate in both cache levels, + * Full 32-bit address space goes through this table. + */ + mov_w r0, (TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0)) + mcr CP32(r0, HTCR) + /* * The state of the TLBs is unknown before turning on the MMU. * Flush them to avoid stale one. -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/5] xen/arm: Move some of the functions to common file 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 1/5] xen/arm32: Move MM specific registers to enable_mmu Ayan Kumar Halder @ 2025-03-30 18:03 ` Ayan Kumar Halder 2025-03-30 21:06 ` Julien Grall 2025-03-30 18:03 ` [PATCH v3 3/5] xen/arm32: Create the same boot-time MPU regions as arm64 Ayan Kumar Halder ` (3 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw) To: xen-devel Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk Added a new file prepare_xen_region.inc to hold the common earlyboot MPU regions configurations across arm64 and arm32. prepare_xen_region, enable_boot_cpu, fail_insufficient_regions() will be used by both arm32 and arm64. Thus, they have been moved to prepare_xen_region.inc. REGION_* are moved to arm64/sysregs.h. Introduced LOAD_SYSREG and STORE_SYSREG to read/write to the system registers from the common asm file. One could not reuse READ_SYSREG and WRITE_SYSREG as they have been defined to be invoked from C files. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from v1 - 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to what is being done in enable_mmu(). All the mm related configurations happen in this function. 2. Fixed some typos. v2 - 1. Extracted the arm64 head.S functions/macros in a common file. xen/arch/arm/arm64/mpu/head.S | 132 +----------------- xen/arch/arm/include/asm/arm64/sysregs.h | 15 ++ .../include/asm/mpu/prepare_xen_region.inc | 128 +++++++++++++++++ 3 files changed, 148 insertions(+), 127 deletions(-) create mode 100644 xen/arch/arm/include/asm/mpu/prepare_xen_region.inc diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S index 4d00de4869..90b4c8c18f 100644 --- a/xen/arch/arm/arm64/mpu/head.S +++ b/xen/arch/arm/arm64/mpu/head.S @@ -3,83 +3,7 @@ * Start-of-day code for an Armv8-R MPU system. */ -#include <asm/early_printk.h> -#include <asm/mpu.h> - -/* Backgroud region enable/disable */ -#define SCTLR_ELx_BR BIT(17, UL) - -#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ -#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ -#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ -#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ - -#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ -#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ - -/* - * Macro to prepare and set a EL2 MPU memory region. - * We will also create an according MPU memory region entry, which - * is a structure of pr_t, in table \prmap. - * - * sel: region selector - * base: reg storing base address - * limit: reg storing limit address - * prbar: store computed PRBAR_EL2 value - * prlar: store computed PRLAR_EL2 value - * maxcount: maximum number of EL2 regions supported - * 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 - * - * Preserves \maxcount - * Output: - * \sel: Next available region selector index. - * Clobbers \base, \limit, \prbar, \prlar - * - * Note that all parameters using registers should be distinct. - */ -.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR - /* Check if the region is empty */ - cmp \base, \limit - beq 1f - - /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */ - cmp \sel, \maxcount - bge fail_insufficient_regions - - /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ - and \base, \base, #MPU_REGION_MASK - mov \prbar, #\attr_prbar - orr \prbar, \prbar, \base - - /* Limit address should be inclusive */ - sub \limit, \limit, #1 - and \limit, \limit, #MPU_REGION_MASK - mov \prlar, #\attr_prlar - orr \prlar, \prlar, \limit - - msr PRSELR_EL2, \sel - isb - msr PRBAR_EL2, \prbar - msr PRLAR_EL2, \prlar - dsb sy - isb - - add \sel, \sel, #1 - -1: -.endm - -/* - * Failure caused due to insufficient MPU regions. - */ -FUNC_LOCAL(fail_insufficient_regions) - PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n") -1: wfe - b 1b -END(fail_insufficient_regions) +#include <asm/mpu/prepare_xen_region.inc> /* * Enable EL2 MPU and data cache @@ -108,62 +32,16 @@ END(enable_mpu) * Maps the various sections of Xen (described in xen.lds.S) as different MPU * regions. * - * Clobbers x0 - x5 + * Clobbers x0 - x6 * */ FUNC(enable_boot_cpu_mm) - /* Get the number of regions specified in MPUIR_EL2 */ - mrs x5, MPUIR_EL2 - and x5, x5, #NUM_MPU_REGIONS_MASK - - /* x0: region sel */ - mov x0, xzr - /* Xen text section. */ - ldr x1, =_stext - ldr x2, =_etext - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_TEXT_PRBAR - - /* Xen read-only data section. */ - ldr x1, =_srodata - ldr x2, =_erodata - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_RO_PRBAR - - /* Xen read-only after init and data section. (RW data) */ - ldr x1, =__ro_after_init_start - ldr x2, =__init_begin - prepare_xen_region x0, x1, x2, x3, x4, x5 - - /* Xen code section. */ - ldr x1, =__init_begin - ldr x2, =__init_data_begin - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_TEXT_PRBAR - - /* Xen data and BSS section. */ - ldr x1, =__init_data_begin - ldr x2, =__bss_end - prepare_xen_region x0, x1, x2, x3, x4, x5 - -#ifdef CONFIG_EARLY_PRINTK - /* Xen early UART section. */ - ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS - ldr x2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE) - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR -#endif - - b enable_mpu + mov x6, lr + enable_boot_cpu x0, x1, x2, x3, x4, x5 + mov lr, x6 ret END(enable_boot_cpu_mm) -/* - * We don't yet support secondary CPUs bring-up. Implement a dummy helper to - * please the common code. - */ -ENTRY(enable_secondary_cpu_mm) - PRINT("- SMP not enabled yet -\r\n") -1: wfe - b 1b -ENDPROC(enable_secondary_cpu_mm) - /* * Local variables: * mode: ASM diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index b593e4028b..9b833fe73b 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -462,6 +462,19 @@ #define ZCR_ELx_LEN_SIZE 9 #define ZCR_ELx_LEN_MASK 0x1ff +#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ +#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ +#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ +#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ + +#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ + +#define STORE_SYSREG(v, name) "msr " __stringify(name,) #v; +#define LOAD_SYSREG(v, name) "mrs " #v __stringify(,) #name; + +#ifndef __ASSEMBLY__ + /* Access to system registers */ #define WRITE_SYSREG64(v, name) do { \ @@ -481,6 +494,8 @@ #define WRITE_SYSREG_LR(v, index) WRITE_SYSREG(v, ICH_LR_REG(index)) #define READ_SYSREG_LR(index) READ_SYSREG(ICH_LR_REG(index)) +#endif /* __ASSEMBLY__ */ + #endif /* _ASM_ARM_ARM64_SYSREGS_H */ /* diff --git a/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc new file mode 100644 index 0000000000..3402ed23da --- /dev/null +++ b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc @@ -0,0 +1,128 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <asm/sysregs.h> +#include <asm/mpu.h> + +/* Backgroud region enable/disable */ +#define SCTLR_ELx_BR BIT(17, UL) + +/* + * Macro to prepare and set a EL2 MPU memory region. + * We will also create an according MPU memory region entry, which + * is a structure of pr_t, in table \prmap. + * + * sel: region selector + * base: reg storing base address + * limit: reg storing limit address + * prbar: store computed PRBAR_EL2 value + * prlar: store computed PRLAR_EL2 value + * maxcount: maximum number of EL2 regions supported + * 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 + * + * Preserves maxcount + * Output: + * sel: Next available region selector index. + * Clobbers base, limit, prbar, prlar + * + * Note that all parameters using registers should be distinct. + */ +.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR + /* Check if the region is empty */ + cmp \base, \limit + beq 1f + + /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */ + cmp \sel, \maxcount + bge fail_insufficient_regions + + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ + and \base, \base, #MPU_REGION_MASK + mov \prbar, #\attr_prbar + orr \prbar, \prbar, \base + + /* Limit address should be inclusive */ + sub \limit, \limit, #1 + and \limit, \limit, #MPU_REGION_MASK + mov \prlar, #\attr_prlar + orr \prlar, \prlar, \limit + + STORE_SYSREG(\sel, PRSELR_EL2) + isb + STORE_SYSREG(\prbar, PRBAR_EL2) + STORE_SYSREG(\prlar, PRLAR_EL2) + dsb sy + isb + + add \sel, \sel, #1 + +1: +.endm + +.macro enable_boot_cpu, reg0, reg1, reg2, reg3, reg4, reg5 + /* Get the number of regions specified in MPUIR_EL2 */ + LOAD_SYSREG(\reg5, MPUIR_EL2) + and \reg5, \reg5, #NUM_MPU_REGIONS_MASK + + /* reg0: region sel */ + mov \reg0, #0 + /* Xen text section. */ + ldr \reg1, =_stext + ldr \reg2, =_etext + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_TEXT_PRBAR + + /* Xen read-only data section. */ + ldr \reg1, =_srodata + ldr \reg2, =_erodata + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_RO_PRBAR + + /* Xen read-only after init and data section. (RW data) */ + ldr \reg1, =__ro_after_init_start + ldr \reg2, =__init_begin + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 + + /* Xen code section. */ + ldr \reg1, =__init_begin + ldr \reg2, =__init_data_begin + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_TEXT_PRBAR + + /* Xen data and BSS section. */ + ldr \reg1, =__init_data_begin + ldr \reg2, =__bss_end + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 + +#ifdef CONFIG_EARLY_PRINTK + /* Xen early UART section. */ + ldr \reg1, =CONFIG_EARLY_UART_BASE_ADDRESS + ldr \reg2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE) + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR +#endif + + bl enable_mpu +.endm + +/* Failure caused due to insufficient MPU regions. */ +FUNC_LOCAL(fail_insufficient_regions) + PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n") +1: wfe + b 1b +END(fail_insufficient_regions) + +/* + * We don't yet support secondary CPUs bring-up. Implement a dummy helper to + * please the common code. + */ +ENTRY(enable_secondary_cpu_mm) + PRINT("- SMP not enabled yet -\r\n") +1: wfe + b 1b +ENDPROC(enable_secondary_cpu_mm) + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] xen/arm: Move some of the functions to common file 2025-03-30 18:03 ` [PATCH v3 2/5] xen/arm: Move some of the functions to common file Ayan Kumar Halder @ 2025-03-30 21:06 ` Julien Grall 2025-03-31 8:28 ` Orzel, Michal 2025-03-31 12:38 ` Ayan Kumar Halder 0 siblings, 2 replies; 14+ messages in thread From: Julien Grall @ 2025-03-30 21:06 UTC (permalink / raw) To: Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk Hi Ayan, On 30/03/2025 19:03, Ayan Kumar Halder wrote: > Added a new file prepare_xen_region.inc to hold the common earlyboot MPU regions > configurations across arm64 and arm32. While I understand the desire to consolidate the code, I am quite unconvinced this should be done for assembly code. A few examples below why. I would be interested to hear the view of the other Arm maintainers. > > prepare_xen_region, enable_boot_cpu, fail_insufficient_regions() will be used by > both arm32 and arm64. Thus, they have been moved to prepare_xen_region.inc. > > REGION_* are moved to arm64/sysregs.h. Introduced LOAD_SYSREG and STORE_SYSREG > to read/write to the system registers from the common asm file. One could not > reuse READ_SYSREG and WRITE_SYSREG as they have been defined to be invoked from > C files. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > Changes from > > v1 - > > 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to what is > being done in enable_mmu(). All the mm related configurations happen in this > function. > > 2. Fixed some typos. > > v2 - > 1. Extracted the arm64 head.S functions/macros in a common file. > > xen/arch/arm/arm64/mpu/head.S | 132 +----------------- > xen/arch/arm/include/asm/arm64/sysregs.h | 15 ++ > .../include/asm/mpu/prepare_xen_region.inc | 128 +++++++++++++++++ > 3 files changed, 148 insertions(+), 127 deletions(-) > create mode 100644 xen/arch/arm/include/asm/mpu/prepare_xen_region.inc > > diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S > index 4d00de4869..90b4c8c18f 100644 > --- a/xen/arch/arm/arm64/mpu/head.S > +++ b/xen/arch/arm/arm64/mpu/head.S > @@ -3,83 +3,7 @@ > * Start-of-day code for an Armv8-R MPU system. > */ > > -#include <asm/early_printk.h> > -#include <asm/mpu.h> > - > -/* Backgroud region enable/disable */ > -#define SCTLR_ELx_BR BIT(17, UL) > - > -#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ > -#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ > -#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ > -#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ > - > -#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ > -#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ > - > -/* > - * Macro to prepare and set a EL2 MPU memory region. > - * We will also create an according MPU memory region entry, which > - * is a structure of pr_t, in table \prmap. > - * > - * sel: region selector > - * base: reg storing base address > - * limit: reg storing limit address > - * prbar: store computed PRBAR_EL2 value > - * prlar: store computed PRLAR_EL2 value > - * maxcount: maximum number of EL2 regions supported > - * 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 > - * > - * Preserves \maxcount > - * Output: > - * \sel: Next available region selector index. > - * Clobbers \base, \limit, \prbar, \prlar > - * > - * Note that all parameters using registers should be distinct. > - */ > -.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR > - /* Check if the region is empty */ > - cmp \base, \limit > - beq 1f > - > - /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */ > - cmp \sel, \maxcount > - bge fail_insufficient_regions > - > - /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ > - and \base, \base, #MPU_REGION_MASK > - mov \prbar, #\attr_prbar > - orr \prbar, \prbar, \base > - > - /* Limit address should be inclusive */ > - sub \limit, \limit, #1 > - and \limit, \limit, #MPU_REGION_MASK > - mov \prlar, #\attr_prlar > - orr \prlar, \prlar, \limit > - > - msr PRSELR_EL2, \sel > - isb > - msr PRBAR_EL2, \prbar > - msr PRLAR_EL2, \prlar > - dsb sy > - isb > - > - add \sel, \sel, #1 > - > -1: > -.endm > - > -/* > - * Failure caused due to insufficient MPU regions. > - */ > -FUNC_LOCAL(fail_insufficient_regions) > - PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n") > -1: wfe > - b 1b > -END(fail_insufficient_regions) > +#include <asm/mpu/prepare_xen_region.inc> > > /* > * Enable EL2 MPU and data cache > @@ -108,62 +32,16 @@ END(enable_mpu) > * Maps the various sections of Xen (described in xen.lds.S) as different MPU > * regions. > * > - * Clobbers x0 - x5 > + * Clobbers x0 - x6 > * > */ > FUNC(enable_boot_cpu_mm) > - /* Get the number of regions specified in MPUIR_EL2 */ > - mrs x5, MPUIR_EL2 > - and x5, x5, #NUM_MPU_REGIONS_MASK > - > - /* x0: region sel */ > - mov x0, xzr > - /* Xen text section. */ > - ldr x1, =_stext > - ldr x2, =_etext > - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_TEXT_PRBAR > - > - /* Xen read-only data section. */ > - ldr x1, =_srodata > - ldr x2, =_erodata > - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_RO_PRBAR > - > - /* Xen read-only after init and data section. (RW data) */ > - ldr x1, =__ro_after_init_start > - ldr x2, =__init_begin > - prepare_xen_region x0, x1, x2, x3, x4, x5 > - > - /* Xen code section. */ > - ldr x1, =__init_begin > - ldr x2, =__init_data_begin > - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_TEXT_PRBAR > - > - /* Xen data and BSS section. */ > - ldr x1, =__init_data_begin > - ldr x2, =__bss_end > - prepare_xen_region x0, x1, x2, x3, x4, x5 > - > -#ifdef CONFIG_EARLY_PRINTK > - /* Xen early UART section. */ > - ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS > - ldr x2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE) > - prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR > -#endif > - > - b enable_mpu > + mov x6, lr Aside what I wrote above, why do we need to save/restore lr? > + enable_boot_cpu x0, x1, x2, x3, x4, x5 > + mov lr, x6 > ret > END(enable_boot_cpu_mm) > > -/* > - * We don't yet support secondary CPUs bring-up. Implement a dummy helper to > - * please the common code. > - */ > -ENTRY(enable_secondary_cpu_mm) > - PRINT("- SMP not enabled yet -\r\n") > -1: wfe > - b 1b > -ENDPROC(enable_secondary_cpu_mm) > - > /* > * Local variables: > * mode: ASM > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h > index b593e4028b..9b833fe73b 100644 > --- a/xen/arch/arm/include/asm/arm64/sysregs.h > +++ b/xen/arch/arm/include/asm/arm64/sysregs.h > @@ -462,6 +462,19 @@ > #define ZCR_ELx_LEN_SIZE 9 > #define ZCR_ELx_LEN_MASK 0x1ff > > +#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ > +#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ > +#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ > +#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ While those makes sense in sysreg.h because they are definition based on the Arm Arm. The definition for ... > + > +#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ > +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ for PRLAR are specific to Xen. So I think they should be moved under mpu/. I would also consider *_PRBAR ones. Also, are those values truely arm64 specific? Asking because you are using them in common code. > + > +#define STORE_SYSREG(v, name) "msr " __stringify(name,) #v; > +#define LOAD_SYSREG(v, name) "mrs " #v __stringify(,) #name; > + > +#ifndef __ASSEMBLY__ > + > /* Access to system registers */ > > #define WRITE_SYSREG64(v, name) do { \ > @@ -481,6 +494,8 @@ > #define WRITE_SYSREG_LR(v, index) WRITE_SYSREG(v, ICH_LR_REG(index)) > #define READ_SYSREG_LR(index) READ_SYSREG(ICH_LR_REG(index)) > > +#endif /* __ASSEMBLY__ */ > + > #endif /* _ASM_ARM_ARM64_SYSREGS_H */ > > /* > diff --git a/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc > new file mode 100644 > index 0000000000..3402ed23da > --- /dev/null > +++ b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#include <asm/sysregs.h> > +#include <asm/mpu.h> > + > +/* Backgroud region enable/disable */ > +#define SCTLR_ELx_BR BIT(17, UL) > + > +/* > + * Macro to prepare and set a EL2 MPU memory region. > + * We will also create an according MPU memory region entry, which > + * is a structure of pr_t, in table \prmap. > + * > + * sel: region selector > + * base: reg storing base address > + * limit: reg storing limit address > + * prbar: store computed PRBAR_EL2 value > + * prlar: store computed PRLAR_EL2 value > + * maxcount: maximum number of EL2 regions supported > + * 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 > + * > + * Preserves maxcount > + * Output: > + * sel: Next available region selector index. > + * Clobbers base, limit, prbar, prlar > + * > + * Note that all parameters using registers should be distinct. > + */ > +.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR > + /* Check if the region is empty */ > + cmp \base, \limit > + beq 1f > + > + /* Check if the number of regions exceeded the count specified in MPUIR_EL2 */ > + cmp \sel, \maxcount > + bge fail_insufficient_regions > + > + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ > + and \base, \base, #MPU_REGION_MASK > + mov \prbar, #\attr_prbar > + orr \prbar, \prbar, \base > + > + /* Limit address should be inclusive */ > + sub \limit, \limit, #1 > + and \limit, \limit, #MPU_REGION_MASK > + mov \prlar, #\attr_prlar > + orr \prlar, \prlar, \limit > + > + STORE_SYSREG(\sel, PRSELR_EL2) > + isb > + STORE_SYSREG(\prbar, PRBAR_EL2) > + STORE_SYSREG(\prlar, PRLAR_EL2) > + dsb sy > + isb > + > + add \sel, \sel, #1 > + > +1: > +.endm > + > +.macro enable_boot_cpu, reg0, reg1, reg2, reg3, reg4, reg5 If we go this approach, this will need some documentation on top (similar to the other macro in this file). > + /* Get the number of regions specified in MPUIR_EL2 */ > + LOAD_SYSREG(\reg5, MPUIR_EL2) > + and \reg5, \reg5, #NUM_MPU_REGIONS_MASK > + > + /* reg0: region sel */ > + mov \reg0, #0 > + /* Xen text section. */ > + ldr \reg1, =_stext For instance, on Arm32, this could be replaced with ``mov_w`` which is doesn't involve memory load. > + ldr \reg2, =_etext > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_TEXT_PRBAR > + > + /* Xen read-only data section. */ > + ldr \reg1, =_srodata > + ldr \reg2, =_erodata > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_RO_PRBAR > + > + /* Xen read-only after init and data section. (RW data) */ > + ldr \reg1, =__ro_after_init_start > + ldr \reg2, =__init_begin > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 > + > + /* Xen code section. */ > + ldr \reg1, =__init_begin > + ldr \reg2, =__init_data_begin > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_TEXT_PRBAR > + > + /* Xen data and BSS section. */ > + ldr \reg1, =__init_data_begin > + ldr \reg2, =__bss_end > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 > + > +#ifdef CONFIG_EARLY_PRINTK > + /* Xen early UART section. */ > + ldr \reg1, =CONFIG_EARLY_UART_BASE_ADDRESS > + ldr \reg2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE) > + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR > +#endif > + > + bl enable_mpu > +.endm > + > +/* Failure caused due to insufficient MPU regions. */ > +FUNC_LOCAL(fail_insufficient_regions) > + PRINT("- Selected MPU region is above the implemented number in MPUIR_EL2 -\r\n") > +1: wfe > + b 1b > +END(fail_insufficient_regions) > + > +/* > + * We don't yet support secondary CPUs bring-up. Implement a dummy helper to > + * please the common code. > + */ > +ENTRY(enable_secondary_cpu_mm) I really doubt we will be able to keep this function common in the future. > + PRINT("- SMP not enabled yet -\r\n") > +1: wfe > + b 1b > +ENDPROC(enable_secondary_cpu_mm) > + > +/* > + * Local variables: > + * mode: ASM > + * indent-tabs-mode: nil > + * End: > + */ Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] xen/arm: Move some of the functions to common file 2025-03-30 21:06 ` Julien Grall @ 2025-03-31 8:28 ` Orzel, Michal 2025-03-31 12:38 ` Ayan Kumar Halder 1 sibling, 0 replies; 14+ messages in thread From: Orzel, Michal @ 2025-03-31 8:28 UTC (permalink / raw) To: Julien Grall, Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk On 30/03/2025 23:06, Julien Grall wrote: > > > Hi Ayan, > > On 30/03/2025 19:03, Ayan Kumar Halder wrote: >> Added a new file prepare_xen_region.inc to hold the common earlyboot MPU regions >> configurations across arm64 and arm32. > > While I understand the desire to consolidate the code, I am quite > unconvinced this should be done for assembly code. A few examples below why. > > I would be interested to hear the view of the other Arm maintainers. I think consolidating the code is ok as long as we don't go to extremes. Consolidation should be done for things only where it makes really sense (macros, helpers). For example, in places where we don't predict that given implementation might be different due to certain arch optimizations, etc. (like you mentioned for mov_w). If we worry that it'll be difficult to asses what to make common, I might suggest to consolidate only common macros. In case of this patch, I think we could have a single implementation for prepare_xen_region and fail_insufficient_regions, whereas enable_boot_cpu should be implementation specific. I also think that a common .inc file is a good place for storing stubs such as enable_secondary_cpu_mm(). One day, if let's say arm64 decide to add support for SMP, it will add it's own implementation, leaving the stub only for arm32. > >> >> prepare_xen_region, enable_boot_cpu, fail_insufficient_regions() will be used by >> both arm32 and arm64. Thus, they have been moved to prepare_xen_region.inc. >> >> REGION_* are moved to arm64/sysregs.h. Introduced LOAD_SYSREG and STORE_SYSREG That's not a good name as it makes me think it's ldr/str. >> to read/write to the system registers from the common asm file. One could not >> reuse READ_SYSREG and WRITE_SYSREG as they have been defined to be invoked from >> C files. Therefore {READ/WRTIE}_SYSREG_ASM() could be used to denote ASM only usage. ~Michal ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] xen/arm: Move some of the functions to common file 2025-03-30 21:06 ` Julien Grall 2025-03-31 8:28 ` Orzel, Michal @ 2025-03-31 12:38 ` Ayan Kumar Halder 2025-03-31 13:04 ` Julien Grall 1 sibling, 1 reply; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-31 12:38 UTC (permalink / raw) To: Julien Grall, Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk On 30/03/2025 22:06, Julien Grall wrote: > Hi Ayan, Hi Julien/Michal, Michal - I agree to use {READ/WRTIE}_SYSREG_ASM() instead of LOAD/STORE. I need your thoughts on the following. > > On 30/03/2025 19:03, Ayan Kumar Halder wrote: >> Added a new file prepare_xen_region.inc to hold the common earlyboot >> MPU regions >> configurations across arm64 and arm32. > > While I understand the desire to consolidate the code, I am quite > unconvinced this should be done for assembly code. A few examples > below why. > > I would be interested to hear the view of the other Arm maintainers. > >> >> prepare_xen_region, enable_boot_cpu, fail_insufficient_regions() will >> be used by >> both arm32 and arm64. Thus, they have been moved to >> prepare_xen_region.inc. >> >> REGION_* are moved to arm64/sysregs.h. Introduced LOAD_SYSREG and >> STORE_SYSREG >> to read/write to the system registers from the common asm file. One >> could not >> reuse READ_SYSREG and WRITE_SYSREG as they have been defined to be >> invoked from >> C files. >> >> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> >> --- >> Changes from >> >> v1 - >> >> 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to >> what is >> being done in enable_mmu(). All the mm related configurations happen >> in this >> function. >> >> 2. Fixed some typos. >> >> v2 - >> 1. Extracted the arm64 head.S functions/macros in a common file. >> >> xen/arch/arm/arm64/mpu/head.S | 132 +----------------- >> xen/arch/arm/include/asm/arm64/sysregs.h | 15 ++ >> .../include/asm/mpu/prepare_xen_region.inc | 128 +++++++++++++++++ >> 3 files changed, 148 insertions(+), 127 deletions(-) >> create mode 100644 xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >> >> diff --git a/xen/arch/arm/arm64/mpu/head.S >> b/xen/arch/arm/arm64/mpu/head.S >> index 4d00de4869..90b4c8c18f 100644 >> --- a/xen/arch/arm/arm64/mpu/head.S >> +++ b/xen/arch/arm/arm64/mpu/head.S >> @@ -3,83 +3,7 @@ >> * Start-of-day code for an Armv8-R MPU system. >> */ >> -#include <asm/early_printk.h> >> -#include <asm/mpu.h> >> - >> -/* Backgroud region enable/disable */ >> -#define SCTLR_ELx_BR BIT(17, UL) >> - >> -#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ >> -#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ >> -#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ >> -#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ >> - >> -#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ >> -#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ >> - >> -/* >> - * Macro to prepare and set a EL2 MPU memory region. >> - * We will also create an according MPU memory region entry, which >> - * is a structure of pr_t, in table \prmap. >> - * >> - * sel: region selector >> - * base: reg storing base address >> - * limit: reg storing limit address >> - * prbar: store computed PRBAR_EL2 value >> - * prlar: store computed PRLAR_EL2 value >> - * maxcount: maximum number of EL2 regions supported >> - * 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 >> - * >> - * Preserves \maxcount >> - * Output: >> - * \sel: Next available region selector index. >> - * Clobbers \base, \limit, \prbar, \prlar >> - * >> - * Note that all parameters using registers should be distinct. >> - */ >> -.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, >> attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR >> - /* Check if the region is empty */ >> - cmp \base, \limit >> - beq 1f >> - >> - /* Check if the number of regions exceeded the count specified >> in MPUIR_EL2 */ >> - cmp \sel, \maxcount >> - bge fail_insufficient_regions >> - >> - /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ >> - and \base, \base, #MPU_REGION_MASK >> - mov \prbar, #\attr_prbar >> - orr \prbar, \prbar, \base >> - >> - /* Limit address should be inclusive */ >> - sub \limit, \limit, #1 >> - and \limit, \limit, #MPU_REGION_MASK >> - mov \prlar, #\attr_prlar >> - orr \prlar, \prlar, \limit >> - >> - msr PRSELR_EL2, \sel >> - isb >> - msr PRBAR_EL2, \prbar >> - msr PRLAR_EL2, \prlar >> - dsb sy >> - isb >> - >> - add \sel, \sel, #1 >> - >> -1: >> -.endm >> - >> -/* >> - * Failure caused due to insufficient MPU regions. >> - */ >> -FUNC_LOCAL(fail_insufficient_regions) >> - PRINT("- Selected MPU region is above the implemented number in >> MPUIR_EL2 -\r\n") >> -1: wfe >> - b 1b >> -END(fail_insufficient_regions) >> +#include <asm/mpu/prepare_xen_region.inc> >> /* >> * Enable EL2 MPU and data cache >> @@ -108,62 +32,16 @@ END(enable_mpu) >> * Maps the various sections of Xen (described in xen.lds.S) as >> different MPU >> * regions. >> * >> - * Clobbers x0 - x5 >> + * Clobbers x0 - x6 >> * >> */ >> FUNC(enable_boot_cpu_mm) >> - /* Get the number of regions specified in MPUIR_EL2 */ >> - mrs x5, MPUIR_EL2 >> - and x5, x5, #NUM_MPU_REGIONS_MASK >> - >> - /* x0: region sel */ >> - mov x0, xzr >> - /* Xen text section. */ >> - ldr x1, =_stext >> - ldr x2, =_etext >> - prepare_xen_region x0, x1, x2, x3, x4, x5, >> attr_prbar=REGION_TEXT_PRBAR >> - >> - /* Xen read-only data section. */ >> - ldr x1, =_srodata >> - ldr x2, =_erodata >> - prepare_xen_region x0, x1, x2, x3, x4, x5, >> attr_prbar=REGION_RO_PRBAR >> - >> - /* Xen read-only after init and data section. (RW data) */ >> - ldr x1, =__ro_after_init_start >> - ldr x2, =__init_begin >> - prepare_xen_region x0, x1, x2, x3, x4, x5 >> - >> - /* Xen code section. */ >> - ldr x1, =__init_begin >> - ldr x2, =__init_data_begin >> - prepare_xen_region x0, x1, x2, x3, x4, x5, >> attr_prbar=REGION_TEXT_PRBAR >> - >> - /* Xen data and BSS section. */ >> - ldr x1, =__init_data_begin >> - ldr x2, =__bss_end >> - prepare_xen_region x0, x1, x2, x3, x4, x5 >> - >> -#ifdef CONFIG_EARLY_PRINTK >> - /* Xen early UART section. */ >> - ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS >> - ldr x2, =(CONFIG_EARLY_UART_BASE_ADDRESS + >> CONFIG_EARLY_UART_SIZE) >> - prepare_xen_region x0, x1, x2, x3, x4, x5, >> attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR >> -#endif >> - >> - b enable_mpu >> + mov x6, lr > > Aside what I wrote above, why do we need to save/restore lr? In the case of arm32, there is no 'ret' instruction. When enable_boot_cpu_mm() ----(invokes)--> enable_mpu(), LR should be saved and restored. Otherwise, one cannot return to the caller of enable_boot_cpu_mm() > >> + enable_boot_cpu x0, x1, x2, x3, x4, x5 >> + mov lr, x6 >> ret >> END(enable_boot_cpu_mm) >> -/* >> - * We don't yet support secondary CPUs bring-up. Implement a dummy >> helper to >> - * please the common code. >> - */ >> -ENTRY(enable_secondary_cpu_mm) >> - PRINT("- SMP not enabled yet -\r\n") >> -1: wfe >> - b 1b >> -ENDPROC(enable_secondary_cpu_mm) >> - >> /* >> * Local variables: >> * mode: ASM >> diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h >> b/xen/arch/arm/include/asm/arm64/sysregs.h >> index b593e4028b..9b833fe73b 100644 >> --- a/xen/arch/arm/include/asm/arm64/sysregs.h >> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h >> @@ -462,6 +462,19 @@ >> #define ZCR_ELx_LEN_SIZE 9 >> #define ZCR_ELx_LEN_MASK 0x1ff >> +#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ >> +#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ >> +#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ >> +#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ > > While those makes sense in sysreg.h because they are definition based > on the Arm Arm. The definition for ... > >> + >> +#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ >> +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ > > for PRLAR are specific to Xen. So I think they should be moved under > mpu/. I would also consider *_PRBAR ones. > > Also, are those values truely arm64 specific? Asking because you are > using them in common code. These definitions (like the _PRBAR) are based on Arm Arm as well. The only difference is that these two values are common for arm32 and arm64. I preferred to duplicate these values in both the header files so that they stay together with _PRBAR. One needs to use both _PRBAR and _PRLAR to set a MPU region. Thus, I don't prefer to split them in separate header files. > >> + >> +#define STORE_SYSREG(v, name) "msr " __stringify(name,) #v; >> +#define LOAD_SYSREG(v, name) "mrs " #v __stringify(,) #name; >> + >> +#ifndef __ASSEMBLY__ >> + >> /* Access to system registers */ >> #define WRITE_SYSREG64(v, name) do { \ >> @@ -481,6 +494,8 @@ >> #define WRITE_SYSREG_LR(v, index) WRITE_SYSREG(v, ICH_LR_REG(index)) >> #define READ_SYSREG_LR(index) READ_SYSREG(ICH_LR_REG(index)) >> +#endif /* __ASSEMBLY__ */ >> + >> #endif /* _ASM_ARM_ARM64_SYSREGS_H */ >> /* >> diff --git a/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >> b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >> new file mode 100644 >> index 0000000000..3402ed23da >> --- /dev/null >> +++ b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >> @@ -0,0 +1,128 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> + >> +#include <asm/sysregs.h> >> +#include <asm/mpu.h> >> + >> +/* Backgroud region enable/disable */ >> +#define SCTLR_ELx_BR BIT(17, UL) >> + >> +/* >> + * Macro to prepare and set a EL2 MPU memory region. >> + * We will also create an according MPU memory region entry, which >> + * is a structure of pr_t, in table \prmap. >> + * >> + * sel: region selector >> + * base: reg storing base address >> + * limit: reg storing limit address >> + * prbar: store computed PRBAR_EL2 value >> + * prlar: store computed PRLAR_EL2 value >> + * maxcount: maximum number of EL2 regions supported >> + * 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 >> + * >> + * Preserves maxcount >> + * Output: >> + * sel: Next available region selector index. >> + * Clobbers base, limit, prbar, prlar >> + * >> + * Note that all parameters using registers should be distinct. >> + */ >> +.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, >> attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR >> + /* Check if the region is empty */ >> + cmp \base, \limit >> + beq 1f >> + >> + /* Check if the number of regions exceeded the count specified >> in MPUIR_EL2 */ >> + cmp \sel, \maxcount >> + bge fail_insufficient_regions >> + >> + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ >> + and \base, \base, #MPU_REGION_MASK >> + mov \prbar, #\attr_prbar >> + orr \prbar, \prbar, \base >> + >> + /* Limit address should be inclusive */ >> + sub \limit, \limit, #1 >> + and \limit, \limit, #MPU_REGION_MASK >> + mov \prlar, #\attr_prlar >> + orr \prlar, \prlar, \limit >> + >> + STORE_SYSREG(\sel, PRSELR_EL2) >> + isb >> + STORE_SYSREG(\prbar, PRBAR_EL2) >> + STORE_SYSREG(\prlar, PRLAR_EL2) >> + dsb sy >> + isb >> + >> + add \sel, \sel, #1 >> + >> +1: >> +.endm >> + > > +.macro enable_boot_cpu, reg0, reg1, reg2, reg3, reg4, reg5 > > If we go this approach, this will need some documentation on top > (similar to the other macro in this file). Ack. will do. > >> + /* Get the number of regions specified in MPUIR_EL2 */ >> + LOAD_SYSREG(\reg5, MPUIR_EL2) >> + and \reg5, \reg5, #NUM_MPU_REGIONS_MASK >> + >> + /* reg0: region sel */ >> + mov \reg0, #0 >> + /* Xen text section. */ >> + ldr \reg1, =_stext > > For instance, on Arm32, this could be replaced with ``mov_w`` which is > doesn't involve memory load. Yes, this is a minor difference. Remember that this code is executed only once during boot time. While there is a performance penalty with ldr, should we really use mov_w (and as a consequence have a different implementation for arm32) where 99% code is the same and we have lesser code to mantain. (Julien/Michal :- need your comments here) > >> + ldr \reg2, =_etext >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >> attr_prbar=REGION_TEXT_PRBAR >> + >> + /* Xen read-only data section. */ >> + ldr \reg1, =_srodata >> + ldr \reg2, =_erodata >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >> attr_prbar=REGION_RO_PRBAR >> + >> + /* Xen read-only after init and data section. (RW data) */ >> + ldr \reg1, =__ro_after_init_start >> + ldr \reg2, =__init_begin >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 >> + >> + /* Xen code section. */ >> + ldr \reg1, =__init_begin >> + ldr \reg2, =__init_data_begin >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >> attr_prbar=REGION_TEXT_PRBAR >> + >> + /* Xen data and BSS section. */ >> + ldr \reg1, =__init_data_begin >> + ldr \reg2, =__bss_end >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 >> + >> +#ifdef CONFIG_EARLY_PRINTK >> + /* Xen early UART section. */ >> + ldr \reg1, =CONFIG_EARLY_UART_BASE_ADDRESS >> + ldr \reg2, =(CONFIG_EARLY_UART_BASE_ADDRESS + >> CONFIG_EARLY_UART_SIZE) >> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >> attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR >> +#endif >> + >> + bl enable_mpu >> +.endm >> + >> +/* Failure caused due to insufficient MPU regions. */ >> +FUNC_LOCAL(fail_insufficient_regions) >> + PRINT("- Selected MPU region is above the implemented number in >> MPUIR_EL2 -\r\n") >> +1: wfe >> + b 1b >> +END(fail_insufficient_regions) >> + >> +/* >> + * We don't yet support secondary CPUs bring-up. Implement a dummy >> helper to >> + * please the common code. >> + */ >> +ENTRY(enable_secondary_cpu_mm) > > I really doubt we will be able to keep this function common in the > future. Are you ok if we keep it here for now ? Looking a bit further ahead, the MPU registers are not core specific (unlike MMU). Thus, there is no MPU specific configuration that we should be doing for the secondary cores. We can investigate more on this when we enable SMP support. - Ayan > >> + PRINT("- SMP not enabled yet -\r\n") >> +1: wfe >> + b 1b >> +ENDPROC(enable_secondary_cpu_mm) >> + >> +/* >> + * Local variables: >> + * mode: ASM >> + * indent-tabs-mode: nil >> + * End: >> + */ > > Cheers, > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] xen/arm: Move some of the functions to common file 2025-03-31 12:38 ` Ayan Kumar Halder @ 2025-03-31 13:04 ` Julien Grall 0 siblings, 0 replies; 14+ messages in thread From: Julien Grall @ 2025-03-31 13:04 UTC (permalink / raw) To: Ayan Kumar Halder, Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk Hi Ayan, On 31/03/2025 13:38, Ayan Kumar Halder wrote: > > On 30/03/2025 22:06, Julien Grall wrote: >> Hi Ayan, > > Hi Julien/Michal, > > Michal - I agree to use {READ/WRTIE}_SYSREG_ASM() instead of LOAD/STORE. > > I need your thoughts on the following. > >> >> On 30/03/2025 19:03, Ayan Kumar Halder wrote: >>> Added a new file prepare_xen_region.inc to hold the common earlyboot >>> MPU regions >>> configurations across arm64 and arm32. >> >> While I understand the desire to consolidate the code, I am quite >> unconvinced this should be done for assembly code. A few examples >> below why. >> >> I would be interested to hear the view of the other Arm maintainers. >> >>> >>> prepare_xen_region, enable_boot_cpu, fail_insufficient_regions() will >>> be used by >>> both arm32 and arm64. Thus, they have been moved to >>> prepare_xen_region.inc. >>> >>> REGION_* are moved to arm64/sysregs.h. Introduced LOAD_SYSREG and >>> STORE_SYSREG >>> to read/write to the system registers from the common asm file. One >>> could not >>> reuse READ_SYSREG and WRITE_SYSREG as they have been defined to be >>> invoked from >>> C files. >>> >>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> >>> --- >>> Changes from >>> >>> v1 - >>> >>> 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to >>> what is >>> being done in enable_mmu(). All the mm related configurations happen >>> in this >>> function. >>> >>> 2. Fixed some typos. >>> >>> v2 - >>> 1. Extracted the arm64 head.S functions/macros in a common file. >>> >>> xen/arch/arm/arm64/mpu/head.S | 132 +----------------- >>> xen/arch/arm/include/asm/arm64/sysregs.h | 15 ++ >>> .../include/asm/mpu/prepare_xen_region.inc | 128 +++++++++++++++++ >>> 3 files changed, 148 insertions(+), 127 deletions(-) >>> create mode 100644 xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >>> >>> diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/ >>> head.S >>> index 4d00de4869..90b4c8c18f 100644 >>> --- a/xen/arch/arm/arm64/mpu/head.S >>> +++ b/xen/arch/arm/arm64/mpu/head.S >>> @@ -3,83 +3,7 @@ >>> * Start-of-day code for an Armv8-R MPU system. >>> */ >>> -#include <asm/early_printk.h> >>> -#include <asm/mpu.h> >>> - >>> -/* Backgroud region enable/disable */ >>> -#define SCTLR_ELx_BR BIT(17, UL) >>> - >>> -#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ >>> -#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ >>> -#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ >>> -#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ >>> - >>> -#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ >>> -#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ >>> - >>> -/* >>> - * Macro to prepare and set a EL2 MPU memory region. >>> - * We will also create an according MPU memory region entry, which >>> - * is a structure of pr_t, in table \prmap. >>> - * >>> - * sel: region selector >>> - * base: reg storing base address >>> - * limit: reg storing limit address >>> - * prbar: store computed PRBAR_EL2 value >>> - * prlar: store computed PRLAR_EL2 value >>> - * maxcount: maximum number of EL2 regions supported >>> - * 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 >>> - * >>> - * Preserves \maxcount >>> - * Output: >>> - * \sel: Next available region selector index. >>> - * Clobbers \base, \limit, \prbar, \prlar >>> - * >>> - * Note that all parameters using registers should be distinct. >>> - */ >>> -.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, >>> attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR >>> - /* Check if the region is empty */ >>> - cmp \base, \limit >>> - beq 1f >>> - >>> - /* Check if the number of regions exceeded the count specified >>> in MPUIR_EL2 */ >>> - cmp \sel, \maxcount >>> - bge fail_insufficient_regions >>> - >>> - /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ >>> - and \base, \base, #MPU_REGION_MASK >>> - mov \prbar, #\attr_prbar >>> - orr \prbar, \prbar, \base >>> - >>> - /* Limit address should be inclusive */ >>> - sub \limit, \limit, #1 >>> - and \limit, \limit, #MPU_REGION_MASK >>> - mov \prlar, #\attr_prlar >>> - orr \prlar, \prlar, \limit >>> - >>> - msr PRSELR_EL2, \sel >>> - isb >>> - msr PRBAR_EL2, \prbar >>> - msr PRLAR_EL2, \prlar >>> - dsb sy >>> - isb >>> - >>> - add \sel, \sel, #1 >>> - >>> -1: >>> -.endm >>> - >>> -/* >>> - * Failure caused due to insufficient MPU regions. >>> - */ >>> -FUNC_LOCAL(fail_insufficient_regions) >>> - PRINT("- Selected MPU region is above the implemented number in >>> MPUIR_EL2 -\r\n") >>> -1: wfe >>> - b 1b >>> -END(fail_insufficient_regions) >>> +#include <asm/mpu/prepare_xen_region.inc> >>> /* >>> * Enable EL2 MPU and data cache >>> @@ -108,62 +32,16 @@ END(enable_mpu) >>> * Maps the various sections of Xen (described in xen.lds.S) as >>> different MPU >>> * regions. >>> * >>> - * Clobbers x0 - x5 >>> + * Clobbers x0 - x6 >>> * >>> */ >>> FUNC(enable_boot_cpu_mm) >>> - /* Get the number of regions specified in MPUIR_EL2 */ >>> - mrs x5, MPUIR_EL2 >>> - and x5, x5, #NUM_MPU_REGIONS_MASK >>> - >>> - /* x0: region sel */ >>> - mov x0, xzr >>> - /* Xen text section. */ >>> - ldr x1, =_stext >>> - ldr x2, =_etext >>> - prepare_xen_region x0, x1, x2, x3, x4, x5, >>> attr_prbar=REGION_TEXT_PRBAR >>> - >>> - /* Xen read-only data section. */ >>> - ldr x1, =_srodata >>> - ldr x2, =_erodata >>> - prepare_xen_region x0, x1, x2, x3, x4, x5, >>> attr_prbar=REGION_RO_PRBAR >>> - >>> - /* Xen read-only after init and data section. (RW data) */ >>> - ldr x1, =__ro_after_init_start >>> - ldr x2, =__init_begin >>> - prepare_xen_region x0, x1, x2, x3, x4, x5 >>> - >>> - /* Xen code section. */ >>> - ldr x1, =__init_begin >>> - ldr x2, =__init_data_begin >>> - prepare_xen_region x0, x1, x2, x3, x4, x5, >>> attr_prbar=REGION_TEXT_PRBAR >>> - >>> - /* Xen data and BSS section. */ >>> - ldr x1, =__init_data_begin >>> - ldr x2, =__bss_end >>> - prepare_xen_region x0, x1, x2, x3, x4, x5 >>> - >>> -#ifdef CONFIG_EARLY_PRINTK >>> - /* Xen early UART section. */ >>> - ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS >>> - ldr x2, =(CONFIG_EARLY_UART_BASE_ADDRESS + >>> CONFIG_EARLY_UART_SIZE) >>> - prepare_xen_region x0, x1, x2, x3, x4, x5, >>> attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR >>> -#endif >>> - >>> - b enable_mpu >>> + mov x6, lr >> >> Aside what I wrote above, why do we need to save/restore lr? > > In the case of arm32, there is no 'ret' instruction. There is a macro "ret" in asm/arm42/macros.h that does the same job. > > When enable_boot_cpu_mm() ----(invokes)--> enable_mpu(), LR should be > saved and restored. Otherwise, one cannot return to the caller of > enable_boot_cpu_mm() Oh I didn't notice you were using "bl enable_mpu" rather than the existing "b enable_mpu". But then I am confused why we can't use "b"? [...] >> While those makes sense in sysreg.h because they are definition based >> on the Arm Arm. The definition for ... >> >>> + >>> +#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ >>> +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ >> >> for PRLAR are specific to Xen. So I think they should be moved under >> mpu/. I would also consider *_PRBAR ones. >> >> Also, are those values truely arm64 specific? Asking because you are >> using them in common code. > > These definitions (like the _PRBAR) are based on Arm Arm as well. From my understanding, ATTR is an index in the MAIR and the content is Xen specific. Furthermore, the name of the define are based on the value in the MAIR. So I disagree this is solely based on the Arm Arm. > I preferred to duplicate these values in both the header files so that > they stay together with _PRBAR. One needs to use both _PRBAR and _PRLAR > to set a MPU region. Thus, I don't prefer to split them in separate > header files. > >> >>> + >>> +#define STORE_SYSREG(v, name) "msr " __stringify(name,) #v; >>> +#define LOAD_SYSREG(v, name) "mrs " #v __stringify(,) #name; >>> + >>> +#ifndef __ASSEMBLY__ >>> + >>> /* Access to system registers */ >>> #define WRITE_SYSREG64(v, name) do { \ >>> @@ -481,6 +494,8 @@ >>> #define WRITE_SYSREG_LR(v, index) WRITE_SYSREG(v, ICH_LR_REG(index)) >>> #define READ_SYSREG_LR(index) READ_SYSREG(ICH_LR_REG(index)) >>> +#endif /* __ASSEMBLY__ */ >>> + >>> #endif /* _ASM_ARM_ARM64_SYSREGS_H */ >>> /* >>> diff --git a/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc b/ >>> xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >>> new file mode 100644 >>> index 0000000000..3402ed23da >>> --- /dev/null >>> +++ b/xen/arch/arm/include/asm/mpu/prepare_xen_region.inc >>> @@ -0,0 +1,128 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> + >>> +#include <asm/sysregs.h> >>> +#include <asm/mpu.h> >>> + >>> +/* Backgroud region enable/disable */ >>> +#define SCTLR_ELx_BR BIT(17, UL) >>> + >>> +/* >>> + * Macro to prepare and set a EL2 MPU memory region. >>> + * We will also create an according MPU memory region entry, which >>> + * is a structure of pr_t, in table \prmap. >>> + * >>> + * sel: region selector >>> + * base: reg storing base address >>> + * limit: reg storing limit address >>> + * prbar: store computed PRBAR_EL2 value >>> + * prlar: store computed PRLAR_EL2 value >>> + * maxcount: maximum number of EL2 regions supported >>> + * 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 >>> + * >>> + * Preserves maxcount >>> + * Output: >>> + * sel: Next available region selector index. >>> + * Clobbers base, limit, prbar, prlar >>> + * >>> + * Note that all parameters using registers should be distinct. >>> + */ >>> +.macro prepare_xen_region, sel, base, limit, prbar, prlar, maxcount, >>> attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR >>> + /* Check if the region is empty */ >>> + cmp \base, \limit >>> + beq 1f >>> + >>> + /* Check if the number of regions exceeded the count specified >>> in MPUIR_EL2 */ >>> + cmp \sel, \maxcount >>> + bge fail_insufficient_regions >>> + >>> + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ >>> + and \base, \base, #MPU_REGION_MASK >>> + mov \prbar, #\attr_prbar >>> + orr \prbar, \prbar, \base >>> + >>> + /* Limit address should be inclusive */ >>> + sub \limit, \limit, #1 >>> + and \limit, \limit, #MPU_REGION_MASK >>> + mov \prlar, #\attr_prlar >>> + orr \prlar, \prlar, \limit >>> + >>> + STORE_SYSREG(\sel, PRSELR_EL2) >>> + isb >>> + STORE_SYSREG(\prbar, PRBAR_EL2) >>> + STORE_SYSREG(\prlar, PRLAR_EL2) >>> + dsb sy >>> + isb >>> + >>> + add \sel, \sel, #1 >>> + >>> +1: >>> +.endm >>> + >> > +.macro enable_boot_cpu, reg0, reg1, reg2, reg3, reg4, reg5 >> >> If we go this approach, this will need some documentation on top >> (similar to the other macro in this file). > Ack. will do. >> >>> + /* Get the number of regions specified in MPUIR_EL2 */ >>> + LOAD_SYSREG(\reg5, MPUIR_EL2) >>> + and \reg5, \reg5, #NUM_MPU_REGIONS_MASK >>> + >>> + /* reg0: region sel */ >>> + mov \reg0, #0 >>> + /* Xen text section. */ >>> + ldr \reg1, =_stext >> >> For instance, on Arm32, this could be replaced with ``mov_w`` which is >> doesn't involve memory load. > > Yes, this is a minor difference. Remember that this code is executed > only once during boot time. I know the code is only executed at boot time (assuming we don't plan support suspend/resume...). But I dislike "ldr x, =..." because this is introducing a constant pool at the end of the section. Although, I agree this is a matter of taste. However... > While there is a performance penalty with > ldr, should we really use mov_w (and as a consequence have a different > implementation for arm32) where 99% code is the same and we have lesser > code to mantain. ... I disagree with this point. To me, this is not as simple as "Less code so more maintainenable. My concern is when reading/updating/... this code we have to read the code with both arm32 and arm64 in mind. It is much easier if I only know the assembly code applies for one architecture. So I still disagree with trying to make this code common. [...] >> >>> + ldr \reg2, =_etext >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >>> attr_prbar=REGION_TEXT_PRBAR >>> + >>> + /* Xen read-only data section. */ >>> + ldr \reg1, =_srodata >>> + ldr \reg2, =_erodata >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >>> attr_prbar=REGION_RO_PRBAR >>> + >>> + /* Xen read-only after init and data section. (RW data) */ >>> + ldr \reg1, =__ro_after_init_start >>> + ldr \reg2, =__init_begin >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 >>> + >>> + /* Xen code section. */ >>> + ldr \reg1, =__init_begin >>> + ldr \reg2, =__init_data_begin >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >>> attr_prbar=REGION_TEXT_PRBAR >>> + >>> + /* Xen data and BSS section. */ >>> + ldr \reg1, =__init_data_begin >>> + ldr \reg2, =__bss_end >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5 >>> + >>> +#ifdef CONFIG_EARLY_PRINTK >>> + /* Xen early UART section. */ >>> + ldr \reg1, =CONFIG_EARLY_UART_BASE_ADDRESS >>> + ldr \reg2, =(CONFIG_EARLY_UART_BASE_ADDRESS + >>> CONFIG_EARLY_UART_SIZE) >>> + prepare_xen_region \reg0, \reg1, \reg2, \reg3, \reg4, \reg5, >>> attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR >>> +#endif >>> + >>> + bl enable_mpu >>> +.endm >>> + >>> +/* Failure caused due to insufficient MPU regions. */ >>> +FUNC_LOCAL(fail_insufficient_regions) >>> + PRINT("- Selected MPU region is above the implemented number in >>> MPUIR_EL2 -\r\n") >>> +1: wfe >>> + b 1b >>> +END(fail_insufficient_regions) >>> + >>> +/* >>> + * We don't yet support secondary CPUs bring-up. Implement a dummy >>> helper to >>> + * please the common code. >>> + */ >>> +ENTRY(enable_secondary_cpu_mm) >> >> I really doubt we will be able to keep this function common in the >> future. > > Are you ok if we keep it here for now ? > > > Looking a bit further ahead, the MPU registers are not core specific > (unlike MMU). > Thus, there is no MPU specific configuration that we > should be doing for the secondary cores. We can investigate more on this > when we enable SMP support. Are you sure? Are you saying you don't need to update SCTLR_EL2 nor setting up PRLAR_* & co? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/5] xen/arm32: Create the same boot-time MPU regions as arm64 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 1/5] xen/arm32: Move MM specific registers to enable_mmu Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 2/5] xen/arm: Move some of the functions to common file Ayan Kumar Halder @ 2025-03-30 18:03 ` Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU Ayan Kumar Halder ` (2 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw) To: xen-devel Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk We have created the same boot-time MPU protection regions as Armv8-R AArch64. Also, we have defined REGION_* macros for arm32. The only difference from arm64 is that XN is 1-bit for arm32. The macros have been defined in arm32/sysregs.h. Though REGION_NORMAL_PRLAR and REGION_DEVICE_PRLAR are same between arm32 and arm64, we have duplicated them to keep the definitions at the same place as the other REGION_* macros. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from v1 - 1. enable_mpu() now sets HMAIR{0,1} registers. This is similar to what is being done in enable_mmu(). All the mm related configurations happen in this function. 2. Fixed some typos. v2 - 1. Include the common prepare_xen_region.inc in head.S. 2. Define LOAD_SYSREG()/STORE_SYSREG() for arm32. xen/arch/arm/arm32/Makefile | 1 + xen/arch/arm/arm32/mpu/Makefile | 1 + xen/arch/arm/arm32/mpu/head.S | 52 ++++++++++++++++++++++++ xen/arch/arm/include/asm/arm32/sysregs.h | 11 +++++ xen/arch/arm/include/asm/cpregs.h | 4 ++ xen/arch/arm/include/asm/mpu/cpregs.h | 23 +++++++++++ 6 files changed, 92 insertions(+) create mode 100644 xen/arch/arm/arm32/mpu/Makefile create mode 100644 xen/arch/arm/arm32/mpu/head.S create mode 100644 xen/arch/arm/include/asm/mpu/cpregs.h diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile index 40a2b4803f..537969d753 100644 --- a/xen/arch/arm/arm32/Makefile +++ b/xen/arch/arm/arm32/Makefile @@ -1,5 +1,6 @@ obj-y += lib/ obj-$(CONFIG_MMU) += mmu/ +obj-$(CONFIG_MPU) += mpu/ obj-$(CONFIG_EARLY_PRINTK) += debug.o obj-y += domctl.o diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile new file mode 100644 index 0000000000..3340058c08 --- /dev/null +++ b/xen/arch/arm/arm32/mpu/Makefile @@ -0,0 +1 @@ +obj-y += head.o diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S new file mode 100644 index 0000000000..30c901525a --- /dev/null +++ b/xen/arch/arm/arm32/mpu/head.S @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Start-of-day code for an Armv8-R MPU system. + */ + +#include <asm/page.h> +#include <asm/mpu/prepare_xen_region.inc> + +/* + * Set up the memory attribute type tables and enable EL2 MPU and data cache. + * If the Background region is enabled, then the MPU uses the default memory + * map as the Background region for generating the memory + * attributes when MPU is disabled. + * Since the default memory map of the Armv8-R AArch32 architecture is + * IMPLEMENTATION DEFINED, we intend to turn off the Background region here. + * + * Clobbers r0 - r1 + */ +FUNC_LOCAL(enable_mpu) + /* Set up memory attribute type tables */ + mov_w r0, MAIR0VAL + mov_w r1, MAIR1VAL + mcr CP32(r0, HMAIR0) + mcr CP32(r1, HMAIR1) + + mrc CP32(r0, HSCTLR) + bic r0, r0, #SCTLR_ELx_BR /* Disable Background region */ + orr r0, r0, #SCTLR_Axx_ELx_M /* Enable MPU */ + orr r0, r0, #SCTLR_Axx_ELx_C /* Enable D-cache */ + mcr CP32(r0, HSCTLR) + isb + mov pc, lr +END(enable_mpu) + +/* + * Maps the various sections of Xen (decsribed in xen.lds.S) as different MPU + * regions. + * + * Clobbers r0 - r6 + */ +FUNC(enable_boot_cpu_mm) + mov r6, lr + enable_boot_cpu r0, r1, r2, r3, r4, r5 + mov pc, r6 +END(enable_boot_cpu_mm) + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/include/asm/arm32/sysregs.h b/xen/arch/arm/include/asm/arm32/sysregs.h index 22871999af..e02c0932e6 100644 --- a/xen/arch/arm/include/asm/arm32/sysregs.h +++ b/xen/arch/arm/include/asm/arm32/sysregs.h @@ -4,6 +4,14 @@ #include <xen/stringify.h> #include <asm/cpregs.h> +#define REGION_TEXT_PRBAR 0x18 /* SH=11 AP=10 XN=0 */ +#define REGION_RO_PRBAR 0x1D /* SH=11 AP=10 XN=1 */ +#define REGION_DATA_PRBAR 0x19 /* SH=11 AP=00 XN=1 */ +#define REGION_DEVICE_PRBAR 0x11 /* SH=10 AP=00 XN=1 */ + +#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ + /* Layout as used in assembly, with src/dest registers mixed in */ #define __CP32(r, coproc, opc1, crn, crm, opc2) coproc, opc1, r, crn, crm, opc2 #define __CP64(r1, r2, coproc, opc, crm) coproc, opc, r1, r2, crm @@ -16,6 +24,9 @@ #define LOAD_CP64(r, name...) "mrrc " __stringify(CP64(%r, %H##r, name)) ";" #define STORE_CP64(r, name...) "mcrr " __stringify(CP64(%r, %H##r, name)) ";" +#define LOAD_SYSREG(v, name) mrc CP32(v, name) +#define STORE_SYSREG(v, name) mcr CP32(v, name) + /* Issue a CP operation which takes no argument, * uses r0 as a placeholder register. */ #define CMD_CP32(name...) "mcr " __stringify(CP32(r0, name)) ";" diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h index aec9e8f329..6019a2cbdd 100644 --- a/xen/arch/arm/include/asm/cpregs.h +++ b/xen/arch/arm/include/asm/cpregs.h @@ -1,6 +1,10 @@ #ifndef __ASM_ARM_CPREGS_H #define __ASM_ARM_CPREGS_H +#ifdef CONFIG_MPU +#include <asm/mpu/cpregs.h> +#endif + /* * AArch32 Co-processor registers. * diff --git a/xen/arch/arm/include/asm/mpu/cpregs.h b/xen/arch/arm/include/asm/mpu/cpregs.h new file mode 100644 index 0000000000..cf63730233 --- /dev/null +++ b/xen/arch/arm/include/asm/mpu/cpregs.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __ASM_ARM_MPU_CPREGS_H +#define __ASM_ARM_MPU_CPREGS_H + +#define HMPUIR p15,4,c0,c0,4 + +/* CP15 CR6: MPU Protection Region Base/Limit/Select Address Register */ +#define HPRSELR p15,4,c6,c2,1 +#define PRBAR_EL2 p15,4,c6,c3,0 +#define PRLAR_EL2 p15,4,c6,c8,1 + +#define MPUIR_EL2 HMPUIR +#define PRSELR_EL2 HPRSELR + +#endif /* __ASM_ARM_MPU_CPREGS_H */ + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder ` (2 preceding siblings ...) 2025-03-30 18:03 ` [PATCH v3 3/5] xen/arm32: Create the same boot-time MPU regions as arm64 Ayan Kumar Halder @ 2025-03-30 18:03 ` Ayan Kumar Halder 2025-03-30 21:07 ` Julien Grall 2025-03-30 18:03 ` [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 Ayan Kumar Halder 2025-03-31 10:13 ` [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Orzel, Michal 5 siblings, 1 reply; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw) To: xen-devel Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk, Ayan Kumar Halder From: Michal Orzel <michal.orzel@amd.com> ArmV8-R AArch32 does not support LPAE. The reason being PMSAv8-32 supports 32-bit physical address only. Signed-off-by: Michal Orzel <michal.orzel@amd.com> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from v1 - 1. New patch. v2 - 1. No changes xen/arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index a4af0b85f1..565f288331 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -58,7 +58,7 @@ config ARM_PA_BITS_32 config ARM_PA_BITS_40 bool "40-bit" - depends on ARM_32 + depends on ARM_32 && !MPU endchoice config PADDR_BITS -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU 2025-03-30 18:03 ` [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU Ayan Kumar Halder @ 2025-03-30 21:07 ` Julien Grall 0 siblings, 0 replies; 14+ messages in thread From: Julien Grall @ 2025-03-30 21:07 UTC (permalink / raw) To: Ayan Kumar Halder, xen-devel Cc: Michal Orzel, Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk Hi, On 30/03/2025 19:03, Ayan Kumar Halder wrote: > From: Michal Orzel <michal.orzel@amd.com> > > ArmV8-R AArch32 does not support LPAE. The reason being PMSAv8-32 > supports 32-bit physical address only. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder ` (3 preceding siblings ...) 2025-03-30 18:03 ` [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU Ayan Kumar Halder @ 2025-03-30 18:03 ` Ayan Kumar Halder 2025-03-30 21:11 ` Julien Grall 2025-03-31 10:13 ` [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Orzel, Michal 5 siblings, 1 reply; 14+ messages in thread From: Ayan Kumar Halder @ 2025-03-30 18:03 UTC (permalink / raw) To: xen-devel Cc: Ayan Kumar Halder, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from :- v1, v2 - 1. New patch introduced in v3. 2. Should be applied on top of https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@arm.com/ xen/arch/arm/Kconfig | 2 +- xen/arch/arm/arm32/mpu/Makefile | 2 ++ xen/arch/arm/arm32/mpu/p2m.c | 18 ++++++++++++++++++ xen/arch/arm/arm32/mpu/smpboot.c | 23 +++++++++++++++++++++++ xen/arch/arm/include/asm/mm.h | 5 +++++ 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 xen/arch/arm/arm32/mpu/p2m.c create mode 100644 xen/arch/arm/arm32/mpu/smpboot.c diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 565f288331..a1dd942091 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -1,7 +1,7 @@ config ARM_32 def_bool y depends on "$(ARCH)" = "arm32" - select ARCH_MAP_DOMAIN_PAGE + select ARCH_MAP_DOMAIN_PAGE if MMU config ARM_64 def_bool y diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile index 3340058c08..38797f28af 100644 --- a/xen/arch/arm/arm32/mpu/Makefile +++ b/xen/arch/arm/arm32/mpu/Makefile @@ -1 +1,3 @@ obj-y += head.o +obj-y += smpboot.o +obj-y += p2m.o diff --git a/xen/arch/arm/arm32/mpu/p2m.c b/xen/arch/arm/arm32/mpu/p2m.c new file mode 100644 index 0000000000..df8de5c7d8 --- /dev/null +++ b/xen/arch/arm/arm32/mpu/p2m.c @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <xen/init.h> +#include <asm/p2m.h> + +void __init setup_virt_paging(void) +{ + BUG_ON("unimplemented"); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/arm32/mpu/smpboot.c b/xen/arch/arm/arm32/mpu/smpboot.c new file mode 100644 index 0000000000..3f3e54294e --- /dev/null +++ b/xen/arch/arm/arm32/mpu/smpboot.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <xen/mm.h> + +int prepare_secondary_mm(int cpu) +{ + BUG_ON("unimplemented"); + return -EINVAL; +} + +void update_boot_mapping(bool enable) +{ + BUG_ON("unimplemented"); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h index fbffaccef4..a894e28ac9 100644 --- a/xen/arch/arm/include/asm/mm.h +++ b/xen/arch/arm/include/asm/mm.h @@ -171,12 +171,17 @@ struct page_info #define PGC_need_scrub PGC_allocated #ifdef CONFIG_ARM_32 +#ifdef CONFIG_MPU +#define is_xen_heap_page(page) false +#define is_xen_heap_mfn(mfn) false +#else #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page)) #define is_xen_heap_mfn(mfn) ({ \ unsigned long mfn_ = mfn_x(mfn); \ (mfn_ >= mfn_x(directmap_mfn_start) && \ mfn_ < mfn_x(directmap_mfn_end)); \ }) +#endif #else #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap) #define is_xen_heap_mfn(mfn) \ -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 2025-03-30 18:03 ` [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 Ayan Kumar Halder @ 2025-03-30 21:11 ` Julien Grall 2025-04-03 16:35 ` Ayan Kumar Halder 0 siblings, 1 reply; 14+ messages in thread From: Julien Grall @ 2025-03-30 21:11 UTC (permalink / raw) To: Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk Hi Ayan, On 30/03/2025 19:03, Ayan Kumar Halder wrote: > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > Changes from :- > > v1, v2 - > 1. New patch introduced in v3. > 2. Should be applied on top of > https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@arm.com/ > > xen/arch/arm/Kconfig | 2 +- > xen/arch/arm/arm32/mpu/Makefile | 2 ++ > xen/arch/arm/arm32/mpu/p2m.c | 18 ++++++++++++++++++ > xen/arch/arm/arm32/mpu/smpboot.c | 23 +++++++++++++++++++++++ > xen/arch/arm/include/asm/mm.h | 5 +++++ > 5 files changed, 49 insertions(+), 1 deletion(-) > create mode 100644 xen/arch/arm/arm32/mpu/p2m.c > create mode 100644 xen/arch/arm/arm32/mpu/smpboot.c > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 565f288331..a1dd942091 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -1,7 +1,7 @@ > config ARM_32 > def_bool y > depends on "$(ARCH)" = "arm32" > - select ARCH_MAP_DOMAIN_PAGE > + select ARCH_MAP_DOMAIN_PAGE if MMU If ARCH_MAP_DOMAIN_PAGE is not present, then the assumption is Xen will be able to access all the memory. Is this what we want long term? If not, then I would consider providing stubs rather than disable ARCH_MAP_DOMAIN_PAGE. > > config ARM_64 > def_bool y > diff --git a/xen/arch/arm/arm32/mpu/Makefile b/xen/arch/arm/arm32/mpu/Makefile > index 3340058c08..38797f28af 100644 > --- a/xen/arch/arm/arm32/mpu/Makefile > +++ b/xen/arch/arm/arm32/mpu/Makefile > @@ -1 +1,3 @@ > obj-y += head.o > +obj-y += smpboot.o > +obj-y += p2m.o > diff --git a/xen/arch/arm/arm32/mpu/p2m.c b/xen/arch/arm/arm32/mpu/p2m.c > new file mode 100644 > index 0000000000..df8de5c7d8 > --- /dev/null > +++ b/xen/arch/arm/arm32/mpu/p2m.c > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#include <xen/init.h> > +#include <asm/p2m.h> > + > +void __init setup_virt_paging(void) > +{ > + BUG_ON("unimplemented"); > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/arm32/mpu/smpboot.c b/xen/arch/arm/arm32/mpu/smpboot.c > new file mode 100644 > index 0000000000..3f3e54294e > --- /dev/null > +++ b/xen/arch/arm/arm32/mpu/smpboot.c > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#include <xen/mm.h> > + > +int prepare_secondary_mm(int cpu) > +{ > + BUG_ON("unimplemented"); > + return -EINVAL; > +} > + > +void update_boot_mapping(bool enable) > +{ > + BUG_ON("unimplemented"); > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h > index fbffaccef4..a894e28ac9 100644 > --- a/xen/arch/arm/include/asm/mm.h > +++ b/xen/arch/arm/include/asm/mm.h > @@ -171,12 +171,17 @@ struct page_info > #define PGC_need_scrub PGC_allocated > > #ifdef CONFIG_ARM_32 > +#ifdef CONFIG_MPU > +#define is_xen_heap_page(page) false > +#define is_xen_heap_mfn(mfn) false Can you clarify whether this is the intended implementation? If not then we can be use BUG_ON("unimplemented")? Also... > +#else > #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page)) > #define is_xen_heap_mfn(mfn) ({ \ > unsigned long mfn_ = mfn_x(mfn); \ > (mfn_ >= mfn_x(directmap_mfn_start) && \ > mfn_ < mfn_x(directmap_mfn_end)); \ > }) > +#endif ... is the implementation will be similar to is_xen_heap_page() for MMU? > #else > #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap) > #define is_xen_heap_mfn(mfn) \ Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 2025-03-30 21:11 ` Julien Grall @ 2025-04-03 16:35 ` Ayan Kumar Halder 0 siblings, 0 replies; 14+ messages in thread From: Ayan Kumar Halder @ 2025-04-03 16:35 UTC (permalink / raw) To: Julien Grall, Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk On 30/03/2025 22:11, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 30/03/2025 19:03, Ayan Kumar Halder wrote: >> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> >> --- >> Changes from :- >> >> v1, v2 - >> 1. New patch introduced in v3. >> 2. Should be applied on top of >> https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@arm.com/ >> >> >> xen/arch/arm/Kconfig | 2 +- >> xen/arch/arm/arm32/mpu/Makefile | 2 ++ >> xen/arch/arm/arm32/mpu/p2m.c | 18 ++++++++++++++++++ >> xen/arch/arm/arm32/mpu/smpboot.c | 23 +++++++++++++++++++++++ >> xen/arch/arm/include/asm/mm.h | 5 +++++ >> 5 files changed, 49 insertions(+), 1 deletion(-) >> create mode 100644 xen/arch/arm/arm32/mpu/p2m.c >> create mode 100644 xen/arch/arm/arm32/mpu/smpboot.c >> >> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig >> index 565f288331..a1dd942091 100644 >> --- a/xen/arch/arm/Kconfig >> +++ b/xen/arch/arm/Kconfig >> @@ -1,7 +1,7 @@ >> config ARM_32 >> def_bool y >> depends on "$(ARCH)" = "arm32" >> - select ARCH_MAP_DOMAIN_PAGE >> + select ARCH_MAP_DOMAIN_PAGE if MMU > > If ARCH_MAP_DOMAIN_PAGE is not present, then the assumption is Xen > will be able to access all the memory. Is this what we want long term? > > If not, then I would consider providing stubs rather than disable > ARCH_MAP_DOMAIN_PAGE. I will provide the stubs. > >> config ARM_64 >> def_bool y >> diff --git a/xen/arch/arm/arm32/mpu/Makefile >> b/xen/arch/arm/arm32/mpu/Makefile >> index 3340058c08..38797f28af 100644 >> --- a/xen/arch/arm/arm32/mpu/Makefile >> +++ b/xen/arch/arm/arm32/mpu/Makefile >> @@ -1 +1,3 @@ >> obj-y += head.o >> +obj-y += smpboot.o >> +obj-y += p2m.o >> diff --git a/xen/arch/arm/arm32/mpu/p2m.c b/xen/arch/arm/arm32/mpu/p2m.c >> new file mode 100644 >> index 0000000000..df8de5c7d8 >> --- /dev/null >> +++ b/xen/arch/arm/arm32/mpu/p2m.c >> @@ -0,0 +1,18 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> + >> +#include <xen/init.h> >> +#include <asm/p2m.h> >> + >> +void __init setup_virt_paging(void) >> +{ >> + BUG_ON("unimplemented"); >> +} >> + >> +/* >> + * Local variables: >> + * mode: C >> + * c-file-style: "BSD" >> + * c-basic-offset: 4 >> + * indent-tabs-mode: nil >> + * End: >> + */ >> diff --git a/xen/arch/arm/arm32/mpu/smpboot.c >> b/xen/arch/arm/arm32/mpu/smpboot.c >> new file mode 100644 >> index 0000000000..3f3e54294e >> --- /dev/null >> +++ b/xen/arch/arm/arm32/mpu/smpboot.c >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> + >> +#include <xen/mm.h> >> + >> +int prepare_secondary_mm(int cpu) >> +{ >> + BUG_ON("unimplemented"); >> + return -EINVAL; >> +} >> + >> +void update_boot_mapping(bool enable) >> +{ >> + BUG_ON("unimplemented"); >> +} >> + >> +/* >> + * Local variables: >> + * mode: C >> + * c-file-style: "BSD" >> + * c-basic-offset: 4 >> + * indent-tabs-mode: nil >> + * End: >> + */ >> diff --git a/xen/arch/arm/include/asm/mm.h >> b/xen/arch/arm/include/asm/mm.h >> index fbffaccef4..a894e28ac9 100644 >> --- a/xen/arch/arm/include/asm/mm.h >> +++ b/xen/arch/arm/include/asm/mm.h >> @@ -171,12 +171,17 @@ struct page_info >> #define PGC_need_scrub PGC_allocated >> #ifdef CONFIG_ARM_32 >> +#ifdef CONFIG_MPU >> +#define is_xen_heap_page(page) false >> +#define is_xen_heap_mfn(mfn) false > > Can you clarify whether this is the intended implementation? If not > then we can be use BUG_ON("unimplemented")? yes, we will use BUG_ON("unimplemented"). > > Also... > >> +#else >> #define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page)) >> #define is_xen_heap_mfn(mfn) ({ \ >> unsigned long mfn_ = mfn_x(mfn); \ >> (mfn_ >= mfn_x(directmap_mfn_start) && \ >> mfn_ < mfn_x(directmap_mfn_end)); \ >> }) >> +#endif > > ... is the implementation will be similar to is_xen_heap_page() for MMU? > >> #else >> #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap) >> #define is_xen_heap_mfn(mfn) \ > - Ayan > Cheers, > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder ` (4 preceding siblings ...) 2025-03-30 18:03 ` [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 Ayan Kumar Halder @ 2025-03-31 10:13 ` Orzel, Michal 5 siblings, 0 replies; 14+ messages in thread From: Orzel, Michal @ 2025-03-31 10:13 UTC (permalink / raw) To: Ayan Kumar Halder, xen-devel Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Volodymyr Babchuk On 30/03/2025 20:03, Ayan Kumar Halder wrote: > Enabled early booting of Armv8-R AArch32 based systems. > > This patch serie with > https://patchwork.kernel.org/project/xen-devel/cover/20250316192445.2376484-1-luca.fancellu@arm.com/ > will enable build on Xen on Armv8-R AArch32 for MPU. > > Patches 1-4 can be applied independently. > Patch 5 depends on Luca's series. > > Changes from :- > > V1, v2 - Added "xen/arm32: mpu: Stubs to build MPU for arm32" to build Xen for > Armv8-R AArch32 on top of Luca's series. > > Ayan Kumar Halder (4): > xen/arm32: Move MM specific registers to enable_mmu I committed this patch and ... > xen/arm: Move some of the functions to common file > xen/arm32: Create the same boot-time MPU regions as arm64 > xen/arm32: mpu: Stubs to build MPU for arm32 > > Michal Orzel (1): > xen/arm32: Allow ARM_PA_BITS_40 only if !MPU this one. ~Michal ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-04-03 16:36 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-30 18:03 [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 1/5] xen/arm32: Move MM specific registers to enable_mmu Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 2/5] xen/arm: Move some of the functions to common file Ayan Kumar Halder 2025-03-30 21:06 ` Julien Grall 2025-03-31 8:28 ` Orzel, Michal 2025-03-31 12:38 ` Ayan Kumar Halder 2025-03-31 13:04 ` Julien Grall 2025-03-30 18:03 ` [PATCH v3 3/5] xen/arm32: Create the same boot-time MPU regions as arm64 Ayan Kumar Halder 2025-03-30 18:03 ` [PATCH v3 4/5] xen/arm32: Allow ARM_PA_BITS_40 only if !MPU Ayan Kumar Halder 2025-03-30 21:07 ` Julien Grall 2025-03-30 18:03 ` [PATCH v3 5/5] xen/arm32: mpu: Stubs to build MPU for arm32 Ayan Kumar Halder 2025-03-30 21:11 ` Julien Grall 2025-04-03 16:35 ` Ayan Kumar Halder 2025-03-31 10:13 ` [PATCH v3 0/5] Enable early bootup of Armv8-R AArch32 systems Orzel, Michal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.