* [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm @ 2019-06-10 19:31 Julien Grall 2019-06-10 19:31 ` [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC Julien Grall ` (16 more replies) 0 siblings, 17 replies; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:31 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Hi all, This is part of the boot/memory rework for Xen on Arm, but not sent as MM-PARTx as this is focusing on the boot code. Similar to the memory code, the boot code is not following the Arm Arm and could lead to memory corruption/TLB conflict abort. I am not aware of any platforms where Xen fails to boot, yet it should be fixed sooner rather than later. While making the code more compliant, I have also took the opportunity to simplify the boot and also add more documentation. After this series, the boot CPU and secondary CPUs path is mostly compliant with the Arm Arm. The only non-compliant places I am aware of are: 1) create_page_tables: Some rework is necessary to update the page-tables safely without the MMU on. 2) The switches between boot and runtime page-tables (for both boot CPU and secondary CPUs) are not safe. Both will be addressed in follow-up work. Lastly, only Arm64 has been modified so far. Arm32 requires the same modifications. It will be sent once I gathered feedback on the approach. Note that the series have a minor clash with MM-PART3 and reference some change done in MM-PART1. Yet the code is mostly self-contained to xen/arch/arm64/head.S. For convenience I provided a branch based on staging: git://xenbits.xen.org/people/julieng/xen-unstable.git branch boot/arm64/v1 Cheers, Julien Grall (17): xen/arm64: head Mark the end of subroutines with ENDPROC xen/arm64: head: Don't clobber x30/lr in the macro PRINT xen/arm64: head: Rework UART initialization on boot CPU xen/arm64: head: Don't "reserve" x24 for the CPUID xen/arm64: head: Introduce print_reg xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs xen/arm64: head: Rework and document check_cpu_mode() xen/arm64: head: Rework and document zero_bss() xen/arm64: head: Improve coding style and document cpu_init() xen/arm64: head: Improve coding style and document create_pages_tables() xen/arm64: head: Document enable_mmu() xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path xen/arm64: head: Don't setup the fixmap on secondary CPUs xen/arm64: head: Remove ID map as soon as it is not used xen/arm64: head: Rework and document setup_fixmap() xen/arm64: head: Rework and document launch() xen/arm64: Zero BSS after the MMU and D-cache is turned on xen/arch/arm/arm64/head.S | 396 +++++++++++++++++++++++++++++++++------------- xen/arch/arm/mm.c | 23 ++- 2 files changed, 306 insertions(+), 113 deletions(-) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall @ 2019-06-10 19:31 ` Julien Grall 2019-06-25 23:23 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT Julien Grall ` (15 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:31 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko putn() and puts() are two subroutines. Add ENDPROC for the benefits of static analysis tools and the reader. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index ddd3a33108..c8bbdf05a6 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -646,6 +646,7 @@ puts: b puts 1: ret +ENDPROC(puts) /* Print a 32-bit number in hex. Specific to the PL011 UART. * x0: Number to print. @@ -664,6 +665,7 @@ putn: subs x3, x3, #1 b.ne 1b ret +ENDPROC(putn) hex: .ascii "0123456789abcdef" .align 2 -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC 2019-06-10 19:31 ` [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC Julien Grall @ 2019-06-25 23:23 ` Stefano Stabellini 0 siblings, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-25 23:23 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > putn() and puts() are two subroutines. Add ENDPROC for the benefits of > static analysis tools and the reader. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/arm64/head.S | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index ddd3a33108..c8bbdf05a6 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -646,6 +646,7 @@ puts: > b puts > 1: > ret > +ENDPROC(puts) > > /* Print a 32-bit number in hex. Specific to the PL011 UART. > * x0: Number to print. > @@ -664,6 +665,7 @@ putn: > subs x3, x3, #1 > b.ne 1b > ret > +ENDPROC(putn) > > hex: .ascii "0123456789abcdef" > .align 2 > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall 2019-06-10 19:31 ` [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-25 23:35 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU Julien Grall ` (14 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko The current implementation of the macro PRINT will clobber x30/lr. This means the user should save lr if it cares about it. Follow-up patches will introduce more use of PRINT in place where lr should be preserved. Rather than requiring all the users to preserve lr, the macro PRINT is modified to save and restore it. While the comment state x3 will be clobbered, this is not the case. So PRINT will use x3 to preserve lr. Lastly, take the opportunity to move the comment on top of PRINT and use PRINT in init_uart. Both changes will be helpful in a follow-up patch. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index c8bbdf05a6..a5147c8d80 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -78,12 +78,17 @@ * x30 - lr */ -/* Macro to print a string to the UART, if there is one. - * Clobbers x0-x3. */ #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 x0, 98f ; \ bl puts ; \ + mov lr, x3 ; \ RODATA_STR(98, _s) #else /* CONFIG_EARLY_PRINTK */ #define PRINT(s) @@ -630,9 +635,8 @@ init_uart: #ifdef EARLY_PRINTK_INIT_UART early_uart_init x23, 0 #endif - adr x0, 1f - b puts -RODATA_STR(1, "- UART enabled -\r\n") + PRINT("- UART enabled -\r\n") + ret /* Print early debug messages. * x0: Nul-terminated string to print. -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-10 19:32 ` [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT Julien Grall @ 2019-06-25 23:35 ` Stefano Stabellini 2019-06-25 23:59 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-25 23:35 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: >> The current implementation of the macro PRINT will clobber x30/lr. This > means the user should save lr if it cares about it. By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer expression. Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > Follow-up patches will introduce more use of PRINT in place where lr > should be preserved. Rather than requiring all the users to preserve lr, > the macro PRINT is modified to save and restore it. > > While the comment state x3 will be clobbered, this is not the case. So > PRINT will use x3 to preserve lr. > > Lastly, take the opportunity to move the comment on top of PRINT and use > PRINT in init_uart. Both changes will be helpful in a follow-up patch. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index c8bbdf05a6..a5147c8d80 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -78,12 +78,17 @@ > * x30 - lr > */ > > -/* Macro to print a string to the UART, if there is one. > - * Clobbers x0-x3. */ > #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 x0, 98f ; \ > bl puts ; \ > + mov lr, x3 ; \ > RODATA_STR(98, _s) > #else /* CONFIG_EARLY_PRINTK */ > #define PRINT(s) > @@ -630,9 +635,8 @@ init_uart: > #ifdef EARLY_PRINTK_INIT_UART > early_uart_init x23, 0 > #endif > - adr x0, 1f > - b puts > -RODATA_STR(1, "- UART enabled -\r\n") > + PRINT("- UART enabled -\r\n") > + ret > > /* Print early debug messages. > * x0: Nul-terminated string to print. > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-25 23:35 ` Stefano Stabellini @ 2019-06-25 23:59 ` Stefano Stabellini 2019-06-26 9:07 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-25 23:59 UTC (permalink / raw) To: Stefano Stabellini Cc: Oleksandr_Tyshchenko, xen-devel, Julien Grall, andrii_anisov, andre.przywara On Tue, 25 Jun 2019, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: > >> The current implementation of the macro PRINT will clobber x30/lr. This > > means the user should save lr if it cares about it. > > By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer > expression. No of course not! You meant x30 which is a synonym of lr! It is just that in this case it is also supposed to clobber x0-x3 -- I got confused! The commit message is also fine as is then. More below. > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > > > Follow-up patches will introduce more use of PRINT in place where lr > > should be preserved. Rather than requiring all the users to preserve lr, > > the macro PRINT is modified to save and restore it. > > > > While the comment state x3 will be clobbered, this is not the case. So > > PRINT will use x3 to preserve lr. > > > > Lastly, take the opportunity to move the comment on top of PRINT and use > > PRINT in init_uart. Both changes will be helpful in a follow-up patch. > > > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > --- > > xen/arch/arm/arm64/head.S | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > > index c8bbdf05a6..a5147c8d80 100644 > > --- a/xen/arch/arm/arm64/head.S > > +++ b/xen/arch/arm/arm64/head.S > > @@ -78,12 +78,17 @@ > > * x30 - lr > > */ > > > > -/* Macro to print a string to the UART, if there is one. > > - * Clobbers x0-x3. */ > > #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 x0, 98f ; \ > > bl puts ; \ > > + mov lr, x3 ; \ > > RODATA_STR(98, _s) Strangely enough I get a build error with gcc 7.3.1, but if I use x30 instead of lr, it builds fine. Have you seen this before? The error is: arm64/head.S: Assembler messages: arm64/head.S:272: Error: operand 1 must be an integer register -- `mov lr,x3' [...] arm64/head.S:272: Error: undefined symbol lr used as an immediate value [...] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-25 23:59 ` Stefano Stabellini @ 2019-06-26 9:07 ` Julien Grall 2019-06-26 15:27 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-26 9:07 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 00:59, Stefano Stabellini wrote: > On Tue, 25 Jun 2019, Stefano Stabellini wrote: >> On Mon, 10 Jun 2019, Julien Grall wrote: >>>> The current implementation of the macro PRINT will clobber x30/lr. This >>> means the user should save lr if it cares about it. >> >> By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer >> expression. > > No of course not! You meant x30 which is a synonym of lr! It is just > that in this case it is also supposed to clobber x0-x3 -- I got > confused! The commit message is also fine as is then. More below. > > >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >> >> >>> Follow-up patches will introduce more use of PRINT in place where lr >>> should be preserved. Rather than requiring all the users to preserve lr, >>> the macro PRINT is modified to save and restore it. >>> >>> While the comment state x3 will be clobbered, this is not the case. So >>> PRINT will use x3 to preserve lr. >>> >>> Lastly, take the opportunity to move the comment on top of PRINT and use >>> PRINT in init_uart. Both changes will be helpful in a follow-up patch. >>> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>> --- >>> xen/arch/arm/arm64/head.S | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>> index c8bbdf05a6..a5147c8d80 100644 >>> --- a/xen/arch/arm/arm64/head.S >>> +++ b/xen/arch/arm/arm64/head.S >>> @@ -78,12 +78,17 @@ >>> * x30 - lr >>> */ >>> >>> -/* Macro to print a string to the UART, if there is one. >>> - * Clobbers x0-x3. */ >>> #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 x0, 98f ; \ >>> bl puts ; \ >>> + mov lr, x3 ; \ >>> RODATA_STR(98, _s) > > Strangely enough I get a build error with gcc 7.3.1, but if I use x30 > instead of lr, it builds fine. Have you seen this before? Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is not all the assembler is able to understand "lr". In the file entry.S we have the following line: lr .req x30 // link register Could you give a try to add the line in head.S? > The error is: > > arm64/head.S: Assembler messages: > arm64/head.S:272: Error: operand 1 must be an integer register -- `mov lr,x3' > [...] > arm64/head.S:272: Error: undefined symbol lr used as an immediate value > [...] > Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-26 9:07 ` Julien Grall @ 2019-06-26 15:27 ` Stefano Stabellini 2019-06-26 15:28 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 15:27 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Wed, 26 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 26/06/2019 00:59, Stefano Stabellini wrote: > > On Tue, 25 Jun 2019, Stefano Stabellini wrote: > > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > > > The current implementation of the macro PRINT will clobber x30/lr. > > > > > This > > > > means the user should save lr if it cares about it. > > > > > > By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer > > > expression. > > > > No of course not! You meant x30 which is a synonym of lr! It is just > > that in this case it is also supposed to clobber x0-x3 -- I got > > confused! The commit message is also fine as is then. More below. > > > > > > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > > > > > > > > > Follow-up patches will introduce more use of PRINT in place where lr > > > > should be preserved. Rather than requiring all the users to preserve lr, > > > > the macro PRINT is modified to save and restore it. > > > > > > > > While the comment state x3 will be clobbered, this is not the case. So > > > > PRINT will use x3 to preserve lr. > > > > > > > > Lastly, take the opportunity to move the comment on top of PRINT and use > > > > PRINT in init_uart. Both changes will be helpful in a follow-up patch. > > > > > > > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > > > --- > > > > xen/arch/arm/arm64/head.S | 14 +++++++++----- > > > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > > > > index c8bbdf05a6..a5147c8d80 100644 > > > > --- a/xen/arch/arm/arm64/head.S > > > > +++ b/xen/arch/arm/arm64/head.S > > > > @@ -78,12 +78,17 @@ > > > > * x30 - lr > > > > */ > > > > -/* Macro to print a string to the UART, if there is one. > > > > - * Clobbers x0-x3. */ > > > > #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 x0, 98f ; \ > > > > bl puts ; \ > > > > + mov lr, x3 ; \ > > > > RODATA_STR(98, _s) > > > > Strangely enough I get a build error with gcc 7.3.1, but if I use x30 > > instead of lr, it builds fine. Have you seen this before? > > Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is not > all the assembler is able to understand "lr". > > In the file entry.S we have the following line: > > lr .req x30 // link register > > > Could you give a try to add the line in head.S? That works _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-26 15:27 ` Stefano Stabellini @ 2019-06-26 15:28 ` Julien Grall 2019-06-26 18:32 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-26 15:28 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 16:27, Stefano Stabellini wrote: > On Wed, 26 Jun 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 26/06/2019 00:59, Stefano Stabellini wrote: >>> On Tue, 25 Jun 2019, Stefano Stabellini wrote: >>>> On Mon, 10 Jun 2019, Julien Grall wrote: >>>>>> The current implementation of the macro PRINT will clobber x30/lr. >>>>>> This >>>>> means the user should save lr if it cares about it. >>>> >>>> By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer >>>> expression. >>> >>> No of course not! You meant x30 which is a synonym of lr! It is just >>> that in this case it is also supposed to clobber x0-x3 -- I got >>> confused! The commit message is also fine as is then. More below. >>> >>> >>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >>>> >>>> >>>>> Follow-up patches will introduce more use of PRINT in place where lr >>>>> should be preserved. Rather than requiring all the users to preserve lr, >>>>> the macro PRINT is modified to save and restore it. >>>>> >>>>> While the comment state x3 will be clobbered, this is not the case. So >>>>> PRINT will use x3 to preserve lr. >>>>> >>>>> Lastly, take the opportunity to move the comment on top of PRINT and use >>>>> PRINT in init_uart. Both changes will be helpful in a follow-up patch. >>>>> >>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>> --- >>>>> xen/arch/arm/arm64/head.S | 14 +++++++++----- >>>>> 1 file changed, 9 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>>>> index c8bbdf05a6..a5147c8d80 100644 >>>>> --- a/xen/arch/arm/arm64/head.S >>>>> +++ b/xen/arch/arm/arm64/head.S >>>>> @@ -78,12 +78,17 @@ >>>>> * x30 - lr >>>>> */ >>>>> -/* Macro to print a string to the UART, if there is one. >>>>> - * Clobbers x0-x3. */ >>>>> #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 x0, 98f ; \ >>>>> bl puts ; \ >>>>> + mov lr, x3 ; \ >>>>> RODATA_STR(98, _s) >>> >>> Strangely enough I get a build error with gcc 7.3.1, but if I use x30 >>> instead of lr, it builds fine. Have you seen this before? >> >> Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is not >> all the assembler is able to understand "lr". >> >> In the file entry.S we have the following line: >> >> lr .req x30 // link register >> >> >> Could you give a try to add the line in head.S? > > That works Thank you. I thought a bit more during the day and decided to use "x30" directly rather than lr. We can decide to revisit it if we think the code is too difficult to read. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-26 15:28 ` Julien Grall @ 2019-06-26 18:32 ` Stefano Stabellini 2019-06-26 19:24 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 18:32 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Wed, 26 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 26/06/2019 16:27, Stefano Stabellini wrote: > > On Wed, 26 Jun 2019, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 26/06/2019 00:59, Stefano Stabellini wrote: > > > > On Tue, 25 Jun 2019, Stefano Stabellini wrote: > > > > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > > > > > The current implementation of the macro PRINT will clobber > > > > > > > x30/lr. > > > > > > > This > > > > > > means the user should save lr if it cares about it. > > > > > > > > > > By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer > > > > > expression. > > > > > > > > No of course not! You meant x30 which is a synonym of lr! It is just > > > > that in this case it is also supposed to clobber x0-x3 -- I got > > > > confused! The commit message is also fine as is then. More below. > > > > > > > > > > > > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > > > > > > > > > > > > > > > Follow-up patches will introduce more use of PRINT in place where lr > > > > > > should be preserved. Rather than requiring all the users to preserve > > > > > > lr, > > > > > > the macro PRINT is modified to save and restore it. > > > > > > > > > > > > While the comment state x3 will be clobbered, this is not the case. > > > > > > So > > > > > > PRINT will use x3 to preserve lr. > > > > > > > > > > > > Lastly, take the opportunity to move the comment on top of PRINT and > > > > > > use > > > > > > PRINT in init_uart. Both changes will be helpful in a follow-up > > > > > > patch. > > > > > > > > > > > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > > > > > --- > > > > > > xen/arch/arm/arm64/head.S | 14 +++++++++----- > > > > > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > > > > > > index c8bbdf05a6..a5147c8d80 100644 > > > > > > --- a/xen/arch/arm/arm64/head.S > > > > > > +++ b/xen/arch/arm/arm64/head.S > > > > > > @@ -78,12 +78,17 @@ > > > > > > * x30 - lr > > > > > > */ > > > > > > -/* Macro to print a string to the UART, if there is one. > > > > > > - * Clobbers x0-x3. */ > > > > > > #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 x0, 98f ; \ > > > > > > bl puts ; \ > > > > > > + mov lr, x3 ; \ > > > > > > RODATA_STR(98, _s) > > > > > > > > Strangely enough I get a build error with gcc 7.3.1, but if I use x30 > > > > instead of lr, it builds fine. Have you seen this before? > > > > > > Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is > > > not > > > all the assembler is able to understand "lr". > > > > > > In the file entry.S we have the following line: > > > > > > lr .req x30 // link register > > > > > > > > > Could you give a try to add the line in head.S? > > > > That works > > Thank you. > > I thought a bit more during the day and decided to use "x30" directly rather > than lr. We can decide to revisit it if we think the code is too difficult to > read. I was going to suggest x30 too yesterday, but if we can make `lr' work then that would be my preference because it makes it more immediately obvious what the code is doing. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT 2019-06-26 18:32 ` Stefano Stabellini @ 2019-06-26 19:24 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 19:24 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/26/19 7:32 PM, Stefano Stabellini wrote: > On Wed, 26 Jun 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 26/06/2019 16:27, Stefano Stabellini wrote: >>> On Wed, 26 Jun 2019, Julien Grall wrote: >>>> Hi Stefano, >>>> >>>> On 26/06/2019 00:59, Stefano Stabellini wrote: >>>>> On Tue, 25 Jun 2019, Stefano Stabellini wrote: >>>>>> On Mon, 10 Jun 2019, Julien Grall wrote: >>>>>>>> The current implementation of the macro PRINT will clobber >>>>>>>> x30/lr. >>>>>>>> This >>>>>>> means the user should save lr if it cares about it. >>>>>> >>>>>> By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer >>>>>> expression. >>>>> >>>>> No of course not! You meant x30 which is a synonym of lr! It is just >>>>> that in this case it is also supposed to clobber x0-x3 -- I got >>>>> confused! The commit message is also fine as is then. More below. >>>>> >>>>> >>>>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >>>>>> >>>>>> >>>>>>> Follow-up patches will introduce more use of PRINT in place where lr >>>>>>> should be preserved. Rather than requiring all the users to preserve >>>>>>> lr, >>>>>>> the macro PRINT is modified to save and restore it. >>>>>>> >>>>>>> While the comment state x3 will be clobbered, this is not the case. >>>>>>> So >>>>>>> PRINT will use x3 to preserve lr. >>>>>>> >>>>>>> Lastly, take the opportunity to move the comment on top of PRINT and >>>>>>> use >>>>>>> PRINT in init_uart. Both changes will be helpful in a follow-up >>>>>>> patch. >>>>>>> >>>>>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>>>>>> --- >>>>>>> xen/arch/arm/arm64/head.S | 14 +++++++++----- >>>>>>> 1 file changed, 9 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>>>>>> index c8bbdf05a6..a5147c8d80 100644 >>>>>>> --- a/xen/arch/arm/arm64/head.S >>>>>>> +++ b/xen/arch/arm/arm64/head.S >>>>>>> @@ -78,12 +78,17 @@ >>>>>>> * x30 - lr >>>>>>> */ >>>>>>> -/* Macro to print a string to the UART, if there is one. >>>>>>> - * Clobbers x0-x3. */ >>>>>>> #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 x0, 98f ; \ >>>>>>> bl puts ; \ >>>>>>> + mov lr, x3 ; \ >>>>>>> RODATA_STR(98, _s) >>>>> >>>>> Strangely enough I get a build error with gcc 7.3.1, but if I use x30 >>>>> instead of lr, it builds fine. Have you seen this before? >>>> >>>> Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is >>>> not >>>> all the assembler is able to understand "lr". >>>> >>>> In the file entry.S we have the following line: >>>> >>>> lr .req x30 // link register >>>> >>>> >>>> Could you give a try to add the line in head.S? >>> >>> That works >> >> Thank you. >> >> I thought a bit more during the day and decided to use "x30" directly rather >> than lr. We can decide to revisit it if we think the code is too difficult to >> read. > > I was going to suggest x30 too yesterday, but if we can make `lr' work > then that would be my preference because it makes it more immediately > obvious what the code is doing. I will have a look to move the line in an header. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall 2019-06-10 19:31 ` [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-25 23:49 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID Julien Grall ` (13 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Anything executed after the label common_start can be executed on all CPUs. However most of the instructions executed between the label common_start and init_uart are not executed on the boot CPU. The only instructions executed are to lookup the CPUID so it can be printed on the console (if earlyprintk is enabled). Printing the CPUID is not entirely useful to have for the boot CPU and requires a conditional branch to bypass unused instructions. Furthermore, the function init_uart is only called for boot CPU requiring another conditional branch. This makes the code a bit tricky to follow. The UART initialization is now moved before the label common_start. This now requires to have a slightly altered print for the boot CPU and set the early UART base address in each the two path (boot CPU and secondary CPUs). This has the nice effect to remove a couple of conditional branch in the code. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index a5147c8d80..fd432ee15d 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -265,6 +265,12 @@ real_start_efi: load_paddr x21, _sdtb #endif + /* Initialize the UART if earlyprintk has been enabled. */ +#ifdef CONFIG_EARLY_PRINTK + bl init_uart +#endif + PRINT("- Boot CPU booting -\r\n") + mov x22, #0 /* x22 := is_secondary_cpu */ b common_start @@ -281,14 +287,11 @@ GLOBAL(init_secondary) /* Boot CPU already zero BSS so skip it on secondary CPUs. */ mov x26, #1 /* X26 := skip_zero_bss */ -common_start: mrs x0, mpidr_el1 ldr x13, =(~MPIDR_HWID_MASK) bic x24, x0, x13 /* Mask out flags to get CPU ID */ - /* Non-boot CPUs wait here until __cpu_up is ready for them */ - cbz x22, 1f - + /* Wait here until __cpu_up is ready to handle the CPU */ load_paddr x0, smp_up_cpu dsb sy 2: ldr x1, [x0] @@ -300,14 +303,14 @@ common_start: #ifdef CONFIG_EARLY_PRINTK ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ - cbnz x22, 1f - bl init_uart /* Boot CPU sets up the UART too */ -1: PRINT("- CPU ") + PRINT("- CPU ") mov x0, x24 bl putn PRINT(" booting -\r\n") #endif +common_start: + PRINT("- Current EL ") mrs x4, CurrentEL mov x0, x4 @@ -628,10 +631,16 @@ ENTRY(switch_ttbr) ret #ifdef CONFIG_EARLY_PRINTK -/* Bring up the UART. - * x23: Early UART base address - * Clobbers x0-x1 */ +/* + * Initialize the UART. Should only be called on the boot CPU. + * + * Ouput: + * x23: Early UART base physical address + * + * Clobbers x0 - x1 + */ init_uart: + ldr x23, =EARLY_UART_BASE_ADDRESS #ifdef EARLY_PRINTK_INIT_UART early_uart_init x23, 0 #endif -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU 2019-06-10 19:32 ` [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU Julien Grall @ 2019-06-25 23:49 ` Stefano Stabellini 0 siblings, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-25 23:49 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > Anything executed after the label common_start can be executed on all > CPUs. However most of the instructions executed between the label > common_start and init_uart are not executed on the boot CPU. > > The only instructions executed are to lookup the CPUID so it can be > printed on the console (if earlyprintk is enabled). Printing the CPUID > is not entirely useful to have for the boot CPU and requires a > conditional branch to bypass unused instructions. > > Furthermore, the function init_uart is only called for boot CPU > requiring another conditional branch. This makes the code a bit tricky > to follow. > > The UART initialization is now moved before the label common_start. This > now requires to have a slightly altered print for the boot CPU and set > the early UART base address in each the two path (boot CPU and > secondary CPUs). > > This has the nice effect to remove a couple of conditional branch in > the code. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/arm64/head.S | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index a5147c8d80..fd432ee15d 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -265,6 +265,12 @@ real_start_efi: > load_paddr x21, _sdtb > #endif > > + /* Initialize the UART if earlyprintk has been enabled. */ > +#ifdef CONFIG_EARLY_PRINTK > + bl init_uart > +#endif > + PRINT("- Boot CPU booting -\r\n") > + > mov x22, #0 /* x22 := is_secondary_cpu */ > > b common_start > @@ -281,14 +287,11 @@ GLOBAL(init_secondary) > /* Boot CPU already zero BSS so skip it on secondary CPUs. */ > mov x26, #1 /* X26 := skip_zero_bss */ > > -common_start: > mrs x0, mpidr_el1 > ldr x13, =(~MPIDR_HWID_MASK) > bic x24, x0, x13 /* Mask out flags to get CPU ID */ > > - /* Non-boot CPUs wait here until __cpu_up is ready for them */ > - cbz x22, 1f > - > + /* Wait here until __cpu_up is ready to handle the CPU */ > load_paddr x0, smp_up_cpu > dsb sy > 2: ldr x1, [x0] > @@ -300,14 +303,14 @@ common_start: > > #ifdef CONFIG_EARLY_PRINTK > ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ > - cbnz x22, 1f > - bl init_uart /* Boot CPU sets up the UART too */ > -1: PRINT("- CPU ") > + PRINT("- CPU ") > mov x0, x24 > bl putn > PRINT(" booting -\r\n") > #endif > > +common_start: > + > PRINT("- Current EL ") > mrs x4, CurrentEL > mov x0, x4 > @@ -628,10 +631,16 @@ ENTRY(switch_ttbr) > ret > > #ifdef CONFIG_EARLY_PRINTK > -/* Bring up the UART. > - * x23: Early UART base address > - * Clobbers x0-x1 */ > +/* > + * Initialize the UART. Should only be called on the boot CPU. > + * > + * Ouput: > + * x23: Early UART base physical address > + * > + * Clobbers x0 - x1 > + */ > init_uart: > + ldr x23, =EARLY_UART_BASE_ADDRESS > #ifdef EARLY_PRINTK_INIT_UART > early_uart_init x23, 0 > #endif > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (2 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 0:01 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg Julien Grall ` (12 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko After the recent rework, the CPUID is only used at the very beginning of the secondary CPUs boot path. So there is no need to "reserve" x24 for he CPUID. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index fd432ee15d..84e26582c4 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -69,7 +69,7 @@ * x21 - DTB address (boot cpu only) * x22 - is_secondary_cpu * x23 - UART address - * x24 - cpuid + * x24 - * x25 - identity map in place * x26 - skip_zero_bss * x27 - -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID 2019-06-10 19:32 ` [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID Julien Grall @ 2019-06-26 0:01 ` Stefano Stabellini 2019-06-26 9:09 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 0:01 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > After the recent rework, the CPUID is only used at the very beginning of > the secondary CPUs boot path. So there is no need to "reserve" x24 for > he CPUID. If you are going to resend the series it would probably make sense to fold it in the previous patch, but it is also OK as is Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index fd432ee15d..84e26582c4 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -69,7 +69,7 @@ > * x21 - DTB address (boot cpu only) > * x22 - is_secondary_cpu > * x23 - UART address > - * x24 - cpuid > + * x24 - > * x25 - identity map in place > * x26 - skip_zero_bss > * x27 - > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID 2019-06-26 0:01 ` Stefano Stabellini @ 2019-06-26 9:09 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 9:09 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 01:01, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> After the recent rework, the CPUID is only used at the very beginning of >> the secondary CPUs boot path. So there is no need to "reserve" x24 for >> he CPUID. > > If you are going to resend the series it would probably make sense to > fold it in the previous patch, but it is also OK as is I am planning to resend the series. So I will fold it. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (3 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 0:09 ` Stefano Stabellini 2019-07-15 18:46 ` Volodymyr Babchuk 2019-06-10 19:32 ` [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs Julien Grall ` (11 subsequent siblings) 16 siblings, 2 replies; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko At the moment, the user should save x30/lr if it cares about it. Follow-up patches will introduce more use of putn in place where lr should be preserved. Furthermore, any user of putn should also move the value to register x0 if it was stored in a different register. For convenience, a new macro is introduced to print a given register. The macro will take care for us to move the value to x0 and also preserve lr. Lastly the new macro is used to replace all the callsite of putn. This will simplify rework/review later on. Note that CurrentEL is now stored in x5 instead of x4 because the latter will be clobbered by the macro print_reg. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 84e26582c4..9142b4a774 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -90,8 +90,25 @@ bl puts ; \ mov lr, x3 ; \ RODATA_STR(98, _s) + +/* + * Macro to print the value of register \xb + * + * Clobbers x0 - x4 + */ +.macro print_reg xb + mov x4, lr + mov x0, \xb + bl putn + mov lr, x4 +.endm + #else /* CONFIG_EARLY_PRINTK */ #define PRINT(s) + +.macro print_reg xb +.endm + #endif /* !CONFIG_EARLY_PRINTK */ /* Load the physical address of a symbol into xb */ @@ -304,22 +321,20 @@ GLOBAL(init_secondary) #ifdef CONFIG_EARLY_PRINTK ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ PRINT("- CPU ") - mov x0, x24 - bl putn + print_reg x24 PRINT(" booting -\r\n") #endif common_start: PRINT("- Current EL ") - mrs x4, CurrentEL - mov x0, x4 - bl putn + mrs x5, CurrentEL + print_reg x5 PRINT(" -\r\n") /* Are we in EL2 */ - cmp x4, #PSR_MODE_EL2t - ccmp x4, #PSR_MODE_EL2h, #0x4, ne + cmp x5, #PSR_MODE_EL2t + ccmp x5, #PSR_MODE_EL2h, #0x4, ne b.eq el2 /* Yes */ /* OK, we're boned. */ -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg 2019-06-10 19:32 ` [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg Julien Grall @ 2019-06-26 0:09 ` Stefano Stabellini 2019-06-26 9:10 ` Julien Grall 2019-07-15 18:46 ` Volodymyr Babchuk 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 0:09 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > At the moment, the user should save x30/lr if it cares about it. > > Follow-up patches will introduce more use of putn in place where lr > should be preserved. > > Furthermore, any user of putn should also move the value to register x0 > if it was stored in a different register. > > For convenience, a new macro is introduced to print a given register. > The macro will take care for us to move the value to x0 and also > preserve lr. > > Lastly the new macro is used to replace all the callsite of putn. This > will simplify rework/review later on. > > Note that CurrentEL is now stored in x5 instead of x4 because the latter > will be clobbered by the macro print_reg. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 84e26582c4..9142b4a774 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -90,8 +90,25 @@ > bl puts ; \ > mov lr, x3 ; \ > RODATA_STR(98, _s) > + > +/* > + * Macro to print the value of register \xb > + * > + * Clobbers x0 - x4 > + */ > +.macro print_reg xb > + mov x4, lr > + mov x0, \xb > + bl putn > + mov lr, x4 I have the same weird issues with my compiler as before, replacing 'lr' with 'x30' solves the problem. This patch looks OK though. > +.endm > + > #else /* CONFIG_EARLY_PRINTK */ > #define PRINT(s) > + > +.macro print_reg xb > +.endm > + > #endif /* !CONFIG_EARLY_PRINTK */ > > /* Load the physical address of a symbol into xb */ > @@ -304,22 +321,20 @@ GLOBAL(init_secondary) > #ifdef CONFIG_EARLY_PRINTK > ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ > PRINT("- CPU ") > - mov x0, x24 > - bl putn > + print_reg x24 > PRINT(" booting -\r\n") > #endif > > common_start: > > PRINT("- Current EL ") > - mrs x4, CurrentEL > - mov x0, x4 > - bl putn > + mrs x5, CurrentEL > + print_reg x5 > PRINT(" -\r\n") > > /* Are we in EL2 */ > - cmp x4, #PSR_MODE_EL2t > - ccmp x4, #PSR_MODE_EL2h, #0x4, ne > + cmp x5, #PSR_MODE_EL2t > + ccmp x5, #PSR_MODE_EL2h, #0x4, ne > b.eq el2 /* Yes */ > > /* OK, we're boned. */ > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg 2019-06-26 0:09 ` Stefano Stabellini @ 2019-06-26 9:10 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 9:10 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 01:09, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> At the moment, the user should save x30/lr if it cares about it. >> >> Follow-up patches will introduce more use of putn in place where lr >> should be preserved. >> >> Furthermore, any user of putn should also move the value to register x0 >> if it was stored in a different register. >> >> For convenience, a new macro is introduced to print a given register. >> The macro will take care for us to move the value to x0 and also >> preserve lr. >> >> Lastly the new macro is used to replace all the callsite of putn. This >> will simplify rework/review later on. >> >> Note that CurrentEL is now stored in x5 instead of x4 because the latter >> will be clobbered by the macro print_reg. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 29 ++++++++++++++++++++++------- >> 1 file changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 84e26582c4..9142b4a774 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -90,8 +90,25 @@ >> bl puts ; \ >> mov lr, x3 ; \ >> RODATA_STR(98, _s) >> + >> +/* >> + * Macro to print the value of register \xb >> + * >> + * Clobbers x0 - x4 >> + */ >> +.macro print_reg xb >> + mov x4, lr >> + mov x0, \xb >> + bl putn >> + mov lr, x4 > > I have the same weird issues with my compiler as before, replacing 'lr' > with 'x30' solves the problem. Can you have a try with the following line? lr .req x30 // link register Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg 2019-06-10 19:32 ` [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg Julien Grall 2019-06-26 0:09 ` Stefano Stabellini @ 2019-07-15 18:46 ` Volodymyr Babchuk 2019-07-16 9:55 ` Julien Grall 1 sibling, 1 reply; 70+ messages in thread From: Volodymyr Babchuk @ 2019-07-15 18:46 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr Tyshchenko, Stefano Stabellini, Andrii Anisov, andre.przywara Hi Julien, Julien Grall writes: > At the moment, the user should save x30/lr if it cares about it. > > Follow-up patches will introduce more use of putn in place where lr > should be preserved. > > Furthermore, any user of putn should also move the value to register x0 > if it was stored in a different register. > > For convenience, a new macro is introduced to print a given register. > The macro will take care for us to move the value to x0 and also > preserve lr. > > Lastly the new macro is used to replace all the callsite of putn. This > will simplify rework/review later on. > > Note that CurrentEL is now stored in x5 instead of x4 because the latter > will be clobbered by the macro print_reg. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 84e26582c4..9142b4a774 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -90,8 +90,25 @@ > bl puts ; \ > mov lr, x3 ; \ > RODATA_STR(98, _s) > + > +/* > + * Macro to print the value of register \xb > + * > + * Clobbers x0 - x4 > + */ Despite its name, this macro can't print x4. I would recommend adding at least comment about this. Static assertion would be even better, but looks like we don't have them for asm code. > +.macro print_reg xb > + mov x4, lr > + mov x0, \xb > + bl putn > + mov lr, x4 > +.endm > + > #else /* CONFIG_EARLY_PRINTK */ > #define PRINT(s) > + > +.macro print_reg xb > +.endm > + > #endif /* !CONFIG_EARLY_PRINTK */ > > /* Load the physical address of a symbol into xb */ > @@ -304,22 +321,20 @@ GLOBAL(init_secondary) > #ifdef CONFIG_EARLY_PRINTK > ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */ > PRINT("- CPU ") > - mov x0, x24 > - bl putn > + print_reg x24 > PRINT(" booting -\r\n") > #endif > > common_start: > > PRINT("- Current EL ") > - mrs x4, CurrentEL > - mov x0, x4 > - bl putn > + mrs x5, CurrentEL > + print_reg x5 > PRINT(" -\r\n") > > /* Are we in EL2 */ > - cmp x4, #PSR_MODE_EL2t > - ccmp x4, #PSR_MODE_EL2h, #0x4, ne > + cmp x5, #PSR_MODE_EL2t > + ccmp x5, #PSR_MODE_EL2h, #0x4, ne > b.eq el2 /* Yes */ > > /* OK, we're boned. */ -- Best regards, Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg 2019-07-15 18:46 ` Volodymyr Babchuk @ 2019-07-16 9:55 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-07-16 9:55 UTC (permalink / raw) To: Volodymyr Babchuk Cc: xen-devel, Oleksandr Tyshchenko, Stefano Stabellini, Andrii Anisov, andre.przywara On 7/15/19 7:46 PM, Volodymyr Babchuk wrote: > Hi Julien, Hi, > Julien Grall writes: > >> At the moment, the user should save x30/lr if it cares about it. >> >> Follow-up patches will introduce more use of putn in place where lr >> should be preserved. >> >> Furthermore, any user of putn should also move the value to register x0 >> if it was stored in a different register. >> >> For convenience, a new macro is introduced to print a given register. >> The macro will take care for us to move the value to x0 and also >> preserve lr. >> >> Lastly the new macro is used to replace all the callsite of putn. This >> will simplify rework/review later on. >> >> Note that CurrentEL is now stored in x5 instead of x4 because the latter >> will be clobbered by the macro print_reg. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 29 ++++++++++++++++++++++------- >> 1 file changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 84e26582c4..9142b4a774 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -90,8 +90,25 @@ >> bl puts ; \ >> mov lr, x3 ; \ >> RODATA_STR(98, _s) >> + >> +/* >> + * Macro to print the value of register \xb >> + * >> + * Clobbers x0 - x4 >> + */ > > Despite its name, this macro can't print x4. I would recommend adding at > least comment about this. Static assertion would be even better, but > looks like we don't have them for asm code. I have a new version of the function allowing to print x4. It is basically just swapping "mov x4, lr" with "mov x0, \xb". > >> +.macro print_reg xb >> + mov x4, lr >> + mov x0, \xb >> + bl putn >> + mov lr, x4 >> +.endm >> + >> #else /* CONFIG_EARLY_PRINTK */ >> #define PRINT(s) Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (4 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:00 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() Julien Grall ` (10 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko The boot code is currently quite difficult to go through because of the lack of documentation and a number of indirection to avoid executing some path in either the boot CPU or secondary CPUs. In an attempt to make the boot code easier to follow, each parts of the boot are now in separate functions. Furthermore, the paths for the boot CPU and secondary CPUs are now distincted and for now will call each functions. Follow-ups will remove unecessary calls and do further improvement (such as adding documentation and reshuffling). Note that the switch from using the ID mapping to the runtime mapping is duplicated for each path. This is because in the future we will need to stay longer in the ID mapping for the boot CPU. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 57 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 9142b4a774..ccd8a1b0a8 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -290,7 +290,19 @@ real_start_efi: mov x22, #0 /* x22 := is_secondary_cpu */ - b common_start + bl check_cpu_mode + bl zero_bss + bl cpu_init + bl create_page_tables + bl enable_mmu + + /* We are still in the ID map. Jump to the runtime Virtual Address. */ + ldr x0, =primary_switched + br x0 +primary_switched: + bl setup_fixmap + b launch +ENDPROC(real_start) GLOBAL(init_secondary) msr DAIFSet, 0xf /* Disable all interrupts */ @@ -324,9 +336,21 @@ GLOBAL(init_secondary) print_reg x24 PRINT(" booting -\r\n") #endif - -common_start: - + bl check_cpu_mode + bl zero_bss + bl cpu_init + bl create_page_tables + bl enable_mmu + + /* We are still in the ID map. Jump to the runtime Virtual Address. */ + ldr x0, =secondary_switched + br x0 +secondary_switched: + bl setup_fixmap + b launch +ENDPROC(init_secondary) + +check_cpu_mode: PRINT("- Current EL ") mrs x5, CurrentEL print_reg x5 @@ -343,7 +367,10 @@ common_start: b fail el2: PRINT("- Xen starting at EL2 -\r\n") + ret +ENDPROC(check_cpu_mode) +zero_bss: /* Zero BSS only when requested */ cbnz x26, skip_bss @@ -356,6 +383,10 @@ el2: PRINT("- Xen starting at EL2 -\r\n") b.lo 1b skip_bss: + ret +ENDPROC(zero_bss) + +cpu_init: PRINT("- Setting up control registers -\r\n") /* Set up memory attribute type tables */ @@ -390,7 +421,10 @@ skip_bss: * are handled using the EL2 stack pointer, rather * than SP_EL0. */ msr spsel, #1 + ret +ENDPROC(cpu_init) +create_page_tables: /* Rebuild the boot pagetable's first-level entries. The structure * is described in mm.c. * @@ -515,6 +549,10 @@ virtphys_clash: b fail 1: + ret +ENDPROC(create_page_tables) + +enable_mmu: PRINT("- Turning on paging -\r\n") /* @@ -524,16 +562,16 @@ virtphys_clash: tlbi alle2 /* Flush hypervisor TLBs */ dsb nsh - ldr x1, =paging /* Explicit vaddr, not RIP-relative */ mrs x0, SCTLR_EL2 orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MMU */ orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ dsb sy /* Flush PTE writes and finish reads */ msr SCTLR_EL2, x0 /* now paging is enabled */ isb /* Now, flush the icache */ - br x1 /* Get a proper vaddr into PC */ -paging: + ret +ENDPROC(enable_mmu) +setup_fixmap: /* Now we can install the fixmap and dtb mappings, since we * don't need the 1:1 map any more */ dsb sy @@ -575,7 +613,10 @@ paging: tlbi alle2 dsb sy /* Ensure completion of TLB flush */ isb + ret +ENDPROC(setup_fixmap) +launch: PRINT("- Ready -\r\n") /* The boot CPU should go straight into C now */ @@ -594,7 +635,6 @@ paging: dsb sy /* Ensure completion of TLB flush */ isb -launch: ldr x0, =init_data add x0, x0, #INITINFO_stack /* Find the boot-time stack */ ldr x0, [x0] @@ -609,6 +649,7 @@ launch: b start_xen /* and disappear into the land of C */ 1: b start_secondary /* (to the appropriate entry point) */ +ENDPROC(launch) /* Fail-stop */ fail: PRINT("- Boot failed -\r\n") -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs 2019-06-10 19:32 ` [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs Julien Grall @ 2019-06-26 1:00 ` Stefano Stabellini 2019-06-26 9:14 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:00 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > The boot code is currently quite difficult to go through because of the > lack of documentation and a number of indirection to avoid executing > some path in either the boot CPU or secondary CPUs. > > In an attempt to make the boot code easier to follow, each parts of the > boot are now in separate functions. Furthermore, the paths for the boot > CPU and secondary CPUs are now distincted and for now will call each > functions. > > Follow-ups will remove unecessary calls and do further improvement > (such as adding documentation and reshuffling). > > Note that the switch from using the ID mapping to the runtime mapping > is duplicated for each path. This is because in the future we will need > to stay longer in the ID mapping for the boot CPU. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 57 ++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 49 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 9142b4a774..ccd8a1b0a8 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -290,7 +290,19 @@ real_start_efi: > > mov x22, #0 /* x22 := is_secondary_cpu */ > > - b common_start > + bl check_cpu_mode > + bl zero_bss > + bl cpu_init > + bl create_page_tables > + bl enable_mmu > + > + /* We are still in the ID map. Jump to the runtime Virtual Address. */ > + ldr x0, =primary_switched > + br x0 > +primary_switched: > + bl setup_fixmap > + b launch > +ENDPROC(real_start) > > GLOBAL(init_secondary) > msr DAIFSet, 0xf /* Disable all interrupts */ > @@ -324,9 +336,21 @@ GLOBAL(init_secondary) > print_reg x24 > PRINT(" booting -\r\n") > #endif > - > -common_start: > - > + bl check_cpu_mode > + bl zero_bss > + bl cpu_init > + bl create_page_tables > + bl enable_mmu > + > + /* We are still in the ID map. Jump to the runtime Virtual Address. */ > + ldr x0, =secondary_switched > + br x0 > +secondary_switched: > + bl setup_fixmap > + b launch > +ENDPROC(init_secondary) > + > +check_cpu_mode: > PRINT("- Current EL ") > mrs x5, CurrentEL > print_reg x5 > @@ -343,7 +367,10 @@ common_start: > b fail > > el2: PRINT("- Xen starting at EL2 -\r\n") > + ret > +ENDPROC(check_cpu_mode) > > +zero_bss: > /* Zero BSS only when requested */ > cbnz x26, skip_bss > > @@ -356,6 +383,10 @@ el2: PRINT("- Xen starting at EL2 -\r\n") > b.lo 1b > > skip_bss: > + ret > +ENDPROC(zero_bss) > + > +cpu_init: > PRINT("- Setting up control registers -\r\n") > > /* Set up memory attribute type tables */ > @@ -390,7 +421,10 @@ skip_bss: > * are handled using the EL2 stack pointer, rather > * than SP_EL0. */ > msr spsel, #1 > + ret > +ENDPROC(cpu_init) > > +create_page_tables: > /* Rebuild the boot pagetable's first-level entries. The structure > * is described in mm.c. > * > @@ -515,6 +549,10 @@ virtphys_clash: > b fail > > 1: > + ret > +ENDPROC(create_page_tables) > + > +enable_mmu: > PRINT("- Turning on paging -\r\n") > > /* > @@ -524,16 +562,16 @@ virtphys_clash: > tlbi alle2 /* Flush hypervisor TLBs */ > dsb nsh > > - ldr x1, =paging /* Explicit vaddr, not RIP-relative */ > mrs x0, SCTLR_EL2 > orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MMU */ > orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ > dsb sy /* Flush PTE writes and finish reads */ > msr SCTLR_EL2, x0 /* now paging is enabled */ > isb /* Now, flush the icache */ > - br x1 /* Get a proper vaddr into PC */ > -paging: > + ret > +ENDPROC(enable_mmu) > > +setup_fixmap: > /* Now we can install the fixmap and dtb mappings, since we > * don't need the 1:1 map any more */ > dsb sy > @@ -575,7 +613,10 @@ paging: > tlbi alle2 > dsb sy /* Ensure completion of TLB flush */ > isb > + ret > +ENDPROC(setup_fixmap) > > +launch: > PRINT("- Ready -\r\n") > > /* The boot CPU should go straight into C now */ > @@ -594,7 +635,6 @@ paging: > dsb sy /* Ensure completion of TLB flush */ > isb > > -launch: Just below PRINT("- Ready -\r\n"), there is still a: cbz x22, launch moving the launch label up it looks like it will cause an infinite loop? Everything else looks good, and I like the reorg of the code. > ldr x0, =init_data > add x0, x0, #INITINFO_stack /* Find the boot-time stack */ > ldr x0, [x0] > @@ -609,6 +649,7 @@ launch: > b start_xen /* and disappear into the land of C */ > 1: > b start_secondary /* (to the appropriate entry point) */ > +ENDPROC(launch) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs 2019-06-26 1:00 ` Stefano Stabellini @ 2019-06-26 9:14 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 9:14 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 02:00, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> The boot code is currently quite difficult to go through because of the >> lack of documentation and a number of indirection to avoid executing >> some path in either the boot CPU or secondary CPUs. >> >> In an attempt to make the boot code easier to follow, each parts of the >> boot are now in separate functions. Furthermore, the paths for the boot >> CPU and secondary CPUs are now distincted and for now will call each I notice a few typo in my commit message: s/distincted/distinct/ >> functions. >> >> Follow-ups will remove unecessary calls and do further improvement s/unecessary/unnecessary/ >> +launch: >> PRINT("- Ready -\r\n") >> >> /* The boot CPU should go straight into C now */ >> @@ -594,7 +635,6 @@ paging: >> dsb sy /* Ensure completion of TLB flush */ >> isb >> >> -launch: > > Just below PRINT("- Ready -\r\n"), there is still a: > > cbz x22, launch > > moving the launch label up it looks like it will cause an infinite loop? Urgh. this line is dropped in a later patch, so the issue only would happen during bisection. I will update the code to avoid the infinite loop here. > > Everything else looks good, and I like the reorg of the code. Thank you! I will rework the arm32 code the same way then :). > > >> ldr x0, =init_data >> add x0, x0, #INITINFO_stack /* Find the boot-time stack */ >> ldr x0, [x0] >> @@ -609,6 +649,7 @@ launch: >> b start_xen /* and disappear into the land of C */ >> 1: >> b start_secondary /* (to the appropriate entry point) */ >> +ENDPROC(launch) > Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (5 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:00 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() Julien Grall ` (9 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko A branch in the success case can be avoided by inverting the branch condition. At the same time, remove a pointless comment as Xen can only run at EL2. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index ccd8a1b0a8..87fcd3be6c 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -350,6 +350,13 @@ secondary_switched: b launch ENDPROC(init_secondary) +/* + * Check if the CPU has been booted in Hypervisor mode. + * This function will never return when the CPU is booted in another mode + * than Hypervisor mode. + * + * Clobbers x0 - x5 + */ check_cpu_mode: PRINT("- Current EL ") mrs x5, CurrentEL @@ -359,15 +366,13 @@ check_cpu_mode: /* Are we in EL2 */ cmp x5, #PSR_MODE_EL2t ccmp x5, #PSR_MODE_EL2h, #0x4, ne - b.eq el2 /* Yes */ - + b.ne 1f /* No */ + ret +1: /* OK, we're boned. */ PRINT("- Xen must be entered in NS EL2 mode -\r\n") PRINT("- Please update the bootloader -\r\n") b fail - -el2: PRINT("- Xen starting at EL2 -\r\n") - ret ENDPROC(check_cpu_mode) zero_bss: -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() 2019-06-10 19:32 ` [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() Julien Grall @ 2019-06-26 1:00 ` Stefano Stabellini 0 siblings, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:00 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > A branch in the success case can be avoided by inverting the branch > condition. At the same time, remove a pointless comment as Xen can only > run at EL2. > > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/arm64/head.S | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index ccd8a1b0a8..87fcd3be6c 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -350,6 +350,13 @@ secondary_switched: > b launch > ENDPROC(init_secondary) > > +/* > + * Check if the CPU has been booted in Hypervisor mode. > + * This function will never return when the CPU is booted in another mode > + * than Hypervisor mode. > + * > + * Clobbers x0 - x5 > + */ > check_cpu_mode: > PRINT("- Current EL ") > mrs x5, CurrentEL > @@ -359,15 +366,13 @@ check_cpu_mode: > /* Are we in EL2 */ > cmp x5, #PSR_MODE_EL2t > ccmp x5, #PSR_MODE_EL2h, #0x4, ne > - b.eq el2 /* Yes */ > - > + b.ne 1f /* No */ > + ret > +1: > /* OK, we're boned. */ > PRINT("- Xen must be entered in NS EL2 mode -\r\n") > PRINT("- Please update the bootloader -\r\n") > b fail > - > -el2: PRINT("- Xen starting at EL2 -\r\n") > - ret > ENDPROC(check_cpu_mode) > > zero_bss: > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (6 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:01 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() Julien Grall ` (8 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko On secondary CPUs, zero_bss() will be a NOP because BSS only need to be zeroed once at boot. So the call in the secondary CPUs path can be removed. It also means that x26 does not need to set and is now only used by the boot CPU. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 87fcd3be6c..6aa3148192 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -71,7 +71,7 @@ * x23 - UART address * x24 - * x25 - identity map in place - * x26 - skip_zero_bss + * x26 - skip_zero_bss (boot cpu only) * x27 - * x28 - * x29 - @@ -313,8 +313,6 @@ GLOBAL(init_secondary) sub x20, x19, x0 /* x20 := phys-offset */ mov x22, #1 /* x22 := is_secondary_cpu */ - /* Boot CPU already zero BSS so skip it on secondary CPUs. */ - mov x26, #1 /* X26 := skip_zero_bss */ mrs x0, mpidr_el1 ldr x13, =(~MPIDR_HWID_MASK) @@ -337,7 +335,6 @@ GLOBAL(init_secondary) PRINT(" booting -\r\n") #endif bl check_cpu_mode - bl zero_bss bl cpu_init bl create_page_tables bl enable_mmu @@ -375,6 +372,14 @@ check_cpu_mode: b fail ENDPROC(check_cpu_mode) +/* + * Zero BSS + * + * Inputs: + * x26: Do we need to zero BSS? + * + * Clobbers x0 - x3 + */ zero_bss: /* Zero BSS only when requested */ cbnz x26, skip_bss -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() 2019-06-10 19:32 ` [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() Julien Grall @ 2019-06-26 1:01 ` Stefano Stabellini 2019-06-26 9:16 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:01 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > On secondary CPUs, zero_bss() will be a NOP because BSS only need to be > zeroed once at boot. So the call in the secondary CPUs path can be > removed. It also means that x26 does not need to set and is now only > used by the boot CPU. > > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 87fcd3be6c..6aa3148192 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -71,7 +71,7 @@ > * x23 - UART address > * x24 - > * x25 - identity map in place > - * x26 - skip_zero_bss > + * x26 - skip_zero_bss (boot cpu only) you could remove this, see below... > * x27 - > * x28 - > * x29 - > @@ -313,8 +313,6 @@ GLOBAL(init_secondary) > sub x20, x19, x0 /* x20 := phys-offset */ > > mov x22, #1 /* x22 := is_secondary_cpu */ > - /* Boot CPU already zero BSS so skip it on secondary CPUs. */ > - mov x26, #1 /* X26 := skip_zero_bss */ > > mrs x0, mpidr_el1 > ldr x13, =(~MPIDR_HWID_MASK) > @@ -337,7 +335,6 @@ GLOBAL(init_secondary) > PRINT(" booting -\r\n") > #endif > bl check_cpu_mode > - bl zero_bss > bl cpu_init > bl create_page_tables > bl enable_mmu > @@ -375,6 +372,14 @@ check_cpu_mode: > b fail > ENDPROC(check_cpu_mode) > > +/* > + * Zero BSS > + * > + * Inputs: > + * x26: Do we need to zero BSS? > + * > + * Clobbers x0 - x3 > + */ > zero_bss: > /* Zero BSS only when requested */ > cbnz x26, skip_bss In the commit message you wrote: "It also means that x26 does not need to set and is now only used by the boot CPU." I think this statement is correct, so you could also remove this "cbnz x26, skip_bss" and also the skip_bss label below. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() 2019-06-26 1:01 ` Stefano Stabellini @ 2019-06-26 9:16 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 9:16 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 02:01, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> On secondary CPUs, zero_bss() will be a NOP because BSS only need to be >> zeroed once at boot. So the call in the secondary CPUs path can be >> removed. It also means that x26 does not need to set and is now only >> used by the boot CPU. >> >> Lastly, document the behavior and the main registers usage within the >> function. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 87fcd3be6c..6aa3148192 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -71,7 +71,7 @@ >> * x23 - UART address >> * x24 - >> * x25 - identity map in place >> - * x26 - skip_zero_bss >> + * x26 - skip_zero_bss (boot cpu only) > > you could remove this, see below... > > >> * x27 - >> * x28 - >> * x29 - >> @@ -313,8 +313,6 @@ GLOBAL(init_secondary) >> sub x20, x19, x0 /* x20 := phys-offset */ >> >> mov x22, #1 /* x22 := is_secondary_cpu */ >> - /* Boot CPU already zero BSS so skip it on secondary CPUs. */ >> - mov x26, #1 /* X26 := skip_zero_bss */ >> >> mrs x0, mpidr_el1 >> ldr x13, =(~MPIDR_HWID_MASK) >> @@ -337,7 +335,6 @@ GLOBAL(init_secondary) >> PRINT(" booting -\r\n") >> #endif >> bl check_cpu_mode >> - bl zero_bss >> bl cpu_init >> bl create_page_tables >> bl enable_mmu >> @@ -375,6 +372,14 @@ check_cpu_mode: >> b fail >> ENDPROC(check_cpu_mode) >> >> +/* >> + * Zero BSS >> + * >> + * Inputs: >> + * x26: Do we need to zero BSS? >> + * >> + * Clobbers x0 - x3 >> + */ >> zero_bss: >> /* Zero BSS only when requested */ >> cbnz x26, skip_bss > > In the commit message you wrote: "It also means that x26 does not need > to set and is now only used by the boot CPU." I think this statement is > correct, so you could also remove this "cbnz x26, skip_bss" and also > the skip_bss label below. I meant x26 does not need to be set on the secondary CPUs. However, we still need to keep it for boot CPU as we don't want to zero BSS when booting via EFI. This is because the EFI stub will store information in BSS. Note that BSS was zeroed by EFI loader before hand. I will reword the commit message. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (7 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:01 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() Julien Grall ` (7 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Adjust the coding style used in the comments within cpu_init(). Take the opportunity to alter the early print to match the function name. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 6aa3148192..ee0024173e 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -396,19 +396,26 @@ skip_bss: ret ENDPROC(zero_bss) +/* + * Initialize the processor for turning the MMU on. + * + * Clobbers x0 - x4 + */ cpu_init: - PRINT("- Setting up control registers -\r\n") + PRINT("- Initialize CPU -\r\n") /* Set up memory attribute type tables */ ldr x0, =MAIRVAL msr mair_el2, x0 - /* Set up TCR_EL2: + /* + * Set up TCR_EL2: * PS -- Based on ID_AA64MMFR0_EL1.PARange * Top byte is used * PT walks use Inner-Shareable accesses, * PT walks are write-back, write-allocate in both cache levels, - * 48-bit virtual address space goes through this table. */ + * 48-bit virtual address space goes through this table. + */ ldr x0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(64-48)) /* ID_AA64MMFR0_EL1[3:0] (PARange) corresponds to TCR_EL2[18:16] (PS) */ mrs x1, ID_AA64MMFR0_EL1 @@ -427,9 +434,11 @@ cpu_init: ldr x0, =(HSCTLR_BASE) msr SCTLR_EL2, x0 - /* Ensure that any exceptions encountered at EL2 + /* + * Ensure that any exceptions encountered at EL2 * are handled using the EL2 stack pointer, rather - * than SP_EL0. */ + * than SP_EL0. + */ msr spsel, #1 ret ENDPROC(cpu_init) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() 2019-06-10 19:32 ` [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() Julien Grall @ 2019-06-26 1:01 ` Stefano Stabellini 2019-06-26 10:34 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:01 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > Adjust the coding style used in the comments within cpu_init(). Take the > opportunity to alter the early print to match the function name. > > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 6aa3148192..ee0024173e 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -396,19 +396,26 @@ skip_bss: > ret > ENDPROC(zero_bss) > > +/* > + * Initialize the processor for turning the MMU on. > + * > + * Clobbers x0 - x4 Shouldn't it be x0 - x3? The rest looks fine. > + */ > cpu_init: > - PRINT("- Setting up control registers -\r\n") > + PRINT("- Initialize CPU -\r\n") > > /* Set up memory attribute type tables */ > ldr x0, =MAIRVAL > msr mair_el2, x0 > > - /* Set up TCR_EL2: > + /* > + * Set up TCR_EL2: > * PS -- Based on ID_AA64MMFR0_EL1.PARange > * Top byte is used > * PT walks use Inner-Shareable accesses, > * PT walks are write-back, write-allocate in both cache levels, > - * 48-bit virtual address space goes through this table. */ > + * 48-bit virtual address space goes through this table. > + */ > ldr x0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(64-48)) > /* ID_AA64MMFR0_EL1[3:0] (PARange) corresponds to TCR_EL2[18:16] (PS) */ > mrs x1, ID_AA64MMFR0_EL1 > @@ -427,9 +434,11 @@ cpu_init: > ldr x0, =(HSCTLR_BASE) > msr SCTLR_EL2, x0 > > - /* Ensure that any exceptions encountered at EL2 > + /* > + * Ensure that any exceptions encountered at EL2 > * are handled using the EL2 stack pointer, rather > - * than SP_EL0. */ > + * than SP_EL0. > + */ > msr spsel, #1 > ret > ENDPROC(cpu_init) > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() 2019-06-26 1:01 ` Stefano Stabellini @ 2019-06-26 10:34 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 10:34 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 02:01, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> Adjust the coding style used in the comments within cpu_init(). Take the >> opportunity to alter the early print to match the function name. >> >> Lastly, document the behavior and the main registers usage within the >> function. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 6aa3148192..ee0024173e 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -396,19 +396,26 @@ skip_bss: >> ret >> ENDPROC(zero_bss) >> >> +/* >> + * Initialize the processor for turning the MMU on. >> + * >> + * Clobbers x0 - x4 > > Shouldn't it be x0 - x3? Yes it should be. I will update the comment. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (8 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() Julien Grall ` (6 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Adjust the coding style used in the comments within create_pages_tables() Lastly, document the behavior and the main registers usage within the function. Note that x25 is now only used within the function, so it does not need to be part of the common register. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index ee0024173e..7b92c1c8eb 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -70,7 +70,7 @@ * x22 - is_secondary_cpu * x23 - UART address * x24 - - * x25 - identity map in place + * x25 - * x26 - skip_zero_bss (boot cpu only) * x27 - * x28 - @@ -443,16 +443,27 @@ cpu_init: ret ENDPROC(cpu_init) +/* + * Rebuild the boot pagetable's first-level entries. The structure + * is described in mm.c. + * + * After the CPU enables paging it will add the fixmap mapping + * to these page tables, however this may clash with the 1:1 + * mapping. So each CPU must rebuild the page tables here with + * the 1:1 in place. + * + * Inputs: + * x19: paddr(start) + * x20: phys offset + * + * Clobbers x0 - x4, x25 + * + * Register usage within this function: + * x25: Identity map in place + */ create_page_tables: - /* Rebuild the boot pagetable's first-level entries. The structure - * is described in mm.c. - * - * After the CPU enables paging it will add the fixmap mapping - * to these page tables, however this may clash with the 1:1 - * mapping. So each CPU must rebuild the page tables here with - * the 1:1 in place. */ - - /* If Xen is loaded at exactly XEN_VIRT_START then we don't + /* + * If Xen is loaded at exactly XEN_VIRT_START then we don't * need an additional 1:1 mapping, the virtual mapping will * suffice. */ @@ -476,7 +487,8 @@ create_page_tables: cbz x1, 1f /* It's in slot 0, map in boot_first * or boot_second later on */ - /* Level zero does not support superpage mappings, so we have + /* + * Level zero does not support superpage mappings, so we have * to use an extra first level page in which we create a 1GB mapping. */ load_paddr x2, boot_first_id -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() 2019-06-10 19:32 ` [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() Julien Grall @ 2019-06-26 1:03 ` Stefano Stabellini 2019-06-26 11:20 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:03 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > Adjust the coding style used in the comments within create_pages_tables() > > Lastly, document the behavior and the main registers usage within the > function. Note that x25 is now only used within the function, so it does > not need to be part of the common register. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 34 +++++++++++++++++++++++----------- > 1 file changed, 23 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index ee0024173e..7b92c1c8eb 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -70,7 +70,7 @@ > * x22 - is_secondary_cpu > * x23 - UART address > * x24 - > - * x25 - identity map in place > + * x25 - > * x26 - skip_zero_bss (boot cpu only) > * x27 - > * x28 - > @@ -443,16 +443,27 @@ cpu_init: > ret > ENDPROC(cpu_init) > > +/* > + * Rebuild the boot pagetable's first-level entries. The structure > + * is described in mm.c. > + * > + * After the CPU enables paging it will add the fixmap mapping > + * to these page tables, however this may clash with the 1:1 > + * mapping. So each CPU must rebuild the page tables here with > + * the 1:1 in place. > + * > + * Inputs: > + * x19: paddr(start) > + * x20: phys offset Is x20 actually used? The rest looks fine. > + * Clobbers x0 - x4, x25 > + * > + * Register usage within this function: > + * x25: Identity map in place > + */ > create_page_tables: > - /* Rebuild the boot pagetable's first-level entries. The structure > - * is described in mm.c. > - * > - * After the CPU enables paging it will add the fixmap mapping > - * to these page tables, however this may clash with the 1:1 > - * mapping. So each CPU must rebuild the page tables here with > - * the 1:1 in place. */ > - > - /* If Xen is loaded at exactly XEN_VIRT_START then we don't > + /* > + * If Xen is loaded at exactly XEN_VIRT_START then we don't > * need an additional 1:1 mapping, the virtual mapping will > * suffice. > */ > @@ -476,7 +487,8 @@ create_page_tables: > cbz x1, 1f /* It's in slot 0, map in boot_first > * or boot_second later on */ > > - /* Level zero does not support superpage mappings, so we have > + /* > + * Level zero does not support superpage mappings, so we have > * to use an extra first level page in which we create a 1GB mapping. > */ > load_paddr x2, boot_first_id > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() 2019-06-26 1:03 ` Stefano Stabellini @ 2019-06-26 11:20 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 11:20 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 02:03, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> Adjust the coding style used in the comments within create_pages_tables() >> >> Lastly, document the behavior and the main registers usage within the >> function. Note that x25 is now only used within the function, so it does >> not need to be part of the common register. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 34 +++++++++++++++++++++++----------- >> 1 file changed, 23 insertions(+), 11 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index ee0024173e..7b92c1c8eb 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -70,7 +70,7 @@ >> * x22 - is_secondary_cpu >> * x23 - UART address >> * x24 - >> - * x25 - identity map in place >> + * x25 - >> * x26 - skip_zero_bss (boot cpu only) >> * x27 - >> * x28 - >> @@ -443,16 +443,27 @@ cpu_init: >> ret >> ENDPROC(cpu_init) >> >> +/* >> + * Rebuild the boot pagetable's first-level entries. The structure >> + * is described in mm.c. >> + * >> + * After the CPU enables paging it will add the fixmap mapping >> + * to these page tables, however this may clash with the 1:1 >> + * mapping. So each CPU must rebuild the page tables here with >> + * the 1:1 in place. >> + * >> + * Inputs: >> + * x19: paddr(start) >> + * x20: phys offset > > Is x20 actually used? Yes via the macro load_paddr. > > The rest looks fine. > > >> + * Clobbers x0 - x4, x25 >> + * >> + * Register usage within this function: >> + * x25: Identity map in place >> + */ >> create_page_tables: >> - /* Rebuild the boot pagetable's first-level entries. The structure >> - * is described in mm.c. >> - * >> - * After the CPU enables paging it will add the fixmap mapping >> - * to these page tables, however this may clash with the 1:1 >> - * mapping. So each CPU must rebuild the page tables here with >> - * the 1:1 in place. */ >> - >> - /* If Xen is loaded at exactly XEN_VIRT_START then we don't >> + /* >> + * If Xen is loaded at exactly XEN_VIRT_START then we don't >> * need an additional 1:1 mapping, the virtual mapping will >> * suffice. >> */ >> @@ -476,7 +487,8 @@ create_page_tables: >> cbz x1, 1f /* It's in slot 0, map in boot_first >> * or boot_second later on */ >> >> - /* Level zero does not support superpage mappings, so we have >> + /* >> + * Level zero does not support superpage mappings, so we have >> * to use an extra first level page in which we create a 1GB mapping. >> */ >> load_paddr x2, boot_first_id >> -- >> 2.11.0 >> Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (9 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path Julien Grall ` (5 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Document the behavior and the main registers usage within enable_mmu(). Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 7b92c1c8eb..d673f7c0d8 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -583,6 +583,13 @@ virtphys_clash: ret ENDPROC(create_page_tables) +/* + * Turn on the Data Cache and the MMU. The function will return on the ID + * mapping. In other word, the caller is responsible to switch to the runtime + * mapping. + * + * Clobbers x0 - x1 + */ enable_mmu: PRINT("- Turning on paging -\r\n") -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() 2019-06-10 19:32 ` [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() Julien Grall @ 2019-06-26 1:03 ` Stefano Stabellini 2019-06-26 11:23 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:03 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > Document the behavior and the main registers usage within enable_mmu(). > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 7b92c1c8eb..d673f7c0d8 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -583,6 +583,13 @@ virtphys_clash: > ret > ENDPROC(create_page_tables) > > +/* > + * Turn on the Data Cache and the MMU. The function will return on the ID > + * mapping. In other word, the caller is responsible to switch to the runtime > + * mapping. > + * > + * Clobbers x0 - x1 > + */ as it calls PRINT, shouldn't it be x0 - x3? > enable_mmu: > PRINT("- Turning on paging -\r\n") > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() 2019-06-26 1:03 ` Stefano Stabellini @ 2019-06-26 11:23 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 11:23 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 02:03, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> Document the behavior and the main registers usage within enable_mmu(). >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 7b92c1c8eb..d673f7c0d8 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -583,6 +583,13 @@ virtphys_clash: >> ret >> ENDPROC(create_page_tables) >> >> +/* >> + * Turn on the Data Cache and the MMU. The function will return on the ID >> + * mapping. In other word, the caller is responsible to switch to the runtime >> + * mapping. >> + * >> + * Clobbers x0 - x1 >> + */ > > as it calls PRINT, shouldn't it be x0 - x3? You are right. I will update the comment. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (10 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs Julien Grall ` (4 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko The assembly switch to the runtime PT is only necessary for the secondary CPUs. So move the code in the secondary CPUs path. While this is definitely not compliant with the Arm Arm as we are switching between two differents set of page-tables without turning off the MMU. Turning off the MMU is impossible here as the ID map may clash with other mappings in the runtime page-tables. This will require more rework to avoid the problem. So for now add a TODO in the code. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index d673f7c0d8..6be4af7579 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -344,6 +344,23 @@ GLOBAL(init_secondary) br x0 secondary_switched: bl setup_fixmap + + /* + * Non-boot CPUs need to move on to the proper pagetables, which were + * setup in init_secondary_pagetables. + * + * XXX: This is not compliant with the Arm Arm. + */ + ldr x4, =init_ttbr /* VA of TTBR0_EL2 stashed by CPU 0 */ + ldr x4, [x4] /* Actual value */ + dsb sy + msr TTBR0_EL2, x4 + dsb sy + isb + tlbi alle2 + dsb sy /* Ensure completion of TLB flush */ + isb + b launch ENDPROC(init_secondary) @@ -657,22 +674,6 @@ ENDPROC(setup_fixmap) launch: PRINT("- Ready -\r\n") - /* The boot CPU should go straight into C now */ - cbz x22, launch - - /* Non-boot CPUs need to move on to the proper pagetables, which were - * setup in init_secondary_pagetables. */ - - ldr x4, =init_ttbr /* VA of TTBR0_EL2 stashed by CPU 0 */ - ldr x4, [x4] /* Actual value */ - dsb sy - msr TTBR0_EL2, x4 - dsb sy - isb - tlbi alle2 - dsb sy /* Ensure completion of TLB flush */ - isb - ldr x0, =init_data add x0, x0, #INITINFO_stack /* Find the boot-time stack */ ldr x0, [x0] -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path 2019-06-10 19:32 ` [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path Julien Grall @ 2019-06-26 1:03 ` Stefano Stabellini 0 siblings, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 1:03 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > The assembly switch to the runtime PT is only necessary for the > secondary CPUs. So move the code in the secondary CPUs path. > > While this is definitely not compliant with the Arm Arm as we are > switching between two differents set of page-tables without turning off > the MMU. Turning off the MMU is impossible here as the ID map may clash > with other mappings in the runtime page-tables. This will require more > rework to avoid the problem. So for now add a TODO in the code. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/arm64/head.S | 33 +++++++++++++++++---------------- > 1 file changed, 17 insertions(+), 16 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index d673f7c0d8..6be4af7579 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -344,6 +344,23 @@ GLOBAL(init_secondary) > br x0 > secondary_switched: > bl setup_fixmap > + > + /* > + * Non-boot CPUs need to move on to the proper pagetables, which were > + * setup in init_secondary_pagetables. > + * > + * XXX: This is not compliant with the Arm Arm. > + */ > + ldr x4, =init_ttbr /* VA of TTBR0_EL2 stashed by CPU 0 */ > + ldr x4, [x4] /* Actual value */ > + dsb sy > + msr TTBR0_EL2, x4 > + dsb sy > + isb > + tlbi alle2 > + dsb sy /* Ensure completion of TLB flush */ > + isb > + > b launch > ENDPROC(init_secondary) > > @@ -657,22 +674,6 @@ ENDPROC(setup_fixmap) > launch: > PRINT("- Ready -\r\n") > > - /* The boot CPU should go straight into C now */ > - cbz x22, launch > - > - /* Non-boot CPUs need to move on to the proper pagetables, which were > - * setup in init_secondary_pagetables. */ > - > - ldr x4, =init_ttbr /* VA of TTBR0_EL2 stashed by CPU 0 */ > - ldr x4, [x4] /* Actual value */ > - dsb sy > - msr TTBR0_EL2, x4 > - dsb sy > - isb > - tlbi alle2 > - dsb sy /* Ensure completion of TLB flush */ > - isb > - > ldr x0, =init_data > add x0, x0, #INITINFO_stack /* Find the boot-time stack */ > ldr x0, [x0] > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (11 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 18:51 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used Julien Grall ` (3 subsequent siblings) 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko setup_fixmap() will setup the fixmap in the boot page tables in order to use earlyprintk and also update the register x23 holding the address to the UART. However, secondary CPUs are switching to the runtime page tables before using the earlyprintk again. So settting up the fixmap in the boot pages tables is pointless. This means most of setup_fixmap() is not necessary for the secondary CPUs. The update of UART address is now moved out of setup_fixmap() and duplicated in the CPU boot and secondary CPUs boot. Additionally, the call to setup_fixmap() is removed from secondary CPUs boot. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 6be4af7579..192af3e8a2 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -301,6 +301,10 @@ real_start_efi: br x0 primary_switched: bl setup_fixmap +#ifdef CONFIG_EARLY_PRINTK + /* Use a virtual address to access the UART. */ + ldr x23, =EARLY_UART_VIRTUAL_ADDRESS +#endif b launch ENDPROC(real_start) @@ -343,8 +347,6 @@ GLOBAL(init_secondary) ldr x0, =secondary_switched br x0 secondary_switched: - bl setup_fixmap - /* * Non-boot CPUs need to move on to the proper pagetables, which were * setup in init_secondary_pagetables. @@ -361,6 +363,10 @@ secondary_switched: dsb sy /* Ensure completion of TLB flush */ isb +#ifdef CONFIG_EARLY_PRINTK + /* Use a virtual address to access the UART. */ + ldr x23, =EARLY_UART_VIRTUAL_ADDRESS +#endif b launch ENDPROC(init_secondary) @@ -631,10 +637,6 @@ setup_fixmap: * don't need the 1:1 map any more */ dsb sy #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ - /* Non-boot CPUs don't need to rebuild the fixmap itself, just - * the mapping from boot_second to xen_fixmap */ - cbnz x22, 1f - /* Add UART to the fixmap table */ ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ lsr x2, x23, #THIRD_SHIFT @@ -642,7 +644,6 @@ setup_fixmap: mov x3, #PT_DEV_L3 orr x2, x2, x3 /* x2 := 4K dev map including UART */ str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ -1: /* Map fixmap into boot_second */ ldr x4, =boot_second /* x4 := vaddr (boot_second) */ @@ -652,9 +653,6 @@ setup_fixmap: ldr x1, =FIXMAP_ADDR(0) lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ str x2, [x4, x1] /* Map it in the fixmap's slot */ - - /* Use a virtual address to access the UART. */ - ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif /* -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs 2019-06-10 19:32 ` [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs Julien Grall @ 2019-06-26 18:51 ` Stefano Stabellini 2019-06-26 19:26 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 18:51 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > setup_fixmap() will setup the fixmap in the boot page tables in order to > use earlyprintk and also update the register x23 holding the address to > the UART. > > However, secondary CPUs are switching to the runtime page tables before > using the earlyprintk again. So settting up the fixmap in the boot pages > tables is pointless. Typo: settting Also, you could add that it is "impossible" to find ourselves in the position of needing earlyprintk for secondary CPUs before the runtime page tables are up, because it is done right after in the boot sequence. Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > This means most of setup_fixmap() is not necessary for the secondary > CPUs. The update of UART address is now moved out of setup_fixmap() and > duplicated in the CPU boot and secondary CPUs boot. Additionally, the > call to setup_fixmap() is removed from secondary CPUs boot. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 6be4af7579..192af3e8a2 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -301,6 +301,10 @@ real_start_efi: > br x0 > primary_switched: > bl setup_fixmap > +#ifdef CONFIG_EARLY_PRINTK > + /* Use a virtual address to access the UART. */ > + ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > +#endif > b launch > ENDPROC(real_start) > > @@ -343,8 +347,6 @@ GLOBAL(init_secondary) > ldr x0, =secondary_switched > br x0 > secondary_switched: > - bl setup_fixmap > - > /* > * Non-boot CPUs need to move on to the proper pagetables, which were > * setup in init_secondary_pagetables. > @@ -361,6 +363,10 @@ secondary_switched: > dsb sy /* Ensure completion of TLB flush */ > isb > > +#ifdef CONFIG_EARLY_PRINTK > + /* Use a virtual address to access the UART. */ > + ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > +#endif > b launch > ENDPROC(init_secondary) > > @@ -631,10 +637,6 @@ setup_fixmap: > * don't need the 1:1 map any more */ > dsb sy > #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ > - /* Non-boot CPUs don't need to rebuild the fixmap itself, just > - * the mapping from boot_second to xen_fixmap */ > - cbnz x22, 1f > - > /* Add UART to the fixmap table */ > ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > lsr x2, x23, #THIRD_SHIFT > @@ -642,7 +644,6 @@ setup_fixmap: > mov x3, #PT_DEV_L3 > orr x2, x2, x3 /* x2 := 4K dev map including UART */ > str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ > -1: > > /* Map fixmap into boot_second */ > ldr x4, =boot_second /* x4 := vaddr (boot_second) */ > @@ -652,9 +653,6 @@ setup_fixmap: > ldr x1, =FIXMAP_ADDR(0) > lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ > str x2, [x4, x1] /* Map it in the fixmap's slot */ > - > - /* Use a virtual address to access the UART. */ > - ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > #endif > > /* > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs 2019-06-26 18:51 ` Stefano Stabellini @ 2019-06-26 19:26 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 19:26 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi, On 6/26/19 7:51 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> setup_fixmap() will setup the fixmap in the boot page tables in order to >> use earlyprintk and also update the register x23 holding the address to >> the UART. >> >> However, secondary CPUs are switching to the runtime page tables before >> using the earlyprintk again. So settting up the fixmap in the boot pages >> tables is pointless. > > Typo: settting ok. > > Also, you could add that it is "impossible" to find ourselves in the > position of needing earlyprintk for secondary CPUs before the runtime > page tables are up, because it is done right after in the boot sequence. It is kind of implied by the comment and the code below. But I can clearly state it. > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (12 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 20:25 ` Stefano Stabellini 2019-06-27 18:55 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() Julien Grall ` (2 subsequent siblings) 16 siblings, 2 replies; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko The ID map may clash with other parts of the Xen virtual memory layout. At the moment, Xen is handling the clash by only creating a mapping to the runtime virtual address before enabling the MMU. The rest of the mappings (such as the fixmap) will be mapped after the MMU is enabled. However, the code doing the mapping is not safe as it replace mapping without using the Break-Before-Make sequence. As the ID map can be anywhere in the memory, it is easier to remove all the entries added as soon as the ID map is not used rather than adding the Break-Before-Make sequence everywhere. It is difficult to track where exactly the ID map was created without a full rework of create_page_tables(). Instead, introduce a new function remove_id_map() will look where is the top-level entry for the ID map and remove it. The new function is only called for the boot CPU. Secondary CPUs will switch directly to the runtime page-tables so there are no need to remove the ID mapping. Note that this still doesn't make the Secondary CPUs path safe but it is not making it worst. --- Note that the comment refers to the patch "xen/arm: tlbflush: Rework TLB helpers" under review (see [1]). Furthermore, it is very likely we will need to re-introduce the ID map to cater secondary CPUs boot and suspend/resume. For now, the attempt is to make boot CPU path fully Arm Arm compliant. [1] https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01134.html --- xen/arch/arm/arm64/head.S | 86 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 15 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 192af3e8a2..96e85f8834 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -300,6 +300,13 @@ real_start_efi: ldr x0, =primary_switched br x0 primary_switched: + /* + * The ID 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. + */ + bl remove_id_map bl setup_fixmap #ifdef CONFIG_EARLY_PRINTK /* Use a virtual address to access the UART. */ @@ -632,10 +639,68 @@ enable_mmu: ret ENDPROC(enable_mmu) +/* + * Remove the ID map for the page-tables. It is not easy to keep track + * where the ID map was mapped, so we will look for the top-level entry + * exclusive to the ID Map and remove it. + * + * Inputs: + * x19: paddr(start) + * + * Clobbers x0 - x1 + */ +remove_id_map: + /* + * Find the zeroeth slot used. Remove the entry from zeroeth + * table if the slot is not 0. For slot 0, the ID map was either + * done in first or second table. + */ + lsr x1, x19, #ZEROETH_SHIFT /* x1 := zeroeth slot */ + cbz x1, 1f + /* It is not in slot 0, remove the entry */ + ldr x0, =boot_pgtable /* x0 := root table */ + str xzr, [x0, x1, lsl #3] + b id_map_removed + +1: + /* + * Find the first slot used. Remove the entry for the first + * table if the slot is not 0. For slot 0, the ID map was done + * in the second table. + */ + lsr x1, x19, #FIRST_SHIFT + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ + cbz x1, 1f + /* It is not in slot 0, remove the entry */ + ldr x0, =boot_first /* x0 := first table */ + str xzr, [x0, x1, lsl #3] + b id_map_removed + +1: + /* + * Find the second slot used. Remove the entry for the first + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). + * For slot 1, it means the ID map was not created. + */ + lsr x1, x19, #SECOND_SHIFT + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ + cmp x1, #1 + beq id_map_removed + /* It is not in slot 1, remove the entry */ + ldr x0, =boot_second /* x0 := second table */ + str xzr, [x0, x1, lsl #3] + +id_map_removed: + /* See asm-arm/arm64/flushtlb.h for the explanation of the sequence. */ + dsb nshst + tlbi alle2 + dsb nsh + isb + + ret +ENDPROC(remove_id_map) + setup_fixmap: - /* Now we can install the fixmap and dtb mappings, since we - * don't need the 1:1 map any more */ - dsb sy #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ /* Add UART to the fixmap table */ ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ @@ -653,19 +718,10 @@ setup_fixmap: ldr x1, =FIXMAP_ADDR(0) lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ str x2, [x4, x1] /* Map it in the fixmap's slot */ -#endif - /* - * Flush the TLB in case the 1:1 mapping happens to clash with - * the virtual addresses used by the fixmap or DTB. - */ - dsb sy /* Ensure any page table updates made above - * have occurred. */ - - isb - tlbi alle2 - dsb sy /* Ensure completion of TLB flush */ - isb + /* Ensure any page table updates made above have occurred */ + dsb nshst +#endif ret ENDPROC(setup_fixmap) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-10 19:32 ` [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used Julien Grall @ 2019-06-26 20:25 ` Stefano Stabellini 2019-06-26 20:39 ` Julien Grall 2019-06-27 18:55 ` Stefano Stabellini 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 20:25 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > The ID map may clash with other parts of the Xen virtual memory layout. > At the moment, Xen is handling the clash by only creating a mapping to > the runtime virtual address before enabling the MMU. > > The rest of the mappings (such as the fixmap) will be mapped after the > MMU is enabled. However, the code doing the mapping is not safe as it > replace mapping without using the Break-Before-Make sequence. > > As the ID map can be anywhere in the memory, it is easier to remove all > the entries added as soon as the ID map is not used rather than adding > the Break-Before-Make sequence everywhere. I think it is a good idea, but I would ask you to mention 1:1 map instead of "ID map" in comments and commit messages because that is the wording we used in all comments so far (see the one in create_page_tables and mm.c). It is easier to grep and refer to if we use the same nomenclature. Note that I don't care about which nomenclature we decide to use, I am only asking for consistency. Otherwise, it would also work if you say it both way at least once: The ID map (1:1 map) may clash [...] > It is difficult to track where exactly the ID map was created without a > full rework of create_page_tables(). Instead, introduce a new function > remove_id_map() will look where is the top-level entry for the ID map > and remove it. Do you think it would be worth simplifying this code below by preserving where/how the ID map was created? We could repurpose x25 for that, carrying for instance the address of the ID map section slot or a code number to specify which case we are dealing with. We should be able to turn remove_id_map into only ~5 lines? > The new function is only called for the boot CPU. Secondary CPUs will > switch directly to the runtime page-tables so there are no need to > remove the ID mapping. Note that this still doesn't make the Secondary > CPUs path safe but it is not making it worst. > > --- > Note that the comment refers to the patch "xen/arm: tlbflush: Rework > TLB helpers" under review (see [1]). > > Furthermore, it is very likely we will need to re-introduce the ID > map to cater secondary CPUs boot and suspend/resume. For now, the > attempt is to make boot CPU path fully Arm Arm compliant. > > [1] https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01134.html > --- > xen/arch/arm/arm64/head.S | 86 ++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 71 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 192af3e8a2..96e85f8834 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -300,6 +300,13 @@ real_start_efi: > ldr x0, =primary_switched > br x0 > primary_switched: > + /* > + * The ID 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. > + */ > + bl remove_id_map > bl setup_fixmap > #ifdef CONFIG_EARLY_PRINTK > /* Use a virtual address to access the UART. */ > @@ -632,10 +639,68 @@ enable_mmu: > ret > ENDPROC(enable_mmu) > > +/* > + * Remove the ID map for the page-tables. It is not easy to keep track > + * where the ID map was mapped, so we will look for the top-level entry > + * exclusive to the ID Map and remove it. > + * > + * Inputs: > + * x19: paddr(start) > + * > + * Clobbers x0 - x1 > + */ > +remove_id_map: > + /* > + * Find the zeroeth slot used. Remove the entry from zeroeth > + * table if the slot is not 0. For slot 0, the ID map was either > + * done in first or second table. > + */ > + lsr x1, x19, #ZEROETH_SHIFT /* x1 := zeroeth slot */ > + cbz x1, 1f > + /* It is not in slot 0, remove the entry */ > + ldr x0, =boot_pgtable /* x0 := root table */ > + str xzr, [x0, x1, lsl #3] > + b id_map_removed > + > +1: > + /* > + * Find the first slot used. Remove the entry for the first > + * table if the slot is not 0. For slot 0, the ID map was done > + * in the second table. > + */ > + lsr x1, x19, #FIRST_SHIFT > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > + cbz x1, 1f > + /* It is not in slot 0, remove the entry */ > + ldr x0, =boot_first /* x0 := first table */ > + str xzr, [x0, x1, lsl #3] > + b id_map_removed > + > +1: > + /* > + * Find the second slot used. Remove the entry for the first > + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). > + * For slot 1, it means the ID map was not created. > + */ > + lsr x1, x19, #SECOND_SHIFT > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > + cmp x1, #1 > + beq id_map_removed > + /* It is not in slot 1, remove the entry */ > + ldr x0, =boot_second /* x0 := second table */ > + str xzr, [x0, x1, lsl #3] > + > +id_map_removed: > + /* See asm-arm/arm64/flushtlb.h for the explanation of the sequence. */ > + dsb nshst > + tlbi alle2 > + dsb nsh > + isb > + > + ret > +ENDPROC(remove_id_map) > + > setup_fixmap: > - /* Now we can install the fixmap and dtb mappings, since we > - * don't need the 1:1 map any more */ > - dsb sy > #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ > /* Add UART to the fixmap table */ > ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > @@ -653,19 +718,10 @@ setup_fixmap: > ldr x1, =FIXMAP_ADDR(0) > lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ > str x2, [x4, x1] /* Map it in the fixmap's slot */ > -#endif > > - /* > - * Flush the TLB in case the 1:1 mapping happens to clash with > - * the virtual addresses used by the fixmap or DTB. > - */ > - dsb sy /* Ensure any page table updates made above > - * have occurred. */ > - > - isb > - tlbi alle2 > - dsb sy /* Ensure completion of TLB flush */ > - isb > + /* Ensure any page table updates made above have occurred */ > + dsb nshst > +#endif > ret > ENDPROC(setup_fixmap) > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-26 20:25 ` Stefano Stabellini @ 2019-06-26 20:39 ` Julien Grall 2019-06-26 20:44 ` Andrew Cooper 2019-06-28 0:36 ` Stefano Stabellini 0 siblings, 2 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 20:39 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/26/19 9:25 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> The ID map may clash with other parts of the Xen virtual memory layout. >> At the moment, Xen is handling the clash by only creating a mapping to >> the runtime virtual address before enabling the MMU. >> >> The rest of the mappings (such as the fixmap) will be mapped after the >> MMU is enabled. However, the code doing the mapping is not safe as it >> replace mapping without using the Break-Before-Make sequence. >> >> As the ID map can be anywhere in the memory, it is easier to remove all >> the entries added as soon as the ID map is not used rather than adding >> the Break-Before-Make sequence everywhere. > > I think it is a good idea, but I would ask you to mention 1:1 map > instead of "ID map" in comments and commit messages because that is the > wording we used in all comments so far (see the one in > create_page_tables and mm.c). It is easier to grep and refer to if we > use the same nomenclature. Note that I don't care about which > nomenclature we decide to use, I am only asking for consistency. > Otherwise, it would also work if you say it both way at least once: > > The ID map (1:1 map) may clash [...] I would rather drop the wording 1:1 as this is confusing. It is also not trivial to find anything on google typing "1:1". > > >> It is difficult to track where exactly the ID map was created without a >> full rework of create_page_tables(). Instead, introduce a new function >> remove_id_map() will look where is the top-level entry for the ID map >> and remove it. > > Do you think it would be worth simplifying this code below by preserving > where/how the ID map was created? We could repurpose x25 for that, > carrying for instance the address of the ID map section slot or a code > number to specify which case we are dealing with. We should be able to > turn remove_id_map into only ~5 lines? I thought about it but the current implementation of create_map_tables() is quite awful to read. So the less I touch this function, the better I feel :). I have some rework for the create_page_tables() which simplify it a lot. Yet, I am not entirely sure it is worth to spend time trying to simplify remove_id_map. This is unlikely to make the boot significantly faster and I don't expect the function to survive more than a release as the ID map as to be kept in place (for secondary boot and suspend/resume). The only reason it is removed now is because it clashes with other mapping we may do. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-26 20:39 ` Julien Grall @ 2019-06-26 20:44 ` Andrew Cooper 2019-06-28 0:36 ` Stefano Stabellini 1 sibling, 0 replies; 70+ messages in thread From: Andrew Cooper @ 2019-06-26 20:44 UTC (permalink / raw) To: xen-devel On 26/06/2019 21:39, Julien Grall wrote: > On 6/26/19 9:25 PM, Stefano Stabellini wrote: >> On Mon, 10 Jun 2019, Julien Grall wrote: >>> The ID map may clash with other parts of the Xen virtual memory layout. >>> At the moment, Xen is handling the clash by only creating a mapping to >>> the runtime virtual address before enabling the MMU. >>> >>> The rest of the mappings (such as the fixmap) will be mapped after the >>> MMU is enabled. However, the code doing the mapping is not safe as it >>> replace mapping without using the Break-Before-Make sequence. >>> >>> As the ID map can be anywhere in the memory, it is easier to remove all >>> the entries added as soon as the ID map is not used rather than adding >>> the Break-Before-Make sequence everywhere. >> >> I think it is a good idea, but I would ask you to mention 1:1 map >> instead of "ID map" in comments and commit messages because that is the >> wording we used in all comments so far (see the one in >> create_page_tables and mm.c). It is easier to grep and refer to if we >> use the same nomenclature. Note that I don't care about which >> nomenclature we decide to use, I am only asking for consistency. >> Otherwise, it would also work if you say it both way at least once: >> >> The ID map (1:1 map) may clash [...] > > I would rather drop the wording 1:1 as this is confusing. It is also > not trivial to find anything on google typing "1:1". "one-to-one mapping", or "identity map" are both common terminology. 1:1 is a common representation for the former, whereas ID is not a abbreviation of "Identity". If you don't want to use 1:1, then you need to say "The identity map" to retain clarity. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-26 20:39 ` Julien Grall 2019-06-26 20:44 ` Andrew Cooper @ 2019-06-28 0:36 ` Stefano Stabellini 1 sibling, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-28 0:36 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Wed, 26 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 6/26/19 9:25 PM, Stefano Stabellini wrote: > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > The ID map may clash with other parts of the Xen virtual memory layout. > > > At the moment, Xen is handling the clash by only creating a mapping to > > > the runtime virtual address before enabling the MMU. > > > > > > The rest of the mappings (such as the fixmap) will be mapped after the > > > MMU is enabled. However, the code doing the mapping is not safe as it > > > replace mapping without using the Break-Before-Make sequence. > > > > > > As the ID map can be anywhere in the memory, it is easier to remove all > > > the entries added as soon as the ID map is not used rather than adding > > > the Break-Before-Make sequence everywhere. > > > > I think it is a good idea, but I would ask you to mention 1:1 map > > instead of "ID map" in comments and commit messages because that is the > > wording we used in all comments so far (see the one in > > create_page_tables and mm.c). It is easier to grep and refer to if we > > use the same nomenclature. Note that I don't care about which > > nomenclature we decide to use, I am only asking for consistency. > > Otherwise, it would also work if you say it both way at least once: > > > > The ID map (1:1 map) may clash [...] > > I would rather drop the wording 1:1 as this is confusing. It is also not > trivial to find anything on google typing "1:1". That's fine too _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-10 19:32 ` [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used Julien Grall 2019-06-26 20:25 ` Stefano Stabellini @ 2019-06-27 18:55 ` Stefano Stabellini 2019-06-27 19:30 ` Julien Grall 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-27 18:55 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > The ID map may clash with other parts of the Xen virtual memory layout. > At the moment, Xen is handling the clash by only creating a mapping to > the runtime virtual address before enabling the MMU. > > The rest of the mappings (such as the fixmap) will be mapped after the > MMU is enabled. However, the code doing the mapping is not safe as it > replace mapping without using the Break-Before-Make sequence. > > As the ID map can be anywhere in the memory, it is easier to remove all > the entries added as soon as the ID map is not used rather than adding > the Break-Before-Make sequence everywhere. > > It is difficult to track where exactly the ID map was created without a > full rework of create_page_tables(). Instead, introduce a new function > remove_id_map() will look where is the top-level entry for the ID map > and remove it. > > The new function is only called for the boot CPU. Secondary CPUs will > switch directly to the runtime page-tables so there are no need to > remove the ID mapping. Note that this still doesn't make the Secondary > CPUs path safe but it is not making it worst. > > --- > Note that the comment refers to the patch "xen/arm: tlbflush: Rework > TLB helpers" under review (see [1]). > > Furthermore, it is very likely we will need to re-introduce the ID > map to cater secondary CPUs boot and suspend/resume. For now, the > attempt is to make boot CPU path fully Arm Arm compliant. > > [1] https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01134.html > --- > xen/arch/arm/arm64/head.S | 86 ++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 71 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 192af3e8a2..96e85f8834 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -300,6 +300,13 @@ real_start_efi: > ldr x0, =primary_switched > br x0 > primary_switched: > + /* > + * The ID 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. > + */ > + bl remove_id_map > bl setup_fixmap > #ifdef CONFIG_EARLY_PRINTK > /* Use a virtual address to access the UART. */ > @@ -632,10 +639,68 @@ enable_mmu: > ret > ENDPROC(enable_mmu) > > +/* > + * Remove the ID map for the page-tables. It is not easy to keep track > + * where the ID map was mapped, so we will look for the top-level entry > + * exclusive to the ID Map and remove it. > + * > + * Inputs: > + * x19: paddr(start) > + * > + * Clobbers x0 - x1 > + */ > +remove_id_map: > + /* > + * Find the zeroeth slot used. Remove the entry from zeroeth > + * table if the slot is not 0. For slot 0, the ID map was either > + * done in first or second table. > + */ > + lsr x1, x19, #ZEROETH_SHIFT /* x1 := zeroeth slot */ > + cbz x1, 1f > + /* It is not in slot 0, remove the entry */ > + ldr x0, =boot_pgtable /* x0 := root table */ > + str xzr, [x0, x1, lsl #3] > + b id_map_removed > + > +1: > + /* > + * Find the first slot used. Remove the entry for the first > + * table if the slot is not 0. For slot 0, the ID map was done > + * in the second table. > + */ > + lsr x1, x19, #FIRST_SHIFT > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > + cbz x1, 1f > + /* It is not in slot 0, remove the entry */ > + ldr x0, =boot_first /* x0 := first table */ > + str xzr, [x0, x1, lsl #3] > + b id_map_removed > + > +1: > + /* > + * Find the second slot used. Remove the entry for the first > + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). > + * For slot 1, it means the ID map was not created. > + */ > + lsr x1, x19, #SECOND_SHIFT > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > + cmp x1, #1 > + beq id_map_removed > + /* It is not in slot 1, remove the entry */ > + ldr x0, =boot_second /* x0 := second table */ > + str xzr, [x0, x1, lsl #3] Wouldn't it be a bit more reliable if we checked whether the slot in question for x19 (whether zero, first, second) is a pagetable pointer or section map, then zero it if it is a section map, otherwise go down one level? If we did it this way it would be independent from the way create_page_tables is written. With the current code, we are somewhat reliant on the behavior of create_page_tables, because we rely on the position of the slot for the ID map? Where the assumption for instance is that at level one, if the slot is zero, then we need to go down a level, etc. Instead, if we checked if the slot is a section map, we could remove it immediately, if it is a pagetable pointer, we proceed. The code should be similar in complexity and LOC, but it would be more robust. Something like the following, in pseudo-uncompiled assembly: lsr x1, x19, #FIRST_SHIFT ldr x0, =boot_first /* x0 := first table */ ldr x2, [x0, x1, lsl #3] # check x2 against #PT_MEM cbz x2, 1f str xzr, [x0, x1, lsl #3] b id_map_removed > +id_map_removed: > + /* See asm-arm/arm64/flushtlb.h for the explanation of the sequence. */ Do you mean xen/include/asm-arm/arm64/flushtlb.h? I can't find the explanation you are referring to. > + dsb nshst > + tlbi alle2 > + dsb nsh > + isb > + > + ret > +ENDPROC(remove_id_map) > + > setup_fixmap: > - /* Now we can install the fixmap and dtb mappings, since we > - * don't need the 1:1 map any more */ > - dsb sy > #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ > /* Add UART to the fixmap table */ > ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > @@ -653,19 +718,10 @@ setup_fixmap: > ldr x1, =FIXMAP_ADDR(0) > lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ > str x2, [x4, x1] /* Map it in the fixmap's slot */ > -#endif > > - /* > - * Flush the TLB in case the 1:1 mapping happens to clash with > - * the virtual addresses used by the fixmap or DTB. > - */ > - dsb sy /* Ensure any page table updates made above > - * have occurred. */ > - > - isb > - tlbi alle2 > - dsb sy /* Ensure completion of TLB flush */ > - isb > + /* Ensure any page table updates made above have occurred */ > + dsb nshst > +#endif > ret > ENDPROC(setup_fixmap) > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-27 18:55 ` Stefano Stabellini @ 2019-06-27 19:30 ` Julien Grall 2019-07-10 19:39 ` Julien Grall 2019-07-30 17:33 ` Stefano Stabellini 0 siblings, 2 replies; 70+ messages in thread From: Julien Grall @ 2019-06-27 19:30 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/27/19 7:55 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> +1: >> + /* >> + * Find the second slot used. Remove the entry for the first >> + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). >> + * For slot 1, it means the ID map was not created. >> + */ >> + lsr x1, x19, #SECOND_SHIFT >> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ >> + cmp x1, #1 >> + beq id_map_removed >> + /* It is not in slot 1, remove the entry */ >> + ldr x0, =boot_second /* x0 := second table */ >> + str xzr, [x0, x1, lsl #3] > > Wouldn't it be a bit more reliable if we checked whether the slot in > question for x19 (whether zero, first, second) is a pagetable pointer or > section map, then zero it if it is a section map, otherwise go down one > level? If we did it this way it would be independent from the way > create_page_tables is written. Your suggestion will not comply with the architecture compliance and how Xen is/will be working after the full rework. We want to remove everything (mapping + table) added specifically for the 1:1 mapping. Otherwise, you may end up in a position where boot_first_id is still in place. We would need to use the break-before-make sequence in subsequent code if we were about to insert 1GB mapping at the same place. After my rework, we would have virtually no place where break-before-make will be necessary as it will enforce all the mappings to be destroyed before hand. So I would rather avoid to make a specific case for the 1:1 mapping. As a side note, the current code for the 1:1 mapping is completely wrong as using 1GB (or even 2MB) mapping may result to map MMIO region (or reserved-region). This may result to cache problem. I have this partially fixed on for the next version of series (see [1]). > > With the current code, we are somewhat reliant on the behavior of > create_page_tables, because we rely on the position of the slot for > the ID map? Where the assumption for instance is that at level one, if > the slot is zero, then we need to go down a level, etc. Instead, if we > checked if the slot is a section map, we could remove it immediately, if > it is a pagetable pointer, we proceed. The code should be similar in > complexity and LOC, but it would be more robust. See above :). > > Something like the following, in pseudo-uncompiled assembly: > > lsr x1, x19, #FIRST_SHIFT > ldr x0, =boot_first /* x0 := first table */ > ldr x2, [x0, x1, lsl #3] > # check x2 against #PT_MEM > cbz x2, 1f > str xzr, [x0, x1, lsl #3] > b id_map_removed > > >> +id_map_removed: >> + /* See asm-arm/arm64/flushtlb.h for the explanation of the sequence. */ > > Do you mean xen/include/asm-arm/arm64/flushtlb.h? I can't find the > explanation you are referring to. The big comment at the top of the header: /* * Every invalidation operation use the following patterns: * * DSB ISHST // Ensure prior page-tables updates have completed * TLBI... // Invalidate the TLB * DSB ISH // Ensure the TLB invalidation has completed * ISB // See explanation below * * For Xen page-tables the ISB will discard any instructions fetched * from the old mappings. * * For the Stage-2 page-tables the ISB ensures the completion of the DSB * (and therefore the TLB invalidation) before continuing. So we know * the TLBs cannot contain an entry for a mapping we may have removed. */ Note that we are using nsh (and not ish) because we are using local TLB flush (see page D5-230 ARM DDI 0487D.a). For convenience here is the text: "In all cases in this section where a DMB or DSB is referred to, it refers to a DMB or DSB whose required access type is both loads and stores. A DSB NSH is sufficient to ensure completion of TLB maintenance instructions that apply to a single PE. A DSB ISH is sufficient to ensure completion of TLB maintenance instructions that apply to PEs in the same Inner Shareable domain." I discovered this section after the changes in flushtlb.h has been merged. But I am thinking to do a follow-up the local TLB flush code. > > >> + dsb nshst >> + tlbi alle2 >> + dsb nsh >> + isb >> + >> + ret >> +ENDPROC(remove_id_map) [...] [1] Rework for create_page_tables diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index a79ae54822..c019dd3e04 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -483,6 +483,60 @@ cpu_init: ENDPROC(cpu_init) /* + * Macro to create a page table entry in \ptbl to \tbl + * + * ptbl: table symbol where the entry will be created + * tbl: table symbol to point to + * virt: virtual address + * shift: #imm page table shift + * tmp1: scratch register + * tmp2: scratch register + * tmp3: scratch register + * + * Preserves \virt + * Clobbers \tmp1, \tmp2, \tmp3 + * + * Also use x20 for the phys offset. + * + * Note that all parameters using registers should be distinct. + */ +.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3 + lsr \tmp1, \virt, #\shift + and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ + load_paddr \tmp2, \tbl + mov \tmp3, #PT_PT /* \tmp3 := right for linear PT */ + orr \tmp3, \tmp3, \tmp2 /* + \tlb paddr */ + adr_l \tmp2, \ptbl + str \tmp3, [\tmp2, \tmp1, lsl #3] +.endm + +/* + * Macro to create a mapping entry in \tbl to \paddr. Only mapping in 3rd + * level table is supported. + * + * tbl: table symbol where the entry will be created + * virt: virtual address + * paddr: physical address (should be page aligned) + * tmp1: scratch register + * tmp2: scratch register + * tmp3: scratch register + * type: mapping type. If not specified it will be normal memory (PT_MEM_L3) + * + * Preserves \virt, \paddr + * Clobbers \tmp1, \tmp2, \tmp3 + * + * Note that all parameters using registers should be distinct. + */ +.macro create_mapping_entry, tbl, virt, paddr, tmp1, tmp2, tmp3, type=PT_MEM_L3 + lsr \tmp1, \virt, #THIRD_SHIFT + and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ + mov \tmp2, #\type /* \tmp2 := right for section PT */ + orr \tmp2, \tmp2, \paddr /* + paddr */ + adr_l \tmp3, \tbl + str \tmp2, [\tmp3, \tmp1, lsl #3] +.endm + +/* * Rebuild the boot pagetable's first-level entries. The structure * is described in mm.c. * @@ -495,100 +549,17 @@ ENDPROC(cpu_init) * x19: paddr(start) * x20: phys offset * - * Clobbers x0 - x4, x25 - * - * Register usage within this function: - * x25: Identity map in place + * Clobbers x0 - x4 */ create_page_tables: - /* - * If Xen is loaded at exactly XEN_VIRT_START then we don't - * need an additional 1:1 mapping, the virtual mapping will - * suffice. - */ - cmp x19, #XEN_VIRT_START - cset x25, eq /* x25 := identity map in place, or not */ - - load_paddr x4, boot_pgtable - - /* Setup boot_pgtable: */ - load_paddr x1, boot_first - - /* ... map boot_first in boot_pgtable[0] */ - mov x3, #PT_PT /* x2 := table map of boot_first */ - orr x2, x1, x3 /* + rights for linear PT */ - str x2, [x4, #0] /* Map it in slot 0 */ - - /* ... map of paddr(start) in boot_pgtable+boot_first_id */ - lsr x1, x19, #ZEROETH_SHIFT/* Offset of base paddr in boot_pgtable */ - cbz x1, 1f /* It's in slot 0, map in boot_first - * or boot_second later on */ - - /* - * Level zero does not support superpage mappings, so we have - * to use an extra first level page in which we create a 1GB mapping. - */ - load_paddr x2, boot_first_id - - mov x3, #PT_PT /* x2 := table map of boot_first_id */ - orr x2, x2, x3 /* + rights for linear PT */ - str x2, [x4, x1, lsl #3] - - load_paddr x4, boot_first_id - - lsr x1, x19, #FIRST_SHIFT /* x1 := Offset of base paddr in boot_first_id */ - lsl x2, x1, #FIRST_SHIFT /* x2 := Base address for 1GB mapping */ - mov x3, #PT_MEM /* x2 := Section map */ - orr x2, x2, x3 - and x1, x1, #LPAE_ENTRY_MASK /* x1 := Slot offset */ - str x2, [x4, x1, lsl #3] /* Mapping of paddr(start) */ - mov x25, #1 /* x25 := identity map now in place */ - -1: /* Setup boot_first: */ - load_paddr x4, boot_first /* Next level into boot_first */ - - /* ... map boot_second in boot_first[0] */ - load_paddr x1, boot_second - mov x3, #PT_PT /* x2 := table map of boot_second */ - orr x2, x1, x3 /* + rights for linear PT */ - str x2, [x4, #0] /* Map it in slot 0 */ - - /* ... map of paddr(start) in boot_first */ - cbnz x25, 1f /* x25 is set if already created */ - lsr x2, x19, #FIRST_SHIFT /* x2 := Offset of base paddr in boot_first */ - and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */ - cbz x1, 1f /* It's in slot 0, map in boot_second */ - - lsl x2, x2, #FIRST_SHIFT /* Base address for 1GB mapping */ - mov x3, #PT_MEM /* x2 := Section map */ - orr x2, x2, x3 - str x2, [x4, x1, lsl #3] /* Create mapping of paddr(start)*/ - mov x25, #1 /* x25 := identity map now in place */ - -1: /* Setup boot_second: */ - load_paddr x4, boot_second - - /* ... map boot_third in boot_second[1] */ - load_paddr x1, boot_third - mov x3, #PT_PT /* x2 := table map of boot_third */ - orr x2, x1, x3 /* + rights for linear PT */ - str x2, [x4, #8] /* Map it in slot 1 */ - - /* ... map of paddr(start) in boot_second */ - cbnz x25, 1f /* x25 is set if already created */ - lsr x2, x19, #SECOND_SHIFT /* x2 := Offset of base paddr in boot_second */ - and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */ - cmp x1, #1 - b.eq virtphys_clash /* It's in slot 1, which we cannot handle */ + /* Prepare the page-tables for mapping Xen */ + ldr x0, =XEN_VIRT_START + create_table_entry boot_pgtable, boot_first, x0, ZEROETH_SHIFT, x1, x2, x3 + create_table_entry boot_first, boot_second, x0, FIRST_SHIFT, x1, x2, x3 + create_table_entry boot_second, boot_third, x0, SECOND_SHIFT, x1, x2, x3 - lsl x2, x2, #SECOND_SHIFT /* Base address for 2MB mapping */ - mov x3, #PT_MEM /* x2 := Section map */ - orr x2, x2, x3 - str x2, [x4, x1, lsl #3] /* Create mapping of paddr(start)*/ - mov x25, #1 /* x25 := identity map now in place */ - -1: /* Setup boot_third: */ - load_paddr x4, boot_third + /* Map Xen */ + adr_l x4, boot_third lsr x2, x19, #THIRD_SHIFT /* Base address for 4K mapping */ lsl x2, x2, #THIRD_SHIFT @@ -603,21 +574,68 @@ create_page_tables: cmp x1, #(LPAE_ENTRIES<<3) /* 512 entries per page */ b.lt 1b - /* Defer fixmap and dtb mapping until after paging enabled, to - * avoid them clashing with the 1:1 mapping. */ + /* + * If Xen is loaded at exactly XEN_VIRT_START then we don't + * need an additional 1:1 mapping, the virtual mapping will + * suffice. + */ + cmp x19, #XEN_VIRT_START + bne 1f + ret +1: + /* + * Only the first page of Xen will be part of the 1:1 mapping. + * All the boot_*_id tables are linked together even if they may + * not be all used. They will then be linked to the boot page + * tables at the correct level. + */ + create_table_entry boot_first_id, boot_second_id, x19, FIRST_SHIFT, x0, x1, x2 + create_table_entry boot_second_id, boot_third_id, x19, SECOND_SHIFT, x0, x1, x2 + create_mapping_entry boot_third_id, x19, x19, x0, x1, x2 + + /* + * Find the zeroeth slot used. Link boot_first_id into + * boot_pgtable if the slot is not 0. For slot 0, the tables + * associated with the 1:1 mapping will need to be linked in + * boot_first or boot_second. + */ + lsr x0, x19, #ZEROETH_SHIFT /* x0 := zeroeth slot */ + cbz x0, 1f + /* It is not in slot 0, Link boot_first_id into boot_pgtable */ + create_table_entry boot_pgtable, boot_first_id, x19, ZEROETH_SHIFT, x0, x1, x2 + ret + +1: + /* + * Find the first slot used. Link boot_second_id into boot_first + * if the slot is not 0. For slot 0, the tables associated with + * the 1:1 mapping will need to be linked in boot_second. + */ + lsr x0, x19, #FIRST_SHIFT + and x0, x0, #LPAE_ENTRY_MASK /* x0 := first slot */ + cbz x0, 1f + /* It is not in slot 0, Link boot_second_id into boot_first */ + create_table_entry boot_first, boot_second_id, x19, FIRST_SHIFT, x0, x1, x2 + ret - /* boot pagetable setup complete */ +1: + /* + * Find the second slot used. Link boot_third_id into boot_second + * if the slot is not 1 (runtime Xen mapping is 2M - 4M). + * For slot 1, Xen is not yet able to handle it. + */ + lsr x0, x19, #SECOND_SHIFT + and x0, x0, #LPAE_ENTRY_MASK /* x0 := first slot */ + cmp x0, #1 + beq virtphys_clash + /* It is not in slot 1, link boot_third_id into boot_second */ + create_table_entry boot_second, boot_third_id, x19, SECOND_SHIFT, x0, x1, x2 + ret - cbnz x25, 1f /* Did we manage to create an identity mapping ? */ - PRINT("Unable to build boot page tables - Failed to identity map Xen.\r\n") - b fail virtphys_clash: /* Identity map clashes with boot_third, which we cannot handle yet */ PRINT("- Unable to build boot page tables - virt and phys addresses clash. -\r\n") b fail - -1: - ret ENDPROC(create_page_tables) /* @@ -719,28 +737,15 @@ ENDPROC(remove_identity_mapping) * The fixmap cannot be mapped in create_page_tables because it may * clash with the 1:1 mapping. * - * Clobbers x1 - x4 + * Clobbers x0 - x3 */ setup_fixmap: #ifdef CONFIG_EARLY_PRINTK - /* Add UART to the fixmap table */ - ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ - lsr x2, x23, #THIRD_SHIFT - lsl x2, x2, #THIRD_SHIFT /* 4K aligned paddr of UART */ - mov x3, #PT_DEV_L3 - orr x2, x2, x3 /* x2 := 4K dev map including UART */ - str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ + ldr x0, =EARLY_UART_VIRTUAL_ADDRESS + create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3 #endif - - /* Map fixmap into boot_second */ - ldr x4, =boot_second /* x4 := vaddr (boot_second) */ - load_paddr x2, xen_fixmap - mov x3, #PT_PT - orr x2, x2, x3 /* x2 := table map of xen_fixmap */ - ldr x1, =FIXMAP_ADDR(0) - lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ - str x2, [x4, x1] /* Map it in the fixmap's slot */ - + ldr x0, =FIXMAP_ADDR(0) + create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT, x1, x2, x3 /* Ensure any page table updates made above have occurred */ dsb nshst ret diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index c2f1795a71..bc1824d3ca 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -107,6 +107,8 @@ DEFINE_BOOT_PAGE_TABLE(boot_pgtable); DEFINE_BOOT_PAGE_TABLE(boot_first); DEFINE_BOOT_PAGE_TABLE(boot_first_id); #endif +DEFINE_BOOT_PAGE_TABLE(boot_second_id); +DEFINE_BOOT_PAGE_TABLE(boot_third_id); DEFINE_BOOT_PAGE_TABLE(boot_second); DEFINE_BOOT_PAGE_TABLE(boot_third); -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-27 19:30 ` Julien Grall @ 2019-07-10 19:39 ` Julien Grall 2019-07-30 17:33 ` Stefano Stabellini 1 sibling, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-07-10 19:39 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi, @Stefano, I am going through the series and noticed you didn't give any update. Could you confirm if my reply makes sense? Cheers, On 6/27/19 8:30 PM, Julien Grall wrote: > Hi Stefano, > > On 6/27/19 7:55 PM, Stefano Stabellini wrote: >> On Mon, 10 Jun 2019, Julien Grall wrote: >>> +1: >>> + /* >>> + * Find the second slot used. Remove the entry for the first >>> + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). >>> + * For slot 1, it means the ID map was not created. >>> + */ >>> + lsr x1, x19, #SECOND_SHIFT >>> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ >>> + cmp x1, #1 >>> + beq id_map_removed >>> + /* It is not in slot 1, remove the entry */ >>> + ldr x0, =boot_second /* x0 := second table */ >>> + str xzr, [x0, x1, lsl #3] >> >> Wouldn't it be a bit more reliable if we checked whether the slot in >> question for x19 (whether zero, first, second) is a pagetable pointer or >> section map, then zero it if it is a section map, otherwise go down one >> level? If we did it this way it would be independent from the way >> create_page_tables is written. > > Your suggestion will not comply with the architecture compliance and how > Xen is/will be working after the full rework. We want to remove > everything (mapping + table) added specifically for the 1:1 mapping. > > Otherwise, you may end up in a position where boot_first_id is still in > place. We would need to use the break-before-make sequence in subsequent > code if we were about to insert 1GB mapping at the same place. > > After my rework, we would have virtually no place where > break-before-make will be necessary as it will enforce all the mappings > to be destroyed before hand. So I would rather avoid to make a specific > case for the 1:1 mapping. > > As a side note, the current code for the 1:1 mapping is completely wrong > as using 1GB (or even 2MB) mapping may result to map MMIO region (or > reserved-region). This may result to cache problem. I have this > partially fixed on for the next version of series (see [1]). > >> >> With the current code, we are somewhat reliant on the behavior of >> create_page_tables, because we rely on the position of the slot for >> the ID map? Where the assumption for instance is that at level one, if >> the slot is zero, then we need to go down a level, etc. Instead, if we >> checked if the slot is a section map, we could remove it immediately, if >> it is a pagetable pointer, we proceed. The code should be similar in >> complexity and LOC, but it would be more robust. > > See above :). > >> >> Something like the following, in pseudo-uncompiled assembly: >> >> lsr x1, x19, #FIRST_SHIFT >> ldr x0, =boot_first /* x0 := first table */ >> ldr x2, [x0, x1, lsl #3] >> # check x2 against #PT_MEM >> cbz x2, 1f >> str xzr, [x0, x1, lsl #3] >> b id_map_removed >> >> >>> +id_map_removed: >>> + /* See asm-arm/arm64/flushtlb.h for the explanation of the >>> sequence. */ >> >> Do you mean xen/include/asm-arm/arm64/flushtlb.h? I can't find the >> explanation you are referring to. > > The big comment at the top of the header: > > /* > * Every invalidation operation use the following patterns: > * > * DSB ISHST // Ensure prior page-tables updates have completed > * TLBI... // Invalidate the TLB > * DSB ISH // Ensure the TLB invalidation has completed > * ISB // See explanation below > * > * For Xen page-tables the ISB will discard any instructions fetched > * from the old mappings. > * > * For the Stage-2 page-tables the ISB ensures the completion of the DSB > * (and therefore the TLB invalidation) before continuing. So we know > * the TLBs cannot contain an entry for a mapping we may have removed. > */ > > Note that we are using nsh (and not ish) because we are using local TLB > flush (see page D5-230 ARM DDI 0487D.a). For convenience here is the text: > > "In all cases in this section where a DMB or DSB is referred to, it > refers to a DMB or DSB whose required access type is > both loads and stores. A DSB NSH is sufficient to ensure completion of > TLB maintenance instructions that apply to a > single PE. A DSB ISH is sufficient to ensure completion of TLB > maintenance instructions that apply to PEs in the > same Inner Shareable domain." > > I discovered this section after the changes in flushtlb.h has been > merged. But I am thinking to do a follow-up the local TLB flush code. > >> >> >>> + dsb nshst >>> + tlbi alle2 >>> + dsb nsh >>> + isb >>> + >>> + ret >>> +ENDPROC(remove_id_map) > > [...] > > [1] Rework for create_page_tables > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index a79ae54822..c019dd3e04 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -483,6 +483,60 @@ cpu_init: > ENDPROC(cpu_init) > > /* > + * Macro to create a page table entry in \ptbl to \tbl > + * > + * ptbl: table symbol where the entry will be created > + * tbl: table symbol to point to > + * virt: virtual address > + * shift: #imm page table shift > + * tmp1: scratch register > + * tmp2: scratch register > + * tmp3: scratch register > + * > + * Preserves \virt > + * Clobbers \tmp1, \tmp2, \tmp3 > + * > + * Also use x20 for the phys offset. > + * > + * Note that all parameters using registers should be distinct. > + */ > +.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3 > + lsr \tmp1, \virt, #\shift > + and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ > + load_paddr \tmp2, \tbl > + mov \tmp3, #PT_PT /* \tmp3 := right for > linear PT */ > + orr \tmp3, \tmp3, \tmp2 /* + \tlb paddr */ > + adr_l \tmp2, \ptbl > + str \tmp3, [\tmp2, \tmp1, lsl #3] > +.endm > + > +/* > + * Macro to create a mapping entry in \tbl to \paddr. Only mapping in 3rd > + * level table is supported. > + * > + * tbl: table symbol where the entry will be created > + * virt: virtual address > + * paddr: physical address (should be page aligned) > + * tmp1: scratch register > + * tmp2: scratch register > + * tmp3: scratch register > + * type: mapping type. If not specified it will be normal memory > (PT_MEM_L3) > + * > + * Preserves \virt, \paddr > + * Clobbers \tmp1, \tmp2, \tmp3 > + * > + * Note that all parameters using registers should be distinct. > + */ > +.macro create_mapping_entry, tbl, virt, paddr, tmp1, tmp2, tmp3, > type=PT_MEM_L3 > + lsr \tmp1, \virt, #THIRD_SHIFT > + and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ > + mov \tmp2, #\type /* \tmp2 := right for > section PT */ > + orr \tmp2, \tmp2, \paddr /* + paddr */ > + adr_l \tmp3, \tbl > + str \tmp2, [\tmp3, \tmp1, lsl #3] > +.endm > + > +/* > * Rebuild the boot pagetable's first-level entries. The structure > * is described in mm.c. > * > @@ -495,100 +549,17 @@ ENDPROC(cpu_init) > * x19: paddr(start) > * x20: phys offset > * > - * Clobbers x0 - x4, x25 > - * > - * Register usage within this function: > - * x25: Identity map in place > + * Clobbers x0 - x4 > */ > create_page_tables: > - /* > - * If Xen is loaded at exactly XEN_VIRT_START then we don't > - * need an additional 1:1 mapping, the virtual mapping will > - * suffice. > - */ > - cmp x19, #XEN_VIRT_START > - cset x25, eq /* x25 := identity map in place, > or not */ > - > - load_paddr x4, boot_pgtable > - > - /* Setup boot_pgtable: */ > - load_paddr x1, boot_first > - > - /* ... map boot_first in boot_pgtable[0] */ > - mov x3, #PT_PT /* x2 := table map of boot_first */ > - orr x2, x1, x3 /* + rights for linear PT */ > - str x2, [x4, #0] /* Map it in slot 0 */ > - > - /* ... map of paddr(start) in boot_pgtable+boot_first_id */ > - lsr x1, x19, #ZEROETH_SHIFT/* Offset of base paddr in > boot_pgtable */ > - cbz x1, 1f /* It's in slot 0, map in boot_first > - * or boot_second later on */ > - > - /* > - * Level zero does not support superpage mappings, so we have > - * to use an extra first level page in which we create a 1GB > mapping. > - */ > - load_paddr x2, boot_first_id > - > - mov x3, #PT_PT /* x2 := table map of > boot_first_id */ > - orr x2, x2, x3 /* + rights for linear PT */ > - str x2, [x4, x1, lsl #3] > - > - load_paddr x4, boot_first_id > - > - lsr x1, x19, #FIRST_SHIFT /* x1 := Offset of base paddr in > boot_first_id */ > - lsl x2, x1, #FIRST_SHIFT /* x2 := Base address for 1GB > mapping */ > - mov x3, #PT_MEM /* x2 := Section map */ > - orr x2, x2, x3 > - and x1, x1, #LPAE_ENTRY_MASK /* x1 := Slot offset */ > - str x2, [x4, x1, lsl #3] /* Mapping of paddr(start) */ > - mov x25, #1 /* x25 := identity map now in > place */ > - > -1: /* Setup boot_first: */ > - load_paddr x4, boot_first /* Next level into boot_first */ > - > - /* ... map boot_second in boot_first[0] */ > - load_paddr x1, boot_second > - mov x3, #PT_PT /* x2 := table map of boot_second */ > - orr x2, x1, x3 /* + rights for linear PT */ > - str x2, [x4, #0] /* Map it in slot 0 */ > - > - /* ... map of paddr(start) in boot_first */ > - cbnz x25, 1f /* x25 is set if already created */ > - lsr x2, x19, #FIRST_SHIFT /* x2 := Offset of base paddr in > boot_first */ > - and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */ > - cbz x1, 1f /* It's in slot 0, map in > boot_second */ > - > - lsl x2, x2, #FIRST_SHIFT /* Base address for 1GB mapping */ > - mov x3, #PT_MEM /* x2 := Section map */ > - orr x2, x2, x3 > - str x2, [x4, x1, lsl #3] /* Create mapping of paddr(start)*/ > - mov x25, #1 /* x25 := identity map now in > place */ > - > -1: /* Setup boot_second: */ > - load_paddr x4, boot_second > - > - /* ... map boot_third in boot_second[1] */ > - load_paddr x1, boot_third > - mov x3, #PT_PT /* x2 := table map of boot_third */ > - orr x2, x1, x3 /* + rights for linear PT */ > - str x2, [x4, #8] /* Map it in slot 1 */ > - > - /* ... map of paddr(start) in boot_second */ > - cbnz x25, 1f /* x25 is set if already created */ > - lsr x2, x19, #SECOND_SHIFT /* x2 := Offset of base paddr in > boot_second */ > - and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */ > - cmp x1, #1 > - b.eq virtphys_clash /* It's in slot 1, which we cannot > handle */ > + /* Prepare the page-tables for mapping Xen */ > + ldr x0, =XEN_VIRT_START > + create_table_entry boot_pgtable, boot_first, x0, ZEROETH_SHIFT, > x1, x2, x3 > + create_table_entry boot_first, boot_second, x0, FIRST_SHIFT, > x1, x2, x3 > + create_table_entry boot_second, boot_third, x0, SECOND_SHIFT, > x1, x2, x3 > > - lsl x2, x2, #SECOND_SHIFT /* Base address for 2MB mapping */ > - mov x3, #PT_MEM /* x2 := Section map */ > - orr x2, x2, x3 > - str x2, [x4, x1, lsl #3] /* Create mapping of paddr(start)*/ > - mov x25, #1 /* x25 := identity map now in > place */ > - > -1: /* Setup boot_third: */ > - load_paddr x4, boot_third > + /* Map Xen */ > + adr_l x4, boot_third > > lsr x2, x19, #THIRD_SHIFT /* Base address for 4K mapping */ > lsl x2, x2, #THIRD_SHIFT > @@ -603,21 +574,68 @@ create_page_tables: > cmp x1, #(LPAE_ENTRIES<<3) /* 512 entries per page */ > b.lt 1b > > - /* Defer fixmap and dtb mapping until after paging enabled, to > - * avoid them clashing with the 1:1 mapping. */ > + /* > + * If Xen is loaded at exactly XEN_VIRT_START then we don't > + * need an additional 1:1 mapping, the virtual mapping will > + * suffice. > + */ > + cmp x19, #XEN_VIRT_START > + bne 1f > + ret > +1: > + /* > + * Only the first page of Xen will be part of the 1:1 mapping. > + * All the boot_*_id tables are linked together even if they may > + * not be all used. They will then be linked to the boot page > + * tables at the correct level. > + */ > + create_table_entry boot_first_id, boot_second_id, x19, > FIRST_SHIFT, x0, x1, x2 > + create_table_entry boot_second_id, boot_third_id, x19, > SECOND_SHIFT, x0, x1, x2 > + create_mapping_entry boot_third_id, x19, x19, x0, x1, x2 > + > + /* > + * Find the zeroeth slot used. Link boot_first_id into > + * boot_pgtable if the slot is not 0. For slot 0, the tables > + * associated with the 1:1 mapping will need to be linked in > + * boot_first or boot_second. > + */ > + lsr x0, x19, #ZEROETH_SHIFT /* x0 := zeroeth slot */ > + cbz x0, 1f > + /* It is not in slot 0, Link boot_first_id into boot_pgtable */ > + create_table_entry boot_pgtable, boot_first_id, x19, > ZEROETH_SHIFT, x0, x1, x2 > + ret > + > +1: > + /* > + * Find the first slot used. Link boot_second_id into boot_first > + * if the slot is not 0. For slot 0, the tables associated with > + * the 1:1 mapping will need to be linked in boot_second. > + */ > + lsr x0, x19, #FIRST_SHIFT > + and x0, x0, #LPAE_ENTRY_MASK /* x0 := first slot */ > + cbz x0, 1f > + /* It is not in slot 0, Link boot_second_id into boot_first */ > + create_table_entry boot_first, boot_second_id, x19, > FIRST_SHIFT, x0, x1, x2 > + ret > > - /* boot pagetable setup complete */ > +1: > + /* > + * Find the second slot used. Link boot_third_id into boot_second > + * if the slot is not 1 (runtime Xen mapping is 2M - 4M). > + * For slot 1, Xen is not yet able to handle it. > + */ > + lsr x0, x19, #SECOND_SHIFT > + and x0, x0, #LPAE_ENTRY_MASK /* x0 := first slot */ > + cmp x0, #1 > + beq virtphys_clash > + /* It is not in slot 1, link boot_third_id into boot_second */ > + create_table_entry boot_second, boot_third_id, x19, > SECOND_SHIFT, x0, x1, x2 > + ret > > - cbnz x25, 1f /* Did we manage to create an > identity mapping ? */ > - PRINT("Unable to build boot page tables - Failed to identity > map Xen.\r\n") > - b fail > virtphys_clash: > /* Identity map clashes with boot_third, which we cannot > handle yet */ > PRINT("- Unable to build boot page tables - virt and phys > addresses clash. -\r\n") > b fail > - > -1: > - ret > ENDPROC(create_page_tables) > > /* > @@ -719,28 +737,15 @@ ENDPROC(remove_identity_mapping) > * The fixmap cannot be mapped in create_page_tables because it may > * clash with the 1:1 mapping. > * > - * Clobbers x1 - x4 > + * Clobbers x0 - x3 > */ > setup_fixmap: > #ifdef CONFIG_EARLY_PRINTK > - /* Add UART to the fixmap table */ > - ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > - lsr x2, x23, #THIRD_SHIFT > - lsl x2, x2, #THIRD_SHIFT /* 4K aligned paddr of UART */ > - mov x3, #PT_DEV_L3 > - orr x2, x2, x3 /* x2 := 4K dev map including UART */ > - str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first > fixmap's slot */ > + ldr x0, =EARLY_UART_VIRTUAL_ADDRESS > + create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, > type=PT_DEV_L3 > #endif > - > - /* Map fixmap into boot_second */ > - ldr x4, =boot_second /* x4 := vaddr (boot_second) */ > - load_paddr x2, xen_fixmap > - mov x3, #PT_PT > - orr x2, x2, x3 /* x2 := table map of xen_fixmap */ > - ldr x1, =FIXMAP_ADDR(0) > - lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ > - str x2, [x4, x1] /* Map it in the fixmap's slot */ > - > + ldr x0, =FIXMAP_ADDR(0) > + create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT, > x1, x2, x3 > /* Ensure any page table updates made above have occurred */ > dsb nshst > ret > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index c2f1795a71..bc1824d3ca 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -107,6 +107,8 @@ DEFINE_BOOT_PAGE_TABLE(boot_pgtable); > DEFINE_BOOT_PAGE_TABLE(boot_first); > DEFINE_BOOT_PAGE_TABLE(boot_first_id); > #endif > +DEFINE_BOOT_PAGE_TABLE(boot_second_id); > +DEFINE_BOOT_PAGE_TABLE(boot_third_id); > DEFINE_BOOT_PAGE_TABLE(boot_second); > DEFINE_BOOT_PAGE_TABLE(boot_third); > > -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-06-27 19:30 ` Julien Grall 2019-07-10 19:39 ` Julien Grall @ 2019-07-30 17:33 ` Stefano Stabellini 2019-07-30 19:52 ` Julien Grall 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-07-30 17:33 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Thu, 27 Jun 2019, Julien Grall wrote: > On 6/27/19 7:55 PM, Stefano Stabellini wrote: > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > +1: > > > + /* > > > + * Find the second slot used. Remove the entry for the first > > > + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). > > > + * For slot 1, it means the ID map was not created. > > > + */ > > > + lsr x1, x19, #SECOND_SHIFT > > > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > > > + cmp x1, #1 > > > + beq id_map_removed > > > + /* It is not in slot 1, remove the entry */ > > > + ldr x0, =boot_second /* x0 := second table */ > > > + str xzr, [x0, x1, lsl #3] > > > > Wouldn't it be a bit more reliable if we checked whether the slot in > > question for x19 (whether zero, first, second) is a pagetable pointer or > > section map, then zero it if it is a section map, otherwise go down one > > level? If we did it this way it would be independent from the way > > create_page_tables is written. > > Your suggestion will not comply with the architecture compliance and how Xen > is/will be working after the full rework. We want to remove everything > (mapping + table) added specifically for the 1:1 mapping. > > Otherwise, you may end up in a position where boot_first_id is still in place. > We would need to use the break-before-make sequence in subsequent code if we > were about to insert 1GB mapping at the same place. > > After my rework, we would have virtually no place where break-before-make will > be necessary as it will enforce all the mappings to be destroyed before hand. > So I would rather avoid to make a specific case for the 1:1 mapping. I don't fully understand your explanation. I understand the final goal of "removing everything (mapping + table) added specifically for the 1:1 mapping". I don't understand why my suggestion would be a hindrance toward that goal, compared to what it is done in this patch. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-07-30 17:33 ` Stefano Stabellini @ 2019-07-30 19:52 ` Julien Grall 2019-07-31 20:40 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-07-30 19:52 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 7/30/19 6:33 PM, Stefano Stabellini wrote: > On Thu, 27 Jun 2019, Julien Grall wrote: >> On 6/27/19 7:55 PM, Stefano Stabellini wrote: >>> On Mon, 10 Jun 2019, Julien Grall wrote: >>>> +1: >>>> + /* >>>> + * Find the second slot used. Remove the entry for the first >>>> + * table if the slot is not 1 (runtime Xen mapping is 2M - 4M). >>>> + * For slot 1, it means the ID map was not created. >>>> + */ >>>> + lsr x1, x19, #SECOND_SHIFT >>>> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ >>>> + cmp x1, #1 >>>> + beq id_map_removed >>>> + /* It is not in slot 1, remove the entry */ >>>> + ldr x0, =boot_second /* x0 := second table */ >>>> + str xzr, [x0, x1, lsl #3] >>> >>> Wouldn't it be a bit more reliable if we checked whether the slot in >>> question for x19 (whether zero, first, second) is a pagetable pointer or >>> section map, then zero it if it is a section map, otherwise go down one >>> level? If we did it this way it would be independent from the way >>> create_page_tables is written. >> >> Your suggestion will not comply with the architecture compliance and how Xen >> is/will be working after the full rework. We want to remove everything >> (mapping + table) added specifically for the 1:1 mapping. >> >> Otherwise, you may end up in a position where boot_first_id is still in place. >> We would need to use the break-before-make sequence in subsequent code if we >> were about to insert 1GB mapping at the same place. >> >> After my rework, we would have virtually no place where break-before-make will >> be necessary as it will enforce all the mappings to be destroyed before hand. >> So I would rather avoid to make a specific case for the 1:1 mapping. > > I don't fully understand your explanation. I understand the final goal > of "removing everything (mapping + table) added specifically for the 1:1 > mapping". I don't understand why my suggestion would be a hindrance > toward that goal, compared to what it is done in this patch. Because, AFAICT, your suggestion will only remove the mapping and not the tables (such as boot_first_id). This is different from this patch where both mapping and tables are removed. So yes, my suggestion is not generic, but at least it does the job that is expected by this function. I.e removing anything that was specifically created for the identity mapping. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-07-30 19:52 ` Julien Grall @ 2019-07-31 20:40 ` Stefano Stabellini 2019-07-31 21:07 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-07-31 20:40 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Tue, 30 Jul 2019, Julien Grall wrote: > Hi Stefano, > > On 7/30/19 6:33 PM, Stefano Stabellini wrote: > > On Thu, 27 Jun 2019, Julien Grall wrote: > > > On 6/27/19 7:55 PM, Stefano Stabellini wrote: > > > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > > > +1: > > > > > + /* > > > > > + * Find the second slot used. Remove the entry for the first > > > > > + * table if the slot is not 1 (runtime Xen mapping is 2M - > > > > > 4M). > > > > > + * For slot 1, it means the ID map was not created. > > > > > + */ > > > > > + lsr x1, x19, #SECOND_SHIFT > > > > > + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ > > > > > + cmp x1, #1 > > > > > + beq id_map_removed > > > > > + /* It is not in slot 1, remove the entry */ > > > > > + ldr x0, =boot_second /* x0 := second table */ > > > > > + str xzr, [x0, x1, lsl #3] > > > > > > > > Wouldn't it be a bit more reliable if we checked whether the slot in > > > > question for x19 (whether zero, first, second) is a pagetable pointer or > > > > section map, then zero it if it is a section map, otherwise go down one > > > > level? If we did it this way it would be independent from the way > > > > create_page_tables is written. > > > > > > Your suggestion will not comply with the architecture compliance and how > > > Xen > > > is/will be working after the full rework. We want to remove everything > > > (mapping + table) added specifically for the 1:1 mapping. > > > > > > Otherwise, you may end up in a position where boot_first_id is still in > > > place. > > > We would need to use the break-before-make sequence in subsequent code if > > > we > > > were about to insert 1GB mapping at the same place. > > > > > > After my rework, we would have virtually no place where break-before-make > > > will > > > be necessary as it will enforce all the mappings to be destroyed before > > > hand. > > > So I would rather avoid to make a specific case for the 1:1 mapping. > > > > I don't fully understand your explanation. I understand the final goal > > of "removing everything (mapping + table) added specifically for the 1:1 > > mapping". I don't understand why my suggestion would be a hindrance > > toward that goal, compared to what it is done in this patch. > > Because, AFAICT, your suggestion will only remove the mapping and not the > tables (such as boot_first_id). This is different from this patch where both > mapping and tables are removed. > > So yes, my suggestion is not generic, but at least it does the job that is > expected by this function. I.e removing anything that was specifically created > for the identity mapping. I understand your comment now, and of course I agree that both mapping and tables need to be removed. I am careful making suggestions for assembly coding because I don't really want to suggest something that doesn't work, or even if it works that it's worse than the original. It should be possible to remove both the table and the mapping in a generic way. Instead of hardcoding the assemply equivalent of "It is not in slot 0, remove the entry", we could check whether the table offset matches the table offset of the mapping that we want to preserve. That way, "slot 0" would be calculate instead of hardcoded, and the code would be pretty generic. What do you think? It should only be a small addition. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used 2019-07-31 20:40 ` Stefano Stabellini @ 2019-07-31 21:07 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-07-31 21:07 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 7/31/19 9:40 PM, Stefano Stabellini wrote: > On Tue, 30 Jul 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 7/30/19 6:33 PM, Stefano Stabellini wrote: >>> On Thu, 27 Jun 2019, Julien Grall wrote: >>>> On 6/27/19 7:55 PM, Stefano Stabellini wrote: >>>>> On Mon, 10 Jun 2019, Julien Grall wrote: >>>>>> +1: >>>>>> + /* >>>>>> + * Find the second slot used. Remove the entry for the first >>>>>> + * table if the slot is not 1 (runtime Xen mapping is 2M - >>>>>> 4M). >>>>>> + * For slot 1, it means the ID map was not created. >>>>>> + */ >>>>>> + lsr x1, x19, #SECOND_SHIFT >>>>>> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ >>>>>> + cmp x1, #1 >>>>>> + beq id_map_removed >>>>>> + /* It is not in slot 1, remove the entry */ >>>>>> + ldr x0, =boot_second /* x0 := second table */ >>>>>> + str xzr, [x0, x1, lsl #3] >>>>> >>>>> Wouldn't it be a bit more reliable if we checked whether the slot in >>>>> question for x19 (whether zero, first, second) is a pagetable pointer or >>>>> section map, then zero it if it is a section map, otherwise go down one >>>>> level? If we did it this way it would be independent from the way >>>>> create_page_tables is written. >>>> >>>> Your suggestion will not comply with the architecture compliance and how >>>> Xen >>>> is/will be working after the full rework. We want to remove everything >>>> (mapping + table) added specifically for the 1:1 mapping. >>>> >>>> Otherwise, you may end up in a position where boot_first_id is still in >>>> place. >>>> We would need to use the break-before-make sequence in subsequent code if >>>> we >>>> were about to insert 1GB mapping at the same place. >>>> >>>> After my rework, we would have virtually no place where break-before-make >>>> will >>>> be necessary as it will enforce all the mappings to be destroyed before >>>> hand. >>>> So I would rather avoid to make a specific case for the 1:1 mapping. >>> >>> I don't fully understand your explanation. I understand the final goal >>> of "removing everything (mapping + table) added specifically for the 1:1 >>> mapping". I don't understand why my suggestion would be a hindrance >>> toward that goal, compared to what it is done in this patch. >> >> Because, AFAICT, your suggestion will only remove the mapping and not the >> tables (such as boot_first_id). This is different from this patch where both >> mapping and tables are removed. >> >> So yes, my suggestion is not generic, but at least it does the job that is >> expected by this function. I.e removing anything that was specifically created >> for the identity mapping. > > I understand your comment now, and of course I agree that both mapping > and tables need to be removed. > > I am careful making suggestions for assembly coding because I don't > really want to suggest something that doesn't work, or even if it works > that it's worse than the original. > > It should be possible to remove both the table and the mapping in a > generic way. Instead of hardcoding the assemply equivalent of "It is not > in slot 0, remove the entry", we could check whether the table offset > matches the table offset of the mapping that we want to preserve. That > way, "slot 0" would be calculate instead of hardcoded, and the code > would be pretty generic. What do you think? It should only be a small > addition. It should be feasible and may actually help the next step in my plan where I need to make Xen relocatable. I will have a look for both the arm32 and arm64 code. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (13 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 19:01 ` Stefano Stabellini 2019-06-26 19:02 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on Julien Grall 16 siblings, 2 replies; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko At the moment, the fixmap table is only hooked when earlyprintk is used. This is fine today because in C land, the fixmap is not used by anyone until the the boot CPU is switching to the runtime page-tables. In the future, the boot CPU will not switch between page-tables to avoid TLB conflict. This means the fixmap table will need to be hooked before any use. For simplicity, setup_fixmap() will now do that job. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 96e85f8834..4f7fa6769f 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -700,8 +700,17 @@ id_map_removed: ret ENDPROC(remove_id_map) +/* + * Map the UART in the fixmap (when earlyprintk is used) and hook the + * fixmap table in the page tables. + * + * The fixmap cannot be mapped in create_page_tables because it may + * clash with the ID map. + * + * Clobbers x0 - x1 + */ setup_fixmap: -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ +#ifdef CONFIG_EARLY_PRINTK /* Add UART to the fixmap table */ ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ lsr x2, x23, #THIRD_SHIFT @@ -709,6 +718,7 @@ setup_fixmap: mov x3, #PT_DEV_L3 orr x2, x2, x3 /* x2 := 4K dev map including UART */ str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ +#endif /* Map fixmap into boot_second */ ldr x4, =boot_second /* x4 := vaddr (boot_second) */ @@ -721,7 +731,6 @@ setup_fixmap: /* Ensure any page table updates made above have occurred */ dsb nshst -#endif ret ENDPROC(setup_fixmap) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-10 19:32 ` [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() Julien Grall @ 2019-06-26 19:01 ` Stefano Stabellini 2019-06-26 19:30 ` Julien Grall 2019-06-26 19:02 ` Stefano Stabellini 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 19:01 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > At the moment, the fixmap table is only hooked when earlyprintk is used. > This is fine today because in C land, the fixmap is not used by anyone > until the the boot CPU is switching to the runtime page-tables. > > In the future, the boot CPU will not switch between page-tables to avoid > TLB conflict. This means the fixmap table will need to be hooked before > any use. For simplicity, setup_fixmap() will now do that job. Can I ask you to reword this slightly, especially the last sentence? It took me a while to understand what you meant. I suggest: In the future, the boot CPU will not switch between page-tables to avoid any TLB conflicts. Thus, the fixmap table will need to be always hooked before any use. Let's start doing it now in setup_fixmap(). Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 96e85f8834..4f7fa6769f 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -700,8 +700,17 @@ id_map_removed: > ret > ENDPROC(remove_id_map) > > +/* > + * Map the UART in the fixmap (when earlyprintk is used) and hook the > + * fixmap table in the page tables. > + * > + * The fixmap cannot be mapped in create_page_tables because it may > + * clash with the ID map. > + * > + * Clobbers x0 - x1 > + */ > setup_fixmap: > -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ > +#ifdef CONFIG_EARLY_PRINTK I am curious why you made this code style change > /* Add UART to the fixmap table */ > ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > lsr x2, x23, #THIRD_SHIFT > @@ -709,6 +718,7 @@ setup_fixmap: > mov x3, #PT_DEV_L3 > orr x2, x2, x3 /* x2 := 4K dev map including UART */ > str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ > +#endif > > /* Map fixmap into boot_second */ > ldr x4, =boot_second /* x4 := vaddr (boot_second) */ > @@ -721,7 +731,6 @@ setup_fixmap: > > /* Ensure any page table updates made above have occurred */ > dsb nshst > -#endif > ret > ENDPROC(setup_fixmap) > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-26 19:01 ` Stefano Stabellini @ 2019-06-26 19:30 ` Julien Grall 2019-06-27 9:29 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-26 19:30 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/26/19 8:01 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> At the moment, the fixmap table is only hooked when earlyprintk is used. >> This is fine today because in C land, the fixmap is not used by anyone >> until the the boot CPU is switching to the runtime page-tables. >> >> In the future, the boot CPU will not switch between page-tables to avoid >> TLB conflict. This means the fixmap table will need to be hooked before >> any use. For simplicity, setup_fixmap() will now do that job. > > Can I ask you to reword this slightly, especially the last sentence? It > took me a while to understand what you meant. I suggest: > > In the future, the boot CPU will not switch between page-tables to > avoid any TLB conflicts. Thus, the fixmap table will need to be always > hooked before any use. Let's start doing it now in setup_fixmap(). > I will update the commit message. > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > >> Lastly, document the behavior and the main registers usage within the >> function. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 96e85f8834..4f7fa6769f 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -700,8 +700,17 @@ id_map_removed: >> ret >> ENDPROC(remove_id_map) >> >> +/* >> + * Map the UART in the fixmap (when earlyprintk is used) and hook the >> + * fixmap table in the page tables. >> + * >> + * The fixmap cannot be mapped in create_page_tables because it may >> + * clash with the ID map. >> + * >> + * Clobbers x0 - x1 >> + */ >> setup_fixmap: >> -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ >> +#ifdef CONFIG_EARLY_PRINTK > > I am curious why you made this code style change This is the only #if defined within head.S (the rest use #ifdef) so for consistency. Also, it is simpler to read :). Cheers, -- Julien Grall I _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-26 19:30 ` Julien Grall @ 2019-06-27 9:29 ` Julien Grall 2019-06-27 15:38 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-27 9:29 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 20:30, Julien Grall wrote: > On 6/26/19 8:01 PM, Stefano Stabellini wrote: >> On Mon, 10 Jun 2019, Julien Grall wrote: >>> At the moment, the fixmap table is only hooked when earlyprintk is used. >>> This is fine today because in C land, the fixmap is not used by anyone >>> until the the boot CPU is switching to the runtime page-tables. >>> >>> In the future, the boot CPU will not switch between page-tables to avoid >>> TLB conflict. This means the fixmap table will need to be hooked before >>> any use. For simplicity, setup_fixmap() will now do that job. >> >> Can I ask you to reword this slightly, especially the last sentence? It >> took me a while to understand what you meant. I suggest: >> >> In the future, the boot CPU will not switch between page-tables to >> avoid any TLB conflicts. Thus, the fixmap table will need to be always >> hooked before any use. Let's start doing it now in setup_fixmap(). >> > > I will update the commit message. I realized the commit message I wrote is inaccurate and reflected to your rewording. Not all the platforms will generate a TLB conflict abort. Some of them may just decide to use an amalgamation of two entries (see "TLB matching" page D5-2500 in ARM DDI 0487D.b). I will replace "any TLB conflicts" by "TLB incoherency". > >> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Let me know if you are happy with the change suggested. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-27 9:29 ` Julien Grall @ 2019-06-27 15:38 ` Stefano Stabellini 0 siblings, 0 replies; 70+ messages in thread From: Stefano Stabellini @ 2019-06-27 15:38 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara [-- Attachment #1: Type: text/plain, Size: 1597 bytes --] On Thu, 27 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 26/06/2019 20:30, Julien Grall wrote: > > On 6/26/19 8:01 PM, Stefano Stabellini wrote: > > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > > At the moment, the fixmap table is only hooked when earlyprintk is used. > > > > This is fine today because in C land, the fixmap is not used by anyone > > > > until the the boot CPU is switching to the runtime page-tables. > > > > > > > > In the future, the boot CPU will not switch between page-tables to avoid > > > > TLB conflict. This means the fixmap table will need to be hooked before > > > > any use. For simplicity, setup_fixmap() will now do that job. > > > > > > Can I ask you to reword this slightly, especially the last sentence? It > > > took me a while to understand what you meant. I suggest: > > > > > > In the future, the boot CPU will not switch between page-tables to > > > avoid any TLB conflicts. Thus, the fixmap table will need to be always > > > hooked before any use. Let's start doing it now in setup_fixmap(). > > > > > > > I will update the commit message. > > I realized the commit message I wrote is inaccurate and reflected to your > rewording. > > Not all the platforms will generate a TLB conflict abort. Some of them may > just decide to use an amalgamation of two entries (see "TLB matching" page > D5-2500 in ARM DDI 0487D.b). > > I will replace "any TLB conflicts" by "TLB incoherency". > > > > > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > > > Let me know if you are happy with the change suggested. Yes, that's fine [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-10 19:32 ` [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() Julien Grall 2019-06-26 19:01 ` Stefano Stabellini @ 2019-06-26 19:02 ` Stefano Stabellini 2019-06-27 9:19 ` Julien Grall 1 sibling, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 19:02 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > At the moment, the fixmap table is only hooked when earlyprintk is used. > This is fine today because in C land, the fixmap is not used by anyone > until the the boot CPU is switching to the runtime page-tables. > > In the future, the boot CPU will not switch between page-tables to avoid > TLB conflict. This means the fixmap table will need to be hooked before > any use. For simplicity, setup_fixmap() will now do that job. > > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 96e85f8834..4f7fa6769f 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -700,8 +700,17 @@ id_map_removed: > ret > ENDPROC(remove_id_map) > > +/* > + * Map the UART in the fixmap (when earlyprintk is used) and hook the > + * fixmap table in the page tables. > + * > + * The fixmap cannot be mapped in create_page_tables because it may > + * clash with the ID map. > + * > + * Clobbers x0 - x1 I missed this in the last email: it should be x0 - x4? > + */ > setup_fixmap: > -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ > +#ifdef CONFIG_EARLY_PRINTK > /* Add UART to the fixmap table */ > ldr x1, =xen_fixmap /* x1 := vaddr (xen_fixmap) */ > lsr x2, x23, #THIRD_SHIFT > @@ -709,6 +718,7 @@ setup_fixmap: > mov x3, #PT_DEV_L3 > orr x2, x2, x3 /* x2 := 4K dev map including UART */ > str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ > +#endif > > /* Map fixmap into boot_second */ > ldr x4, =boot_second /* x4 := vaddr (boot_second) */ > @@ -721,7 +731,6 @@ setup_fixmap: > > /* Ensure any page table updates made above have occurred */ > dsb nshst > -#endif > ret > ENDPROC(setup_fixmap) > > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() 2019-06-26 19:02 ` Stefano Stabellini @ 2019-06-27 9:19 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-27 9:19 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 20:02, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> At the moment, the fixmap table is only hooked when earlyprintk is used. >> This is fine today because in C land, the fixmap is not used by anyone >> until the the boot CPU is switching to the runtime page-tables. >> >> In the future, the boot CPU will not switch between page-tables to avoid >> TLB conflict. This means the fixmap table will need to be hooked before >> any use. For simplicity, setup_fixmap() will now do that job. >> >> Lastly, document the behavior and the main registers usage within the >> function. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 96e85f8834..4f7fa6769f 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -700,8 +700,17 @@ id_map_removed: >> ret >> ENDPROC(remove_id_map) >> >> +/* >> + * Map the UART in the fixmap (when earlyprintk is used) and hook the >> + * fixmap table in the page tables. >> + * >> + * The fixmap cannot be mapped in create_page_tables because it may >> + * clash with the ID map. >> + * >> + * Clobbers x0 - x1 > > I missed this in the last email: it should be x0 - x4? x0 is not used in the setup_fixmap. So it should be x1 - x4. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (14 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 19:12 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on Julien Grall 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko Boot CPU and secondary CPUs will use different entry point to C code. At the moment, the decision on which entry to use is taken within launch(). In order to avoid a branch for the decision and make the code clearer, launch() is reworked to take in parameters the entry point and its arguments. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/arm64/head.S | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 4f7fa6769f..130ab66d8e 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -312,6 +312,11 @@ primary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif + PRINT("- Ready -\r\n") + /* Setup the arguments for start_xen and jump to C world */ + mov x0, x20 /* x0 := phys_offset */ + mov x1, x21 /* x1 := paddr(FDT) */ + ldr x2, =start_xen b launch ENDPROC(real_start) @@ -374,6 +379,9 @@ secondary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif + PRINT("- Ready -\r\n") + /* Jump to C world */ + ldr x2, =start_secondary b launch ENDPROC(init_secondary) @@ -734,23 +742,24 @@ setup_fixmap: ret ENDPROC(setup_fixmap) +/* + * Setup the initial stack and jump to the C world + * + * Inputs: + * x0 : Argument 0 of the C function to call + * x1 : Argument 1 of the C function to call + * x2 : C entry point + */ launch: - PRINT("- Ready -\r\n") - - ldr x0, =init_data - add x0, x0, #INITINFO_stack /* Find the boot-time stack */ - ldr x0, [x0] - add x0, x0, #STACK_SIZE /* (which grows down from the top). */ - sub x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */ - mov sp, x0 - - cbnz x22, 1f - - mov x0, x20 /* Marshal args: - phys_offset */ - mov x1, x21 /* - FDT */ - b start_xen /* and disappear into the land of C */ -1: - b start_secondary /* (to the appropriate entry point) */ + ldr x4, =init_data + add x4, x4, #INITINFO_stack /* Find the boot-time stack */ + ldr x4, [x4] + add x4, x4, #STACK_SIZE /* (which grows down from the top). */ + sub x4, x4, #CPUINFO_sizeof /* Make room for CPU save record */ + mov sp, x4 + + /* Jump to C world */ + br x2 ENDPROC(launch) /* Fail-stop */ -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() 2019-06-10 19:32 ` [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() Julien Grall @ 2019-06-26 19:12 ` Stefano Stabellini 2019-06-26 20:09 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 19:12 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > Boot CPU and secondary CPUs will use different entry point to C code. At > the moment, the decision on which entry to use is taken within launch(). > > In order to avoid a branch for the decision and make the code clearer, > launch() is reworked to take in parameters the entry point and its > arguments. > > Lastly, document the behavior and the main registers usage within the > function. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/arm64/head.S | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 4f7fa6769f..130ab66d8e 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -312,6 +312,11 @@ primary_switched: > /* Use a virtual address to access the UART. */ > ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > #endif > + PRINT("- Ready -\r\n") > + /* Setup the arguments for start_xen and jump to C world */ > + mov x0, x20 /* x0 := phys_offset */ > + mov x1, x21 /* x1 := paddr(FDT) */ > + ldr x2, =start_xen > b launch > ENDPROC(real_start) > > @@ -374,6 +379,9 @@ secondary_switched: > /* Use a virtual address to access the UART. */ > ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > #endif > + PRINT("- Ready -\r\n") > + /* Jump to C world */ > + ldr x2, =start_secondary > b launch > ENDPROC(init_secondary) > > @@ -734,23 +742,24 @@ setup_fixmap: > ret > ENDPROC(setup_fixmap) > > +/* > + * Setup the initial stack and jump to the C world > + * > + * Inputs: > + * x0 : Argument 0 of the C function to call > + * x1 : Argument 1 of the C function to call > + * x2 : C entry point I know it is the last one before C-land, but we might as well add a "Clobbers" section too, just in case. Here it clobbers x4 (or x3, see below). > + */ > launch: > - PRINT("- Ready -\r\n") > - > - ldr x0, =init_data > - add x0, x0, #INITINFO_stack /* Find the boot-time stack */ > - ldr x0, [x0] > - add x0, x0, #STACK_SIZE /* (which grows down from the top). */ > - sub x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */ > - mov sp, x0 > - > - cbnz x22, 1f > - > - mov x0, x20 /* Marshal args: - phys_offset */ > - mov x1, x21 /* - FDT */ > - b start_xen /* and disappear into the land of C */ > -1: > - b start_secondary /* (to the appropriate entry point) */ > + ldr x4, =init_data why not use x3 instead of x4? > + add x4, x4, #INITINFO_stack /* Find the boot-time stack */ > + ldr x4, [x4] > + add x4, x4, #STACK_SIZE /* (which grows down from the top). */ If you are going to respin it, could you please align the comments a bit better (one space to the right)? > + sub x4, x4, #CPUINFO_sizeof /* Make room for CPU save record */ > + mov sp, x4 > + > + /* Jump to C world */ > + br x2 > ENDPROC(launch) > > /* Fail-stop */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() 2019-06-26 19:12 ` Stefano Stabellini @ 2019-06-26 20:09 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-26 20:09 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/26/19 8:12 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> Boot CPU and secondary CPUs will use different entry point to C code. At >> the moment, the decision on which entry to use is taken within launch(). >> >> In order to avoid a branch for the decision and make the code clearer, >> launch() is reworked to take in parameters the entry point and its >> arguments. >> >> Lastly, document the behavior and the main registers usage within the >> function. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/arm64/head.S | 41 +++++++++++++++++++++++++---------------- >> 1 file changed, 25 insertions(+), 16 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 4f7fa6769f..130ab66d8e 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -312,6 +312,11 @@ primary_switched: >> /* Use a virtual address to access the UART. */ >> ldr x23, =EARLY_UART_VIRTUAL_ADDRESS >> #endif >> + PRINT("- Ready -\r\n") >> + /* Setup the arguments for start_xen and jump to C world */ >> + mov x0, x20 /* x0 := phys_offset */ >> + mov x1, x21 /* x1 := paddr(FDT) */ >> + ldr x2, =start_xen >> b launch >> ENDPROC(real_start) >> >> @@ -374,6 +379,9 @@ secondary_switched: >> /* Use a virtual address to access the UART. */ >> ldr x23, =EARLY_UART_VIRTUAL_ADDRESS >> #endif >> + PRINT("- Ready -\r\n") >> + /* Jump to C world */ >> + ldr x2, =start_secondary >> b launch >> ENDPROC(init_secondary) >> >> @@ -734,23 +742,24 @@ setup_fixmap: >> ret >> ENDPROC(setup_fixmap) >> >> +/* >> + * Setup the initial stack and jump to the C world >> + * >> + * Inputs: >> + * x0 : Argument 0 of the C function to call >> + * x1 : Argument 1 of the C function to call >> + * x2 : C entry point > > I know it is the last one before C-land, but we might as well add a > "Clobbers" section too, just in case. Here it clobbers x4 (or x3, see > below). Sure. > > >> + */ >> launch: >> - PRINT("- Ready -\r\n") >> - >> - ldr x0, =init_data >> - add x0, x0, #INITINFO_stack /* Find the boot-time stack */ >> - ldr x0, [x0] >> - add x0, x0, #STACK_SIZE /* (which grows down from the top). */ >> - sub x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */ >> - mov sp, x0 >> - >> - cbnz x22, 1f >> - >> - mov x0, x20 /* Marshal args: - phys_offset */ >> - mov x1, x21 /* - FDT */ >> - b start_xen /* and disappear into the land of C */ >> -1: >> - b start_secondary /* (to the appropriate entry point) */ >> + ldr x4, =init_data > > why not use x3 instead of x4? I think a remnant of early rework when start_secondary was taking 3 parameters. I will update it. > > >> + add x4, x4, #INITINFO_stack /* Find the boot-time stack */ >> + ldr x4, [x4] >> + add x4, x4, #STACK_SIZE /* (which grows down from the top). */ > > If you are going to respin it, could you please align the comments a bit > better (one space to the right)? Sure. > > >> + sub x4, x4, #CPUINFO_sizeof /* Make room for CPU save record */ >> + mov sp, x4 >> + >> + /* Jump to C world */ >> + br x2 >> ENDPROC(launch) >> >> /* Fail-stop */ Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall ` (15 preceding siblings ...) 2019-06-10 19:32 ` [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() Julien Grall @ 2019-06-10 19:32 ` Julien Grall 2019-06-26 19:29 ` Stefano Stabellini 16 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-10 19:32 UTC (permalink / raw) To: xen-devel Cc: andre.przywara, Julien Grall, Stefano Stabellini, andrii_anisov, Oleksandr_Tyshchenko At the moment BSS is zeroed before the MMU and D-Cache is turned on. In other words, the cache will be bypassed when zeroing the BSS section. Per the Image protocol [1], the state of the cache for BSS region is not known because it is not part of the "loaded kernel image". This means that the cache will need to be invalidated twice for the BSS region: 1) Before zeroing to remove any dirty cache line. Otherwise they may get evicted while zeroing and therefore overriding the value. 2) After zeroing to remove any cache line that may have been speculated. Otherwise when turning on MMU and D-Cache, the CPU may see old values. However, the only reason to have the BSS zeroed early is because the boot page tables are part of BSS. To avoid the two cache invalidations, it is possible to move the page tables in the section .data.page_aligned. A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark page-tables used before BSS is zeroed. This includes all boot_* but also xen_fixmap as zero_bss() will print a message when earlyprintk is enabled. [1] linux/Documentation/arm64/booting.txt --- Note that the arm32 support is not there yet. This will need to be addressed here or separately depending on when the Arm32 boot rework is sent. --- xen/arch/arm/arm64/head.S | 6 +++--- xen/arch/arm/mm.c | 23 +++++++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 130ab66d8e..6c3edbbc81 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -291,7 +291,6 @@ real_start_efi: mov x22, #0 /* x22 := is_secondary_cpu */ bl check_cpu_mode - bl zero_bss bl cpu_init bl create_page_tables bl enable_mmu @@ -312,6 +311,7 @@ primary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif + bl zero_bss PRINT("- Ready -\r\n") /* Setup the arguments for start_xen and jump to C world */ mov x0, x20 /* x0 := phys_offset */ @@ -423,8 +423,8 @@ zero_bss: cbnz x26, skip_bss PRINT("- Zero BSS -\r\n") - load_paddr x0, __bss_start /* Load paddr of start & end of bss */ - load_paddr x1, __bss_end + ldr x0, =__bss_start /* x0 := vaddr(__bss_start) */ + ldr x1, =__bss_end /* x1 := vaddr(__bss_start) */ 1: str xzr, [x0], #8 cmp x0, x1 diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 6a549e9283..0b2d07a258 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -48,6 +48,17 @@ #undef mfn_to_virt #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn)) +/* + * Macros to define page-tables: + * - DEFINE_BOOT_PAGE_TABLE is used to define page-table that are used + * in assembly code before BSS is zeroed. + * - DEFINE_PAGE_TABLE{,S} are used to define one or multiple + * page-tables to be used after BSS is zeroed (typically they are only used + * in C). + */ +#define DEFINE_BOOT_PAGE_TABLE(name) \ +lpae_t __aligned(PAGE_SIZE) __section(".data.page_aligned") name[LPAE_ENTRIES] + #define DEFINE_PAGE_TABLES(name, nr) \ lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)] @@ -76,13 +87,13 @@ lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)] * Finally, if EARLY_PRINTK is enabled then xen_fixmap will be mapped * by the CPU once it has moved off the 1:1 mapping. */ -DEFINE_PAGE_TABLE(boot_pgtable); +DEFINE_BOOT_PAGE_TABLE(boot_pgtable); #ifdef CONFIG_ARM_64 -DEFINE_PAGE_TABLE(boot_first); -DEFINE_PAGE_TABLE(boot_first_id); +DEFINE_BOOT_PAGE_TABLE(boot_first); +DEFINE_BOOT_PAGE_TABLE(boot_first_id); #endif -DEFINE_PAGE_TABLE(boot_second); -DEFINE_PAGE_TABLE(boot_third); +DEFINE_BOOT_PAGE_TABLE(boot_second); +DEFINE_BOOT_PAGE_TABLE(boot_third); /* Main runtime page tables */ @@ -135,7 +146,7 @@ static __initdata int xenheap_first_first_slot = -1; */ static DEFINE_PAGE_TABLES(xen_second, 2); /* First level page table used for fixmap */ -DEFINE_PAGE_TABLE(xen_fixmap); +DEFINE_BOOT_PAGE_TABLE(xen_fixmap); /* First level page table used to map Xen itself with the XN bit set * as appropriate. */ static DEFINE_PAGE_TABLE(xen_xenmap); -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on 2019-06-10 19:32 ` [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on Julien Grall @ 2019-06-26 19:29 ` Stefano Stabellini 2019-06-26 20:07 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 19:29 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Mon, 10 Jun 2019, Julien Grall wrote: > At the moment BSS is zeroed before the MMU and D-Cache is turned on. > In other words, the cache will be bypassed when zeroing the BSS section. > > Per the Image protocol [1], the state of the cache for BSS region is not > known because it is not part of the "loaded kernel image". > > This means that the cache will need to be invalidated twice for the BSS > region: > 1) Before zeroing to remove any dirty cache line. Otherwise they may > get evicted while zeroing and therefore overriding the value. > 2) After zeroing to remove any cache line that may have been > speculated. Otherwise when turning on MMU and D-Cache, the CPU may > see old values. > > However, the only reason to have the BSS zeroed early is because the > boot page tables are part of BSS. To avoid the two cache invalidations, > it is possible to move the page tables in the section .data.page_aligned. I am not following the last part. How is moving the boot page tables to .data.page_aligned solving the problem? Do we need to zero .data.page_aligned early or can we skip it because it is guaranteed to already be zero? > A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark > page-tables used before BSS is zeroed. This includes all boot_* but also > xen_fixmap as zero_bss() will print a message when earlyprintk is > enabled. On a similar note, and continuing from what I wrote above, do we need to make sure to zero the xen_fixmap before hooking it up setup_fixmap? > [1] linux/Documentation/arm64/booting.txt > > --- > > Note that the arm32 support is not there yet. This will need to be > addressed here or separately depending on when the Arm32 boot rework > is sent. > --- > xen/arch/arm/arm64/head.S | 6 +++--- > xen/arch/arm/mm.c | 23 +++++++++++++++++------ > 2 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 130ab66d8e..6c3edbbc81 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -291,7 +291,6 @@ real_start_efi: > mov x22, #0 /* x22 := is_secondary_cpu */ > > bl check_cpu_mode > - bl zero_bss > bl cpu_init > bl create_page_tables > bl enable_mmu > @@ -312,6 +311,7 @@ primary_switched: > /* Use a virtual address to access the UART. */ > ldr x23, =EARLY_UART_VIRTUAL_ADDRESS > #endif > + bl zero_bss > PRINT("- Ready -\r\n") > /* Setup the arguments for start_xen and jump to C world */ > mov x0, x20 /* x0 := phys_offset */ > @@ -423,8 +423,8 @@ zero_bss: > cbnz x26, skip_bss > > PRINT("- Zero BSS -\r\n") > - load_paddr x0, __bss_start /* Load paddr of start & end of bss */ > - load_paddr x1, __bss_end > + ldr x0, =__bss_start /* x0 := vaddr(__bss_start) */ > + ldr x1, =__bss_end /* x1 := vaddr(__bss_start) */ > > 1: str xzr, [x0], #8 > cmp x0, x1 > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 6a549e9283..0b2d07a258 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -48,6 +48,17 @@ > #undef mfn_to_virt > #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn)) > > +/* > + * Macros to define page-tables: > + * - DEFINE_BOOT_PAGE_TABLE is used to define page-table that are used > + * in assembly code before BSS is zeroed. > + * - DEFINE_PAGE_TABLE{,S} are used to define one or multiple > + * page-tables to be used after BSS is zeroed (typically they are only used > + * in C). > + */ > +#define DEFINE_BOOT_PAGE_TABLE(name) \ > +lpae_t __aligned(PAGE_SIZE) __section(".data.page_aligned") name[LPAE_ENTRIES] > + > #define DEFINE_PAGE_TABLES(name, nr) \ > lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)] > > @@ -76,13 +87,13 @@ lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)] > * Finally, if EARLY_PRINTK is enabled then xen_fixmap will be mapped > * by the CPU once it has moved off the 1:1 mapping. > */ > -DEFINE_PAGE_TABLE(boot_pgtable); > +DEFINE_BOOT_PAGE_TABLE(boot_pgtable); > #ifdef CONFIG_ARM_64 > -DEFINE_PAGE_TABLE(boot_first); > -DEFINE_PAGE_TABLE(boot_first_id); > +DEFINE_BOOT_PAGE_TABLE(boot_first); > +DEFINE_BOOT_PAGE_TABLE(boot_first_id); > #endif > -DEFINE_PAGE_TABLE(boot_second); > -DEFINE_PAGE_TABLE(boot_third); > +DEFINE_BOOT_PAGE_TABLE(boot_second); > +DEFINE_BOOT_PAGE_TABLE(boot_third); > > /* Main runtime page tables */ > > @@ -135,7 +146,7 @@ static __initdata int xenheap_first_first_slot = -1; > */ > static DEFINE_PAGE_TABLES(xen_second, 2); > /* First level page table used for fixmap */ > -DEFINE_PAGE_TABLE(xen_fixmap); > +DEFINE_BOOT_PAGE_TABLE(xen_fixmap); > /* First level page table used to map Xen itself with the XN bit set > * as appropriate. */ > static DEFINE_PAGE_TABLE(xen_xenmap); > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on 2019-06-26 19:29 ` Stefano Stabellini @ 2019-06-26 20:07 ` Julien Grall 2019-06-26 21:08 ` Stefano Stabellini 0 siblings, 1 reply; 70+ messages in thread From: Julien Grall @ 2019-06-26 20:07 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 6/26/19 8:29 PM, Stefano Stabellini wrote: > On Mon, 10 Jun 2019, Julien Grall wrote: >> At the moment BSS is zeroed before the MMU and D-Cache is turned on. >> In other words, the cache will be bypassed when zeroing the BSS section. >> >> Per the Image protocol [1], the state of the cache for BSS region is not >> known because it is not part of the "loaded kernel image". >> >> This means that the cache will need to be invalidated twice for the BSS >> region: >> 1) Before zeroing to remove any dirty cache line. Otherwise they may >> get evicted while zeroing and therefore overriding the value. >> 2) After zeroing to remove any cache line that may have been >> speculated. Otherwise when turning on MMU and D-Cache, the CPU may >> see old values. >> >> However, the only reason to have the BSS zeroed early is because the >> boot page tables are part of BSS. To avoid the two cache invalidations, >> it is possible to move the page tables in the section .data.page_aligned. > > I am not following the last part. How is moving the boot page tables to > .data.page_aligned solving the problem? Do we need to zero > .data.page_aligned early or can we skip it because it is guaranteed to > already be zero? Global variables are initialized to zero by default regardless the section they reside. Usually they are stored in BSS because it saves space in the binary. With the Image protocol, BSS is not initialized by the bootloader so we have to do ourself. The section .data.page_aligned is always part of the binary. So the compiler will write zero in the binary for any global variables part of that section and therefore the corresponding memory will be zeroed when loading the binary. If it makes sense, I can try to incorporate that in the commit message. > > >> A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark >> page-tables used before BSS is zeroed. This includes all boot_* but also >> xen_fixmap as zero_bss() will print a message when earlyprintk is >> enabled. > > On a similar note, and continuing from what I wrote above, do we need to > make sure to zero the xen_fixmap before hooking it up setup_fixmap? See above. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on 2019-06-26 20:07 ` Julien Grall @ 2019-06-26 21:08 ` Stefano Stabellini 2019-06-27 11:04 ` Julien Grall 0 siblings, 1 reply; 70+ messages in thread From: Stefano Stabellini @ 2019-06-26 21:08 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, Oleksandr_Tyshchenko, Stefano Stabellini, andrii_anisov, andre.przywara On Wed, 26 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 6/26/19 8:29 PM, Stefano Stabellini wrote: > > On Mon, 10 Jun 2019, Julien Grall wrote: > > > At the moment BSS is zeroed before the MMU and D-Cache is turned on. > > > In other words, the cache will be bypassed when zeroing the BSS section. > > > > > > Per the Image protocol [1], the state of the cache for BSS region is not > > > known because it is not part of the "loaded kernel image". > > > > > > This means that the cache will need to be invalidated twice for the BSS > > > region: > > > 1) Before zeroing to remove any dirty cache line. Otherwise they may > > > get evicted while zeroing and therefore overriding the value. > > > 2) After zeroing to remove any cache line that may have been > > > speculated. Otherwise when turning on MMU and D-Cache, the CPU may > > > see old values. > > > > > > However, the only reason to have the BSS zeroed early is because the > > > boot page tables are part of BSS. To avoid the two cache invalidations, > > > it is possible to move the page tables in the section .data.page_aligned. > > > > I am not following the last part. How is moving the boot page tables to > > .data.page_aligned solving the problem? Do we need to zero > > .data.page_aligned early or can we skip it because it is guaranteed to > > already be zero? > > Global variables are initialized to zero by default regardless the section > they reside. Usually they are stored in BSS because it saves space in the > binary. > > With the Image protocol, BSS is not initialized by the bootloader so we have > to do ourself. > > The section .data.page_aligned is always part of the binary. So the compiler > will write zero in the binary for any global variables part of that section > and therefore the corresponding memory will be zeroed when loading the binary. > > If it makes sense, I can try to incorporate that in the commit message. So basically it is really only BSS the problem. All right, looks good to me. Acked-by: Stefano Stabellini <sstabellini@kernel.org> > > > A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark > > > page-tables used before BSS is zeroed. This includes all boot_* but also > > > xen_fixmap as zero_bss() will print a message when earlyprintk is > > > enabled. > > > > On a similar note, and continuing from what I wrote above, do we need to > > make sure to zero the xen_fixmap before hooking it up setup_fixmap? > > See above. > > Cheers, > > -- > Julien Grall > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on 2019-06-26 21:08 ` Stefano Stabellini @ 2019-06-27 11:04 ` Julien Grall 0 siblings, 0 replies; 70+ messages in thread From: Julien Grall @ 2019-06-27 11:04 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel, Oleksandr_Tyshchenko, andrii_anisov, andre.przywara Hi Stefano, On 26/06/2019 22:08, Stefano Stabellini wrote: > On Wed, 26 Jun 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 6/26/19 8:29 PM, Stefano Stabellini wrote: >>> On Mon, 10 Jun 2019, Julien Grall wrote: >>>> At the moment BSS is zeroed before the MMU and D-Cache is turned on. >>>> In other words, the cache will be bypassed when zeroing the BSS section. >>>> >>>> Per the Image protocol [1], the state of the cache for BSS region is not >>>> known because it is not part of the "loaded kernel image". >>>> >>>> This means that the cache will need to be invalidated twice for the BSS >>>> region: >>>> 1) Before zeroing to remove any dirty cache line. Otherwise they may >>>> get evicted while zeroing and therefore overriding the value. >>>> 2) After zeroing to remove any cache line that may have been >>>> speculated. Otherwise when turning on MMU and D-Cache, the CPU may >>>> see old values. >>>> >>>> However, the only reason to have the BSS zeroed early is because the >>>> boot page tables are part of BSS. To avoid the two cache invalidations, >>>> it is possible to move the page tables in the section .data.page_aligned. >>> >>> I am not following the last part. How is moving the boot page tables to >>> .data.page_aligned solving the problem? Do we need to zero >>> .data.page_aligned early or can we skip it because it is guaranteed to >>> already be zero? >> >> Global variables are initialized to zero by default regardless the section >> they reside. Usually they are stored in BSS because it saves space in the >> binary. >> >> With the Image protocol, BSS is not initialized by the bootloader so we have >> to do ourself. >> >> The section .data.page_aligned is always part of the binary. So the compiler >> will write zero in the binary for any global variables part of that section >> and therefore the corresponding memory will be zeroed when loading the binary. >> >> If it makes sense, I can try to incorporate that in the commit message. > > So basically it is really only BSS the problem. All right, looks good to > me. Yes that's correct. > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> Thank you! Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 70+ messages in thread
end of thread, other threads:[~2019-07-31 21:07 UTC | newest] Thread overview: 70+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-10 19:31 [Xen-devel] [PATCH 00/17] xen/arm64: Rework head.S to make it more compliant with the Arm Arm Julien Grall 2019-06-10 19:31 ` [Xen-devel] [PATCH 01/17] xen/arm64: head Mark the end of subroutines with ENDPROC Julien Grall 2019-06-25 23:23 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT Julien Grall 2019-06-25 23:35 ` Stefano Stabellini 2019-06-25 23:59 ` Stefano Stabellini 2019-06-26 9:07 ` Julien Grall 2019-06-26 15:27 ` Stefano Stabellini 2019-06-26 15:28 ` Julien Grall 2019-06-26 18:32 ` Stefano Stabellini 2019-06-26 19:24 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU Julien Grall 2019-06-25 23:49 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 04/17] xen/arm64: head: Don't "reserve" x24 for the CPUID Julien Grall 2019-06-26 0:01 ` Stefano Stabellini 2019-06-26 9:09 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg Julien Grall 2019-06-26 0:09 ` Stefano Stabellini 2019-06-26 9:10 ` Julien Grall 2019-07-15 18:46 ` Volodymyr Babchuk 2019-07-16 9:55 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs Julien Grall 2019-06-26 1:00 ` Stefano Stabellini 2019-06-26 9:14 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 07/17] xen/arm64: head: Rework and document check_cpu_mode() Julien Grall 2019-06-26 1:00 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss() Julien Grall 2019-06-26 1:01 ` Stefano Stabellini 2019-06-26 9:16 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 09/17] xen/arm64: head: Improve coding style and document cpu_init() Julien Grall 2019-06-26 1:01 ` Stefano Stabellini 2019-06-26 10:34 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 10/17] xen/arm64: head: Improve coding style and document create_pages_tables() Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-26 11:20 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 11/17] xen/arm64: head: Document enable_mmu() Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-26 11:23 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 12/17] xen/arm64: head: Move assembly switch to the runtime PT in secondary CPUs path Julien Grall 2019-06-26 1:03 ` Stefano Stabellini 2019-06-10 19:32 ` [Xen-devel] [PATCH 13/17] xen/arm64: head: Don't setup the fixmap on secondary CPUs Julien Grall 2019-06-26 18:51 ` Stefano Stabellini 2019-06-26 19:26 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used Julien Grall 2019-06-26 20:25 ` Stefano Stabellini 2019-06-26 20:39 ` Julien Grall 2019-06-26 20:44 ` Andrew Cooper 2019-06-28 0:36 ` Stefano Stabellini 2019-06-27 18:55 ` Stefano Stabellini 2019-06-27 19:30 ` Julien Grall 2019-07-10 19:39 ` Julien Grall 2019-07-30 17:33 ` Stefano Stabellini 2019-07-30 19:52 ` Julien Grall 2019-07-31 20:40 ` Stefano Stabellini 2019-07-31 21:07 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 15/17] xen/arm64: head: Rework and document setup_fixmap() Julien Grall 2019-06-26 19:01 ` Stefano Stabellini 2019-06-26 19:30 ` Julien Grall 2019-06-27 9:29 ` Julien Grall 2019-06-27 15:38 ` Stefano Stabellini 2019-06-26 19:02 ` Stefano Stabellini 2019-06-27 9:19 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 16/17] xen/arm64: head: Rework and document launch() Julien Grall 2019-06-26 19:12 ` Stefano Stabellini 2019-06-26 20:09 ` Julien Grall 2019-06-10 19:32 ` [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on Julien Grall 2019-06-26 19:29 ` Stefano Stabellini 2019-06-26 20:07 ` Julien Grall 2019-06-26 21:08 ` Stefano Stabellini 2019-06-27 11:04 ` Julien Grall
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.