[v2,05/40] xen/arm64: prepare for moving MMU related code from head.S

Commit Message

Penny Zheng Jan. 13, 2023, 5:28 a.m. UTC

From: Wei Chen <[e-mail]>

We want to reuse head.S for MPU systems, but there are some
code implemented for MMU systems only. We will move such
code to another MMU specific file. But before that, we will
do some preparations in this patch to make them easier
for reviewing:
1. Fix the indentations of code comments.
2. Export some symbols that will be accessed out of file
   scope.

Signed-off-by: Wei Chen <[e-mail]>
---
v1 -> v2:
1. New patch.
---
 xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Comments

Julien Grall Jan. 17, 2023, 11:37 p.m. UTC | #1

Hi Penny,

On 13/01/2023 05:28, Penny Zheng wrote:
> From: Wei Chen <[e-mail]>
> 
> We want to reuse head.S for MPU systems, but there are some
> code implemented for MMU systems only. We will move such
> code to another MMU specific file. But before that, we will
> do some preparations in this patch to make them easier
> for reviewing:

Well, I agree that...

> 1. Fix the indentations of code comments.

... changing the indentation is better here. But...

> 2. Export some symbols that will be accessed out of file
>     scope.

... I have no idea which functions are going to be used in a separate 
file. So I think they should belong to the patch moving the code.

> 
> Signed-off-by: Wei Chen <[e-mail]>

Your signed-off-by is missing.

> ---
> v1 -> v2:
> 1. New patch.
> ---
>   xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++--------------------
>   1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 93f9b0b9d5..b2214bc5e3 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -136,22 +136,22 @@
>           add \xb, \xb, x20
>   .endm
>   
> -        .section .text.header, "ax", %progbits
> -        /*.aarch64*/
> +.section .text.header, "ax", %progbits
> +/*.aarch64*/

This change is not mentioned.

>   
> -        /*
> -         * 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)
>           /*
> @@ -586,7 +586,7 @@ ENDPROC(cpu_init)
>    *
>    * Clobbers x0 - x4
>    */
> -create_page_tables:
> +ENTRY(create_page_tables)

I am not sure about keeping this name. Now we have create_page_tables() 
and arch_setup_page_tables().

I would conside to name it create_boot_page_tables().

>           /* Prepare the page-tables for mapping Xen */
>           ldr   x0, =XEN_VIRT_START
>           create_table_entry boot_pgtable, boot_first, x0, 0, x1, x2, x3
> @@ -680,7 +680,7 @@ ENDPROC(create_page_tables)
>    *
>    * Clobbers x0 - x3
>    */
> -enable_mmu:
> +ENTRY(enable_mmu)
>           PRINT("- Turning on paging -\r\n")
>   
>           /*
> @@ -714,7 +714,7 @@ ENDPROC(enable_mmu)
>    *
>    * Clobbers x0 - x1
>    */
> -remove_identity_mapping:
> +ENTRY(remove_identity_mapping)

Patch #14 should be before this patch. So you don't have to export 
remove_identity_mapping temporarily.

This will also avoid (transient) naming confusing with my work (see [1]).

>           /*
>            * Find the zeroeth slot used. Remove the entry from zeroeth
>            * table if the slot is not XEN_ZEROETH_SLOT.
> @@ -775,7 +775,7 @@ ENDPROC(remove_identity_mapping)
>    *
>    * Clobbers x0 - x3
>    */
> -setup_fixmap:
> +ENTRY(setup_fixmap)
>   #ifdef CONFIG_EARLY_PRINTK
>           /* Add UART to the fixmap table */
>           ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
> @@ -871,7 +871,7 @@ ENDPROC(init_uart)
>    * x0: Nul-terminated string to print.
>    * x23: Early UART base address
>    * Clobbers x0-x1 */
> -puts:
> +ENTRY(puts)

This name is a bit too generic to be globally exported. It is also now 
quite confusing because we have "early_puts" and "puts".

I would consider to name it asm_puts(). It is still not great but 
hopefully it would give a hint that should be call from assembly code.

>           early_uart_ready x23, 1
>           ldrb  w1, [x0], #1           /* Load next char */
>           cbz   w1, 1f                 /* Exit on nul */

Cheers,

[1] https://lore.kernel.org/all/[e-mail]/

Wei Chen Jan. 18, 2023, 3:09 a.m. UTC | #2

Hi Julien,

