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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 11/01/2024 19:34, Julien Grall 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>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

with below 2 adjustments:
> 
> ----
> 
>     Changelog since v1:
>         * Rebase
>         * Move the UART mapping enable_mm_boot_cpu()
> ---
>  xen/arch/arm/arm32/mmu/head.S | 61 ++++++++---------------------------
>  1 file changed, 14 insertions(+), 47 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/mmu/head.S b/xen/arch/arm/arm32/mmu/head.S
> index 4e6395e7876d..a90799ad5451 100644
> --- a/xen/arch/arm/arm32/mmu/head.S
> +++ b/xen/arch/arm/arm32/mmu/head.S
> @@ -165,11 +165,6 @@
>   * Rebuild the boot pagetable's first-level entries. The structure
>   * is described in mm.c.
>   *
> - * After the CPU enables paging it will add the fixmap mapping
> - * to these page tables, however this may clash with the 1:1
> - * mapping. So each CPU must rebuild the page tables here with
> - * the 1:1 in place.
> - *
>   * Inputs:
>   *   r9 : paddr(start)
>   *   r10: phys offset
> @@ -197,6 +192,10 @@ create_page_tables:
>          add   r5, r5, #PAGE_SIZE            /* r5 := Next table */
>  .endr
> 
> +        /* Map the fixmap into boot_second */
> +        mov_w r0, FIXMAP_ADDR(0)
> +        create_table_entry boot_second, xen_fixmap, r0, 2
> +
>          /*
>           * Find the size of Xen in pages and multiply by the size of a
>           * PTE. This will then be compared in the mapping loop below.
> @@ -442,19 +441,20 @@ ENDPROC(enable_secondary_cpu_mm)
>  ENTRY(enable_boot_cpu_mm)
>          mov   r6, lr
> 
> +#ifdef CONFIG_EARLY_PRINTK
> +        /*
> +         * Add the UART mapping to the fixmap so the UART can be used
> +         * as the MMU is on. This only need to do done on the boot CPU.
s/need to do done/needs to be done/

> +         */
> +        mov_w r0, EARLY_UART_VIRTUAL_ADDRESS
> +        create_mapping_entry xen_fixmap, r0, r11, type=PT_DEV_L3
Would you mind listing r11 in the Input section of a comment?

~Michal