[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 07/52] xen/arm64: prepare for moving MMU related code from head.S



Hi Penny,

On 26/06/2023 04:33, Penny Zheng wrote:
From: Wei Chen <wei.chen@xxxxxxx>

We want to reuse head.S for MPU systems, but there are some
code are implemented for MMU systems only. We will move such
code to another MMU specific file. But before that we will
do some indentations fix in this patch to make them be easier
for reviewing:
1. Fix the indentations of code comments.
2. Fix the indentations for .text.header section.
3. Rename puts() to asm_puts() for global export

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
---
v1 -> v2:
1. New patch.
---
v3:
1. fix commit message
2. Rename puts() to asm_puts() for global export
---
  xen/arch/arm/arm64/head.S | 42 +++++++++++++++++++--------------------
  1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 4dfbe0bc6f..66347eedcc 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -94,7 +94,7 @@
  #define PRINT(_s)          \
          mov   x3, lr ;     \
          adr   x0, 98f ;    \

This was recently changed to adr_l in staging. So you will need to rebase this patch.

-        bl    puts    ;    \
+        bl    asm_puts ;   \
          mov   lr, x3 ;     \
          RODATA_STR(98, _s)
@@ -136,22 +136,22 @@
          add \xb, \xb, x20
  .endm
- .section .text.header, "ax", %progbits
-        /*.aarch64*/
+.section .text.header, "ax", %progbits
+/*.aarch64*/
- /*
-         * Kernel startup entry point.
-         * ---------------------------
-         *
-         * The requirements are:
-         *   MMU = off, D-cache = off, I-cache = on or off,
-         *   x0 = physical address to the FDT blob.
-         *
-         * This must be the very first address in the loaded image.
-         * It should be linked at XEN_VIRT_START, and loaded at any
-         * 4K-aligned address.  All of text+data+bss must fit in 2MB,
-         * or the initial pagetable code below will need adjustment.
-         */
+/*
+ * Kernel startup entry point.
+ * ---------------------------
+ *
+ * The requirements are:
+ *   MMU = off, D-cache = off, I-cache = on or off,
+ *   x0 = physical address to the FDT blob.
+ *
+ * This must be the very first address in the loaded image.
+ * It should be linked at XEN_VIRT_START, and loaded at any
+ * 4K-aligned address.  All of text+data+bss must fit in 2MB,
+ * or the initial pagetable code below will need adjustment.
+ */
GLOBAL(start)
          /*
@@ -520,7 +520,7 @@ ENDPROC(cpu_init)
   * Macro to create a mapping entry in \tbl to \phys. Only mapping in 3rd
   * level table (i.e page granularity) is supported.
   *
- * ptbl:     table symbol where the entry will be created
+ * ptbl:    table symbol where the entry will be created
   * virt:    virtual address
   * phys:    physical address (should be page aligned)
   * tmp1:    scratch register
@@ -928,15 +928,15 @@ ENDPROC(init_uart)
   * x0: Nul-terminated string to print.
   * x23: Early UART base address
   * Clobbers x0-x1 */
-puts:
+ENTRY(asm_puts)

Can you mention in the comment that this function is only supposed to be called from assembly?

While you are at it, can you update the comment coding style to:

/*
 * Foo
 * Bar
 */

          early_uart_ready x23, 1
          ldrb  w1, [x0], #1           /* Load next char */
          cbz   w1, 1f                 /* Exit on nul */
          early_uart_transmit x23, w1
-        b     puts
+        b     asm_puts
  1:
          ret
-ENDPROC(puts)
+ENDPROC(asm_puts)
/*
   * Print a 64-bit number in hex.
@@ -966,7 +966,7 @@ hex:    .ascii "0123456789abcdef"
ENTRY(early_puts)
  init_uart:
-puts:
+asm_puts:

I find odd that you add ENTRY() to the asm_puts() above but not here. However... I can't find any use of asm_puts() outside #ifdef CONFIG_EARLY_PRINTK. So maybe it can be dropped?


  putn:   ret
#endif /* !CONFIG_EARLY_PRINTK */

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.