Re: [Xen-devel] [PATCH for-4.9 2/4] xen/arm: Move the code to map FDT in the boot tables from assembly to C

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 19 Apr 2017, Julien Grall wrote:
> The FDT will not be accessed before start_xen (begining of C code) is
> called and it will be easier to maintain as the code could be common
> between AArch32 and AArch64.
> 
> A new function early_fdt_map is introduced to map the FDT in the boot
> page table.
> 
> Note that create_mapping will flush the TLBs for us so no need to add an
> extra on in case the entry was previously used by the 1:1 mapping.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
> 
>     I can move the function create_mappings at the beginning of the
>     function instead of adding a static declaration. But I thought it
>     was not Xen 4.9 material. Any opinions?
> ---
>  xen/arch/arm/arm32/head.S | 14 --------------
>  xen/arch/arm/arm64/head.S | 13 -------------
>  xen/arch/arm/mm.c         | 20 ++++++++++++++++++++
>  xen/arch/arm/setup.c      |  4 +---
>  xen/include/asm-arm/mm.h  |  2 ++
>  5 files changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index ec63ba4c04..4090f4a744 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -389,20 +389,6 @@ paging:
>          /* Use a virtual address to access the UART. */
>          ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
> -        /* Map the DTB in the boot misc slot */
> -        teq   r12, #0                /* Only on boot CPU */
> -        bne   1f
> -
> -        ldr   r1, =boot_second
> -        mov   r3, #0x0
> -        lsr   r2, r8, #SECOND_SHIFT
> -        lsl   r2, r2, #SECOND_SHIFT  /* r2: 2MB-aligned paddr of DTB */
> -        orr   r2, r2, #PT_UPPER(MEM)
> -        orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
> -        ldr   r4, =BOOT_FDT_VIRT_START
> -        mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* Slot for 
> BOOT_FDT_VIRT_START */
> -        strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot */
> -1:
>  
>          /*
>           * Flush the TLB in case the 1:1 mapping happens to clash with
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 72ea4e0233..78292f4396 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -550,19 +550,6 @@ paging:
>          ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
>  
> -        /* Map the DTB in the boot misc slot */
> -        cbnz  x22, 1f                /* Only on boot CPU */
> -
> -        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
> -        lsr   x2, x21, #SECOND_SHIFT
> -        lsl   x2, x2, #SECOND_SHIFT  /* x2 := 2MB-aligned paddr of DTB */
> -        mov   x3, #PT_MEM            /* x2 := 2MB RAM incl. DTB */
> -        orr   x2, x2, x3
> -        ldr   x1, =BOOT_FDT_VIRT_START
> -        lsr   x1, x1, #(SECOND_SHIFT - 3)   /* x4 := Slot for 
> BOOT_FDT_VIRT_START */
> -        str   x2, [x4, x1]           /* Map it in the early fdt slot */
> -1:
> -
>          /*
>           * Flush the TLB in case the 1:1 mapping happens to clash with
>           * the virtual addresses used by the fixmap or DTB.
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index f0a2eddaaf..0d076489c6 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -38,6 +38,13 @@
>  #include <xen/vmap.h>
>  #include <xsm/xsm.h>
>  #include <xen/pfn.h>
> +#include <xen/sizes.h>
> +
> +static void __init create_mappings(lpae_t *second,
> +                                   unsigned long virt_offset,
> +                                   unsigned long base_mfn,
> +                                   unsigned long nr_mfns,
> +                                   unsigned int mapping_size);

I would have added early_fdt_map to this file in a way to avoid the need
for duplicating the declaration of create_mappings (because this version
doesn't have the useful comment on top).

In any case

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


>  struct domain *dom_xen, *dom_io, *dom_cow;
>  
> @@ -434,6 +441,19 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
>      return mfn_to_xen_entry(mfn, WRITEALLOC);
>  }
>  
> +
> +/* Map the FDT in the early boot page table */
> +void * __init early_fdt_map(paddr_t fdt_paddr)
> +{
> +    /* We are using 2MB superpage for mapping the FDT */
> +    paddr_t base_paddr = fdt_paddr & SECOND_MASK;
> +
> +    create_mappings(boot_second, BOOT_FDT_VIRT_START, 
> paddr_to_pfn(base_paddr),
> +                    SZ_2M >> PAGE_SHIFT, SZ_2M);
> +
> +    return (void *)BOOT_FDT_VIRT_START + (fdt_paddr % SECOND_SIZE);
> +}
> +
>  void __init remove_early_mappings(void)
>  {
>      lpae_t pte = {0};
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 92a2de6b70..986398970f 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -724,9 +724,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      smp_clear_cpu_maps();
>  
> -    /* This is mapped by head.S */
> -    device_tree_flattened = (void *)BOOT_FDT_VIRT_START
> -        + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
> +    device_tree_flattened = early_fdt_map(fdt_paddr);
>      fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
>  
>      cmdline = boot_fdt_cmdline(device_tree_flattened);
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 0fef612f42..f6915ad882 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -160,6 +160,8 @@ extern unsigned long total_pages;
>  
>  /* Boot-time pagetable setup */
>  extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t 
> xen_paddr);
> +/* Map FDT in boot pagetable */
> +extern void *early_fdt_map(paddr_t fdt_paddr);
>  /* Remove early mappings */
>  extern void remove_early_mappings(void);
>  /* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to 
> the
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel