Re: [PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to mmu/head.S
Hi, > On Sep 16, 2023, at 07:17, Henry Wang wrote: > > Hi Julien, > >> On Sep 16, 2023, at 05:41, Julien Grall wrote: >> >> Hi Henry, >> >> I realize that this was already committed. But something went wrong during >> the code movement. >> >> On 28/08/2023 02:32, Henry Wang wrote: >>> +/* >>> + * Enable mm (turn on the data cache and the MMU) for the boot CPU. >>> + * The function will return to the virtual address provided in LR (e.g. the >>> + * runtime mapping). >>> + * >>> + * Inputs: >>> + * lr : Virtual address to return to. >>> + * >>> + * Clobbers x0 - x5 >>> + */ >>> +ENTRY(enable_boot_cpu_mm) >>> +mov x5, lr >>> + >>> +blcreate_page_tables >>> +load_paddr x0, boot_pgtable >>> + >>> +blenable_mmu >>> +mov lr, x5 >>> + >>> +/* >>> + * The MMU is turned on and we are in the 1:1 mapping. Switch >>> + * to the runtime mapping. >>> + */ >>> +ldr x0, =1f >>> +brx0 >>> +1: >>> +/* >>> + * The 1:1 map may clash with other parts of the Xen virtual memory >>> + * layout. As it is not used anymore, remove it completely to >>> + * avoid having to worry about replacing existing mapping >>> + * afterwards. Function will return to primary_switched. >>> + */ >>> +b remove_identity_mapping >>> + >>> +/* >>> + * Below is supposed to be unreachable code, as "ret" in >>> + * remove_identity_mapping will use the return address in LR in >>> advance. >>> + */ >>> +b fail >> >> The "b fail" didn't exist in head.S. I guess this was due to a wrong >> rebase? Can you check if there is something else that went missing? > > Please correct me if I am wrong but I think the “b fail” of > enable_boot_cpu_mm() is > in the mmu head.S, see line 348 [1]. > > [1] > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/arm64/mmu/head.S;h=d71fdc69a531780501387fc5c717b4c41bb1b66a;hb=6734327d76be38d20f280ecc96392e385fbc1d8b#l348 Oh I realized we agreed to remove this line as it is unreachable, I will send a patch. Sorry about it. Kind regards, Henry > > Kind regards, > Henry > > >> >> Cheers, >> >> -- >> Julien Grall >
Re: [PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to mmu/head.S
Hi Julien, > On Sep 16, 2023, at 05:41, Julien Grall wrote: > > Hi Henry, > > I realize that this was already committed. But something went wrong during > the code movement. > > On 28/08/2023 02:32, Henry Wang wrote: >> +/* >> + * Enable mm (turn on the data cache and the MMU) for the boot CPU. >> + * The function will return to the virtual address provided in LR (e.g. the >> + * runtime mapping). >> + * >> + * Inputs: >> + * lr : Virtual address to return to. >> + * >> + * Clobbers x0 - x5 >> + */ >> +ENTRY(enable_boot_cpu_mm) >> +mov x5, lr >> + >> +blcreate_page_tables >> +load_paddr x0, boot_pgtable >> + >> +blenable_mmu >> +mov lr, x5 >> + >> +/* >> + * The MMU is turned on and we are in the 1:1 mapping. Switch >> + * to the runtime mapping. >> + */ >> +ldr x0, =1f >> +brx0 >> +1: >> +/* >> + * The 1:1 map may clash with other parts of the Xen virtual memory >> + * layout. As it is not used anymore, remove it completely to >> + * avoid having to worry about replacing existing mapping >> + * afterwards. Function will return to primary_switched. >> + */ >> +b remove_identity_mapping >> + >> +/* >> + * Below is supposed to be unreachable code, as "ret" in >> + * remove_identity_mapping will use the return address in LR in >> advance. >> + */ >> +b fail > > The "b fail" didn't exist in head.S. I guess this was due to a wrong > rebase? Can you check if there is something else that went missing? Please correct me if I am wrong but I think the “b fail” of enable_boot_cpu_mm() is in the mmu head.S, see line 348 [1]. [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/arm64/mmu/head.S;h=d71fdc69a531780501387fc5c717b4c41bb1b66a;hb=6734327d76be38d20f280ecc96392e385fbc1d8b#l348 Kind regards, Henry > > Cheers, > > -- > Julien Grall
Re: [PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to mmu/head.S
Hi Henry, I realize that this was already committed. But something went wrong during the code movement. On 28/08/2023 02:32, Henry Wang wrote: +/* + * Enable mm (turn on the data cache and the MMU) for the boot CPU. + * The function will return to the virtual address provided in LR (e.g. the + * runtime mapping). + * + * Inputs: + * lr : Virtual address to return to. + * + * Clobbers x0 - x5 + */ +ENTRY(enable_boot_cpu_mm) +mov x5, lr + +blcreate_page_tables +load_paddr x0, boot_pgtable + +blenable_mmu +mov lr, x5 + +/* + * The MMU is turned on and we are in the 1:1 mapping. Switch + * to the runtime mapping. + */ +ldr x0, =1f +brx0 +1: +/* + * The 1:1 map may clash with other parts of the Xen virtual memory + * layout. As it is not used anymore, remove it completely to + * avoid having to worry about replacing existing mapping + * afterwards. Function will return to primary_switched. + */ +b remove_identity_mapping + +/* + * Below is supposed to be unreachable code, as "ret" in + * remove_identity_mapping will use the return address in LR in advance. + */ +b fail The "b fail" didn't exist in head.S. I guess this was due to a wrong rebase? Can you check if there is something else that went missing? Cheers, -- Julien Grall
[PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to mmu/head.S
The MMU specific code in head.S will not be used on MPU systems. Instead of introducing more #ifdefs which will bring complexity to the code, move MMU related code to mmu/head.S and keep common code in head.S. Two notes while moving: - As "fail" in original head.S is very simple and this name is too easy to be conflicted, duplicate it in mmu/head.S instead of exporting it. - Use ENTRY() for enable_secondary_cpu_mm, enable_boot_cpu_mm and setup_fixmap as they will be used externally. Also move the assembly macros shared by head.S and mmu/head.S to macros.h. Note that, only the first 4KB of Xen image will be mapped as identity (PA == VA). At the moment, Xen guarantees this by having everything that needs to be used in the identity mapping in .text.header section of head.S, and the size will be checked by _idmap_start and _idmap_end at link time if this fits in 4KB. Since we are introducing a new head.S in this patch, although we can add .text.header to the new file to guarantee all identity map code still in the first 4KB. However, the order of these two files on this 4KB depends on the build toolchains. Hence, introduce a new section named .text.idmap in the region between _idmap_start and _idmap_end. And in Xen linker script, we force the .text.idmap contents to linked after .text.header. This will ensure code of head.S always be at the top of Xen binary. Signed-off-by: Henry Wang Signed-off-by: Wei Chen Reviewed-by: Julien Grall --- v6: - Reword the reason of adding "ENTRY()" in commit message. - Add Julien's Reviewed-by tag. v5: - Rebase on top of commit "xen/arm64: head: Introduce a helper to flush local TLBs". v4: - Rework "[v3,08/52] xen/arm64: move MMU related code from head.S to mmu/head.S" - Don't move the "yet to shared" macro such as print_reg. - Fold "[v3,04/52] xen/arm: add .text.idmap in ld script for Xen identity map sections" to this patch. Rework commit msg. --- xen/arch/arm/arm64/Makefile | 1 + xen/arch/arm/arm64/head.S | 492 +--- xen/arch/arm/arm64/mmu/Makefile | 1 + xen/arch/arm/arm64/mmu/head.S | 488 +++ xen/arch/arm/include/asm/arm64/macros.h | 36 ++ xen/arch/arm/xen.lds.S | 1 + 6 files changed, 528 insertions(+), 491 deletions(-) create mode 100644 xen/arch/arm/arm64/mmu/Makefile create mode 100644 xen/arch/arm/arm64/mmu/head.S diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile index 54ad55c75c..f89d5fb4fb 100644 --- a/xen/arch/arm/arm64/Makefile +++ b/xen/arch/arm/arm64/Makefile @@ -1,4 +1,5 @@ obj-y += lib/ +obj-$(CONFIG_MMU) += mmu/ obj-y += cache.o obj-y += cpufeature.o diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index f25a41d36c..3c8a12eda7 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -28,17 +28,6 @@ #include #endif -#define PT_PT 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */ -#define PT_MEM0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */ -#define PT_MEM_L3 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */ -#define PT_DEV0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */ -#define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */ - -/* Convenience defines to get slot used by Xen mapping. */ -#define XEN_ZEROETH_SLOTzeroeth_table_offset(XEN_VIRT_START) -#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START) -#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START) - #define __HEAD_FLAG_PAGE_SIZE ((PAGE_SHIFT - 10) / 2) #define __HEAD_FLAG_PHYS_BASE 1 @@ -85,19 +74,7 @@ * x30 - lr */ -#ifdef CONFIG_EARLY_PRINTK -/* - * Macro to print a string to the UART, if there is one. - * - * Clobbers x0 - x3 - */ -#define PRINT(_s) \ -mov x3, lr ; \ -adr_l x0, 98f ;\ -blasm_puts ; \ -mov lr, x3 ; \ -RODATA_STR(98, _s) - + #ifdef CONFIG_EARLY_PRINTK /* * Macro to print the value of register \xb * @@ -111,43 +88,11 @@ .endm #else /* CONFIG_EARLY_PRINTK */ -#define PRINT(s) - .macro print_reg xb .endm #endif /* !CONFIG_EARLY_PRINTK */ -/* - * Pseudo-op for PC relative adr , where is - * within the range +/- 4GB of the PC. - * - * @dst: destination register (64 bit wide) - * @sym: name of the symbol - */ -.macro adr_l, dst, sym -adrp \dst, \sym -add \dst, \dst, :lo12:\sym -.endm - -/* Load the physical address of a symbol into xb */ -.macro load_paddr xb, sym -ldr \xb, =\sym -add \xb, \xb, x20 -.endm - -/* - * Flush local TLBs - * - * See asm/arm64/flushtlb.h for the explanation of the sequence. - */ -.macro flush_xen_tlb_local -dsb nshst -tlbi alle2 -dsb nsh -isb -.endm - .section .text.header, "ax", %progbits /*.aarch64*/ @@ -484,402 +429,6 @@ cpu_init: ret ENDPROC(cpu_init) -/* - * Macro to find the slot number at a give