> -----Original Message-----
> From: Julien Grall <[e-mail]>
> Sent: 2023年1月18日 7:37
> To: Penny Zheng <[e-mail]>; [e-mail]
> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
> <[e-mail]>; Bertrand Marquis <[e-mail]>;
> Volodymyr Babchuk <[e-mail]>
> Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related
> code from head.S
> 
> Hi Penny,
> 
> On 13/01/2023 05:28, Penny Zheng wrote:
> > From: Wei Chen <[e-mail]>
> >
> > We want to reuse head.S for MPU systems, but there are some
> > code implemented for MMU systems only. We will move such
> > code to another MMU specific file. But before that, we will
> > do some preparations in this patch to make them easier
> > for reviewing:
> 
> Well, I agree that...
> 
> > 1. Fix the indentations of code comments.
> 
> ... changing the indentation is better here. But...
> 
> > 2. Export some symbols that will be accessed out of file
> >     scope.
> 
> ... I have no idea which functions are going to be used in a separate
> file. So I think they should belong to the patch moving the code.
> 

Ok, I will move these changes to the moving code patches.

> >
> > Signed-off-by: Wei Chen <[e-mail]>
> 
> Your signed-off-by is missing.
> 
> > ---
> > v1 -> v2:
> > 1. New patch.
> > ---
> >   xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++--------------------
> >   1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index 93f9b0b9d5..b2214bc5e3 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -136,22 +136,22 @@
> >           add \xb, \xb, x20
> >   .endm
> >
> > -        .section .text.header, "ax", %progbits
> > -        /*.aarch64*/
> > +.section .text.header, "ax", %progbits
> > +/*.aarch64*/
> 
> This change is not mentioned.
> 

I will add the description in commit message.

> >
> > -        /*
> > -         * 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)
> >           /*
> > @@ -586,7 +586,7 @@ ENDPROC(cpu_init)
> >    *
> >    * Clobbers x0 - x4
> >    */
> > -create_page_tables:
> > +ENTRY(create_page_tables)
> 
> I am not sure about keeping this name. Now we have create_page_tables()
> and arch_setup_page_tables().
> 
> I would conside to name it create_boot_page_tables().
> 

Do you need me to rename it in this patch?

> >           /* Prepare the page-tables for mapping Xen */
> >           ldr   x0, =XEN_VIRT_START
> >           create_table_entry boot_pgtable, boot_first, x0, 0, x1, x2, x3
> > @@ -680,7 +680,7 @@ ENDPROC(create_page_tables)
> >    *
> >    * Clobbers x0 - x3
> >    */
> > -enable_mmu:
> > +ENTRY(enable_mmu)
> >           PRINT("- Turning on paging -\r\n")
> >
> >           /*
> > @@ -714,7 +714,7 @@ ENDPROC(enable_mmu)
> >    *
> >    * Clobbers x0 - x1
> >    */
> > -remove_identity_mapping:
> > +ENTRY(remove_identity_mapping)
> 
> Patch #14 should be before this patch. So you don't have to export
> remove_identity_mapping temporarily.
> 
> This will also avoid (transient) naming confusing with my work (see [1]).
> 

Ok, we will do it.

> >           /*
> >            * Find the zeroeth slot used. Remove the entry from zeroeth
> >            * table if the slot is not XEN_ZEROETH_SLOT.
> > @@ -775,7 +775,7 @@ ENDPROC(remove_identity_mapping)
> >    *
> >    * Clobbers x0 - x3
> >    */
> > -setup_fixmap:
> > +ENTRY(setup_fixmap)
> >   #ifdef CONFIG_EARLY_PRINTK
> >           /* Add UART to the fixmap table */
> >           ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
> > @@ -871,7 +871,7 @@ ENDPROC(init_uart)
> >    * x0: Nul-terminated string to print.
> >    * x23: Early UART base address
> >    * Clobbers x0-x1 */
> > -puts:
> > +ENTRY(puts)
> 
> This name is a bit too generic to be globally exported. It is also now
> quite confusing because we have "early_puts" and "puts".
> 
> I would consider to name it asm_puts(). It is still not great but
> hopefully it would give a hint that should be call from assembly code.
> 

Yes, I had the same concern. I will rename it in next version.

Cheers,
Wei Chen

> >           early_uart_ready x23, 1
> >           ldrb  w1, [x0], #1           /* Load next char */
> >           cbz   w1, 1f                 /* Exit on nul */
> 
> Cheers,
> 
> [1] https://lore.kernel.org/all/[e-mail]/
> 
> --
> Julien Grall

