Re: [PATCH 1/2] xen/arm32: head: Rework how the fixmap and early UART mapping are prepared

[Julien Grall <julien@xxxxxxx>]

Hi,

On 02/11/2023 02:29, Henry Wang wrote:
> Hi Julien,
>
> > On Nov 2, 2023, at 07:30, Julien Grall <julien@xxxxxxx> wrote:
> >
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > Since commit 5e213f0f4d2c ("xen/arm32: head: Widen the use of the
> > temporary mapping"), boot_second (used to cover regions like Xen and
> > the fixmap) will not be mapped if the identity mapping overlap.
> >
> > So it is ok to prepare the fixmap table and link it in boot_second
> > earlier. With that, the fixmap can also be used earlier via the
> > temporary mapping.
> >
> > Therefore split setup_fixmap() in two:
> >     * The table is now linked in create_page_tables() because
> >       the boot page tables needs to be recreated for every CPU.
> >     * The early UART mapping is only added for the boot CPU0 as the
> >       fixmap table is not cleared when secondary CPUs boot.
> >
> > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > ---
> > xen/arch/arm/arm32/head.S | 48 ++++++++-------------------------------
> > 1 file changed, 9 insertions(+), 39 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index 33b038e7e0ca..fec2433e568f 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -183,12 +183,16 @@ past_zImage:
> >          bl    check_cpu_mode
> >          bl    cpu_init
> >          bl    create_page_tables
> > +        /* Add the UART mapping if requested */
> > +#ifdef CONFIG_EARLY_PRINTK
> > +        mov_w r0, EARLY_UART_VIRTUAL_ADDRESS
> > +        create_mapping_entry xen_fixmap, r0, r11, type=PT_DEV_L3
> I tried to test this series today and found a corner case in this patch:
>
> If we build Xen only with this patch, and CONFIG_EARLY_PRINTK=y, we
> will get below from compiler:
>
> ```
> arch/arm/arm32/head.S: Assembler messages:
> arch/arm/arm32/head.S:189: Error: bad instruction `create_mapping_entry 
> xen_fixmap,r0,r11,type=0xe73'
> make[3]: *** [Rules.mk:253: arch/arm/arm32/head.o] Error 1
> ```
>
> With the second patch applied, the above error will disappear and I confirm
> arm32 will work fine with both patch applied + either CONFIG_EARLY_PRINTK={y,n}.
Thanks for testing. Yes I merged one hunk into the wrong patch. I will 
re-order the code.
Cheers,

--
Julien Grall