diff mbox series

[2/3] xen/arm64: Move print_reg macro to asm/arm64/macros.h

Message ID 20231121094516.24714-3-michal.orzel@amd.com (mailing list archive)
State New, archived
Headers show
Series xen/arm64: head: misc cleanup | expand

Commit Message

Michal Orzel Nov. 21, 2023, 9:45 a.m. UTC
Macro print_reg is used to print a value of a register passed as an
argument. While today it is only used from within the common head.S,
in the future we might want to make use of it from other files, just
like PRINT(). It also serves as a great aid when debugging.

Expose print_reg macro by moving it to asm/arm64/macros.h and:
 - rename putn to asm_putn to denote the usage from assembly only,
 - use ENTRY() for asm_putn to make it globally visible,
 - get rid of unneeded stubs for early_puts, init_uart and putn since
   the calls to them are already protected by #ifdef CONFIG_EARLY_PRINTK.

This way the behavior will be consistent with what we already do on arm32.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/arm64/head.S               | 32 ++++---------------------
 xen/arch/arm/include/asm/arm64/macros.h | 15 ++++++++++++
 2 files changed, 19 insertions(+), 28 deletions(-)

Comments

Luca Fancellu Nov. 21, 2023, 4:05 p.m. UTC | #1
> On 21 Nov 2023, at 09:45, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> Macro print_reg is used to print a value of a register passed as an
> argument. While today it is only used from within the common head.S,
> in the future we might want to make use of it from other files, just
> like PRINT(). It also serves as a great aid when debugging.
> 
> Expose print_reg macro by moving it to asm/arm64/macros.h and:
> - rename putn to asm_putn to denote the usage from assembly only,
> - use ENTRY() for asm_putn to make it globally visible,
> - get rid of unneeded stubs for early_puts, init_uart and putn since
>   the calls to them are already protected by #ifdef CONFIG_EARLY_PRINTK.
> 
> This way the behavior will be consistent with what we already do on arm32.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Hi Michal,

I’ve also tested on FVP

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Julien Grall Nov. 21, 2023, 4:16 p.m. UTC | #2
On 21/11/2023 09:45, Michal Orzel wrote:
> Macro print_reg is used to print a value of a register passed as an
> argument. While today it is only used from within the common head.S,
> in the future we might want to make use of it from other files, just
> like PRINT(). It also serves as a great aid when debugging.
> 
> Expose print_reg macro by moving it to asm/arm64/macros.h and:
>   - rename putn to asm_putn to denote the usage from assembly only,
>   - use ENTRY() for asm_putn to make it globally visible,
>   - get rid of unneeded stubs for early_puts, init_uart and putn since
>     the calls to them are already protected by #ifdef CONFIG_EARLY_PRINTK.

NIT: The last one read as this is necessary to move print_reg() to 
asm/arm64/macros. But really, this is just a clean-up. So I would add 
something like "Take the opportunity to..." or similar to make it clearer.

Other than that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,
Michal Orzel Nov. 21, 2023, 5:02 p.m. UTC | #3
Hi Julien,

On 21/11/2023 17:16, Julien Grall wrote:
> 
> 
> On 21/11/2023 09:45, Michal Orzel wrote:
>> Macro print_reg is used to print a value of a register passed as an
>> argument. While today it is only used from within the common head.S,
>> in the future we might want to make use of it from other files, just
>> like PRINT(). It also serves as a great aid when debugging.
>>
>> Expose print_reg macro by moving it to asm/arm64/macros.h and:
>>   - rename putn to asm_putn to denote the usage from assembly only,
>>   - use ENTRY() for asm_putn to make it globally visible,
>>   - get rid of unneeded stubs for early_puts, init_uart and putn since
>>     the calls to them are already protected by #ifdef CONFIG_EARLY_PRINTK.
> 
> NIT: The last one read as this is necessary to move print_reg() to
> asm/arm64/macros. But really, this is just a clean-up. So I would add
> something like "Take the opportunity to..." or similar to make it clearer.
Ok, I can do that.

~Michal
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index b6111399e766..a3dbf81ab515 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -74,25 +74,6 @@ 
  *  x30 - lr
  */
 
- #ifdef CONFIG_EARLY_PRINTK
-/*
- * Macro to print the value of register \xb
- *
- * Clobbers x0 - x4
- */
-.macro print_reg xb
-        mov   x0, \xb
-        mov   x4, lr
-        bl    putn
-        mov   lr, x4
-.endm
-
-#else /* CONFIG_EARLY_PRINTK */
-.macro print_reg xb
-.endm
-
-#endif /* !CONFIG_EARLY_PRINTK */
-
 .section .text.header, "ax", %progbits
 /*.aarch64*/
 
@@ -493,11 +474,12 @@  ENDPROC(asm_puts)
 
 /*
  * Print a 64-bit number in hex.
+ * Note: This function must be called from assembly.
  * x0: Number to print.
  * x23: Early UART base address
  * Clobbers x0-x3
  */
-putn:
+ENTRY(asm_putn)
         adr_l x1, hex
         mov   x3, #16
 1:
@@ -510,17 +492,11 @@  putn:
         subs  x3, x3, #1
         b.ne  1b
         ret
-ENDPROC(putn)
+ENDPROC(asm_putn)
 
 RODATA_STR(hex, "0123456789abcdef")
 
-#else  /* CONFIG_EARLY_PRINTK */
-
-ENTRY(early_puts)
-init_uart:
-putn:   ret
-
-#endif /* !CONFIG_EARLY_PRINTK */
+#endif /* CONFIG_EARLY_PRINTK */
 
 /* This provides a C-API version of __lookup_processor_type
  * TODO: For now, the implementation return NULL every time
diff --git a/xen/arch/arm/include/asm/arm64/macros.h b/xen/arch/arm/include/asm/arm64/macros.h
index fb9a0602494d..d108dc3a3a71 100644
--- a/xen/arch/arm/include/asm/arm64/macros.h
+++ b/xen/arch/arm/include/asm/arm64/macros.h
@@ -45,9 +45,24 @@ 
         mov   lr, x3 ;     \
         RODATA_STR(98, _s)
 
+/*
+ * Macro to print the value of register \xb
+ *
+ * Clobbers x0 - x4
+ */
+.macro print_reg xb
+        mov   x0, \xb
+        mov   x4, lr
+        bl    asm_putn
+        mov   lr, x4
+.endm
+
 #else /* CONFIG_EARLY_PRINTK */
 #define PRINT(s)
 
+.macro print_reg xb
+.endm
+
 #endif /* !CONFIG_EARLY_PRINTK */
 
 /*