Julien Grall Jan. 18, 2023, 9:50 a.m. UTC | #3

On 18/01/2023 03:09, Wei Chen wrote:
> Hi Julien,
> 
>> -----Original Message-----
>> From: Julien Grall <[e-mail]>
>> Sent: 2023年1月18日 7:37
>> To: Penny Zheng <[e-mail]>; [e-mail]
>> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
>> <[e-mail]>; Bertrand Marquis <[e-mail]>;
>> Volodymyr Babchuk <[e-mail]>
>> Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related
>> code from head.S
>>
>> Hi Penny,
>>
>> On 13/01/2023 05:28, Penny Zheng wrote:
>>> From: Wei Chen <[e-mail]>
>>>
>>> We want to reuse head.S for MPU systems, but there are some
>>> code implemented for MMU systems only. We will move such
>>> code to another MMU specific file. But before that, we will
>>> do some preparations in this patch to make them easier
>>> for reviewing:
>>
>> Well, I agree that...
>>
>>> 1. Fix the indentations of code comments.
>>
>> ... changing the indentation is better here. But...
>>
>>> 2. Export some symbols that will be accessed out of file
>>>      scope.
>>
>> ... I have no idea which functions are going to be used in a separate
>> file. So I think they should belong to the patch moving the code.
>>
> 
> Ok, I will move these changes to the moving code patches.
> 
>>>
>>> Signed-off-by: Wei Chen <[e-mail]>
>>
>> Your signed-off-by is missing.
>>
>>> ---
>>> v1 -> v2:
>>> 1. New patch.
>>> ---
>>>    xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++--------------------
>>>    1 file changed, 20 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>>> index 93f9b0b9d5..b2214bc5e3 100644
>>> --- a/xen/arch/arm/arm64/head.S
>>> +++ b/xen/arch/arm/arm64/head.S
>>> @@ -136,22 +136,22 @@
>>>            add \xb, \xb, x20
>>>    .endm
>>>
>>> -        .section .text.header, "ax", %progbits
>>> -        /*.aarch64*/
>>> +.section .text.header, "ax", %progbits
>>> +/*.aarch64*/
>>
>> This change is not mentioned.
>>
> 
> I will add the description in commit message.
> 
>>>
>>> -        /*
>>> -         * 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)
>>>            /*
>>> @@ -586,7 +586,7 @@ ENDPROC(cpu_init)
>>>     *
>>>     * Clobbers x0 - x4
>>>     */
>>> -create_page_tables:
>>> +ENTRY(create_page_tables)
>>
>> I am not sure about keeping this name. Now we have create_page_tables()
>> and arch_setup_page_tables().
>>
>> I would conside to name it create_boot_page_tables().
>>
> 
> Do you need me to rename it in this patch?

So looking at the rest of the series, I see you are already renaming the 
helper in patch #11. I think it would be better if the naming is done 
earlier.

That said, I am not convinced that create_page_tables() should actually 
be called externally.

In fact, you have something like:

    bl create_page_tables
    bl enable_mmu

Both will need a MMU/MPU specific implementation. So it would be better 
if we provide a wrapper to limit the number of external functions.

Cheers,

Wei Chen Jan. 18, 2023, 10:24 a.m. UTC | #4

Hi Julien,

