Re: [PATCH v1 repost 3/4] xen/arm64: head: Use PRINT_ID() for secondary CPU MMU-off boot code

[Michal Orzel <michal.orzel@xxxxxxx>]

On 16/01/2024 15:37, Julien Grall wrote:
> 
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> With the upcoming work to color Xen, the binary will not be anymore
> physically contiguous. This will be a problem during boot as the
> assembly code will need to work out where each piece of Xen reside.
> 
> An easy way to solve the issue is to have all code/data accessed
> by the secondary CPUs while the MMU is off within a single page.
> 
> Right now, most of the early printk messages are using PRINT() which
> will add the message in .rodata. This is unlikely to be within the
> same page as the rest of the idmap.
> 
> So replace all the PRINT() that can be reachable by the secondary
> CPU with MMU-off with PRINT_ID().
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

with some remarks below

> ---
>  xen/arch/arm/arm64/head.S               | 14 +++++++-------
>  xen/arch/arm/arm64/mmu/head.S           |  2 +-
>  xen/arch/arm/include/asm/arm64/macros.h |  9 ++++++---
>  3 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index cfc04c755400..fa8b00b6f1db 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -289,9 +289,9 @@ GLOBAL(init_secondary)
> 
>  #ifdef CONFIG_EARLY_PRINTK
>          ldr   x23, =CONFIG_EARLY_UART_BASE_ADDRESS /* x23 := UART base 
> address */
> -        PRINT("- CPU ")
> +        PRINT_ID("- CPU ")
>          print_reg x24
> -        PRINT(" booting -\r\n")
> +        PRINT_ID(" booting -\r\n")
>  #endif
>          bl    check_cpu_mode
>          bl    cpu_init
> @@ -314,10 +314,10 @@ ENDPROC(init_secondary)
>   * Clobbers x0 - x5
>   */
>  check_cpu_mode:
> -        PRINT("- Current EL ")
> +        PRINT_ID("- Current EL ")
>          mrs   x5, CurrentEL
>          print_reg x5
> -        PRINT(" -\r\n")
> +        PRINT_ID(" -\r\n")
> 
>          /* Are we in EL2 */
>          cmp   x5, #PSR_MODE_EL2t
> @@ -326,8 +326,8 @@ check_cpu_mode:
>          ret
>  1:
>          /* OK, we're boned. */
> -        PRINT("- Xen must be entered in NS EL2 mode -\r\n")
> -        PRINT("- Please update the bootloader -\r\n")
> +        PRINT_ID("- Xen must be entered in NS EL2 mode -\r\n")
> +        PRINT_ID("- Please update the bootloader -\r\n")
>          b fail
What about the PRINT in fail?

>  ENDPROC(check_cpu_mode)
> 
> @@ -361,7 +361,7 @@ ENDPROC(zero_bss)
>   * Clobbers x0 - x3
>   */
>  cpu_init:
> -        PRINT("- Initialize CPU -\r\n")
> +        PRINT_ID("- Initialize CPU -\r\n")
> 
>          /* Set up memory attribute type tables */
>          ldr   x0, =MAIRVAL
> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> index 92b62ae94ce5..fa40b696ddc8 100644
> --- a/xen/arch/arm/arm64/mmu/head.S
> +++ b/xen/arch/arm/arm64/mmu/head.S
> @@ -276,7 +276,7 @@ ENDPROC(create_page_tables)
>  enable_mmu:
>          mov   x4, x0
>          mov   x5, x1
> -        PRINT("- Turning on paging -\r\n")
> +        PRINT_ID("- Turning on paging -\r\n")
> 
>          /*
>           * The state of the TLBs is unknown before turning on the MMU.
> diff --git a/xen/arch/arm/include/asm/arm64/macros.h 
> b/xen/arch/arm/include/asm/arm64/macros.h
> index 10e652041f57..6a0108f778a2 100644
> --- a/xen/arch/arm/include/asm/arm64/macros.h
> +++ b/xen/arch/arm/include/asm/arm64/macros.h
> @@ -39,9 +39,12 @@
>   * There are multiple flavors:
>   *  - PRINT_SECT(section, string): The @string will be located in @section
>   *  - PRINT(): The string will be located in .rodata.str.
> - *  - PRINT_ID(): When Xen is running on the Identity Mapping, it is
> - *    only possible to have a limited amount of Xen. This will create
> - *    the string in .rodata.idmap which will always be mapped.
> + *  - PRINT_ID(): This will create the string in .rodata.idmap which
> + *    will always be accessible. This is used when:
> + *      - Xen is running on the identity mapping because not all of Xen is 
> mapped
> + *      - Running with the MMU-off on secondary boots as Xen may not be
s/boots/CPUs

> + *        physically contiguous in memory (e.g. in the case of cache
> + *        coloring).
>   *
>   * Clobbers x0 - x3
>   */
> --
> 2.40.1
> 

~Michal