diff mbox series

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

Message ID 20240116143709.86584-4-julien@xen.org (mailing list archive)
State New
Headers show
Series xen/arm64: Rework the MMU-off code (idmap) so it is self-contained | expand

Commit Message

Julien Grall Jan. 16, 2024, 2:37 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

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@amazon.com>
---
 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(-)

Comments

Michal Orzel Jan. 17, 2024, 8:53 a.m. UTC | #1
On 16/01/2024 15:37, Julien Grall wrote:
> 
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> 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@amazon.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

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
Julien Grall Jan. 18, 2024, 9:23 a.m. UTC | #2
Hi Michal,

On 17/01/2024 08:53, Michal Orzel wrote:
> 
> 
> On 16/01/2024 15:37, Julien Grall wrote:
>>
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> 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@amazon.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> 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?

It should be modified. I will update it on commit.

> 
>>   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

I will fix it.

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

Cheers,
diff mbox series

Patch

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
 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
+ *        physically contiguous in memory (e.g. in the case of cache
+ *        coloring).
  *
  * Clobbers x0 - x3
  */