> -----Original Message-----
> From: Julien Grall <[e-mail]>
> Sent: 2023年1月18日 17:50
> To: Wei Chen <[e-mail]>; Penny Zheng <[e-mail]>; xen-
> [e-mail]
> Cc: Stefano Stabellini <[e-mail]>; Bertrand Marquis
> <[e-mail]>; Volodymyr Babchuk <[e-mail]>
> Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related
> code from head.S
> 
> 
> 
> On 18/01/2023 03:09, Wei Chen wrote:
> > Hi Julien,
> >
> >> -----Original Message-----
> >> From: Julien Grall <[e-mail]>
> >> Sent: 2023年1月18日 7:37
> >> To: Penny Zheng <[e-mail]>; [e-mail]
> >> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
> >> <[e-mail]>; Bertrand Marquis <[e-mail]>;
> >> Volodymyr Babchuk <[e-mail]>
> >> Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related
> >> code from head.S
> >>
> >> Hi Penny,
> >>
> >> On 13/01/2023 05:28, Penny Zheng wrote:
> >>> From: Wei Chen <[e-mail]>
> >>>
> >>> We want to reuse head.S for MPU systems, but there are some
> >>> code implemented for MMU systems only. We will move such
> >>> code to another MMU specific file. But before that, we will
> >>> do some preparations in this patch to make them easier
> >>> for reviewing:
> >>
> >> Well, I agree that...
> >>
> >>> 1. Fix the indentations of code comments.
> >>
> >> ... changing the indentation is better here. But...
> >>
> >>> 2. Export some symbols that will be accessed out of file
> >>>      scope.
> >>
> >> ... I have no idea which functions are going to be used in a separate
> >> file. So I think they should belong to the patch moving the code.
> >>
> >
> > Ok, I will move these changes to the moving code patches.
> >
> >>>
> >>> Signed-off-by: Wei Chen <[e-mail]>
> >>
> >> Your signed-off-by is missing.
> >>
> >>> ---
> >>> v1 -> v2:
> >>> 1. New patch.
> >>> ---
> >>>    xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++-----------------
> ---
> >>>    1 file changed, 20 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> >>> index 93f9b0b9d5..b2214bc5e3 100644
> >>> --- a/xen/arch/arm/arm64/head.S
> >>> +++ b/xen/arch/arm/arm64/head.S
> >>> @@ -136,22 +136,22 @@
> >>>            add \xb, \xb, x20
> >>>    .endm
> >>>
> >>> -        .section .text.header, "ax", %progbits
> >>> -        /*.aarch64*/
> >>> +.section .text.header, "ax", %progbits
> >>> +/*.aarch64*/
> >>
> >> This change is not mentioned.
> >>
> >
> > I will add the description in commit message.
> >
> >>>
> >>> -        /*
> >>> -         * 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)
> >>>            /*
> >>> @@ -586,7 +586,7 @@ ENDPROC(cpu_init)
> >>>     *
> >>>     * Clobbers x0 - x4
> >>>     */
> >>> -create_page_tables:
> >>> +ENTRY(create_page_tables)
> >>
> >> I am not sure about keeping this name. Now we have create_page_tables()
> >> and arch_setup_page_tables().
> >>
> >> I would conside to name it create_boot_page_tables().
> >>
> >
> > Do you need me to rename it in this patch?
> 
> So looking at the rest of the series, I see you are already renaming the
> helper in patch #11. I think it would be better if the naming is done
> earlier.
> 
> That said, I am not convinced that create_page_tables() should actually
> be called externally.
> 
> In fact, you have something like:
> 
>     bl create_page_tables
>     bl enable_mmu
> 
> Both will need a MMU/MPU specific implementation. So it would be better
> if we provide a wrapper to limit the number of external functions.
>

I agree with you, we will try to wrapper some functions instead of
export them.

Cheers,
Wei Chen
 
> Cheers,
> 
> --
> Julien Grall

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 93f9b0b9d5..b2214bc5e3 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.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)
         /*
@@ -586,7 +586,7 @@  ENDPROC(cpu_init)
  *
  * Clobbers x0 - x4
  */
-create_page_tables:
+ENTRY(create_page_tables)
         /* Prepare the page-tables for mapping Xen */
         ldr   x0, =XEN_VIRT_START
         create_table_entry boot_pgtable, boot_first, x0, 0, x1, x2, x3
@@ -680,7 +680,7 @@  ENDPROC(create_page_tables)
  *
  * Clobbers x0 - x3
  */
-enable_mmu:
+ENTRY(enable_mmu)
         PRINT("- Turning on paging -\r\n")
 
         /*
@@ -714,7 +714,7 @@  ENDPROC(enable_mmu)
  *
  * Clobbers x0 - x1
  */
-remove_identity_mapping:
+ENTRY(remove_identity_mapping)
         /*
          * Find the zeroeth slot used. Remove the entry from zeroeth
          * table if the slot is not XEN_ZEROETH_SLOT.
@@ -775,7 +775,7 @@  ENDPROC(remove_identity_mapping)
  *
  * Clobbers x0 - x3
  */
-setup_fixmap:
+ENTRY(setup_fixmap)
 #ifdef CONFIG_EARLY_PRINTK
         /* Add UART to the fixmap table */
         ldr   x0, =EARLY_UART_VIRTUAL_ADDRESS
@@ -871,7 +871,7 @@  ENDPROC(init_uart)
  * x0: Nul-terminated string to print.
  * x23: Early UART base address
  * Clobbers x0-x1 */
-puts:
+ENTRY(puts)
         early_uart_ready x23, 1
         ldrb  w1, [x0], #1           /* Load next char */
         cbz   w1, 1f                 /* Exit on nul */