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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 19/04/2017 22:01, Stefano Stabellini wrote:
> 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).
I wanted to keep the function close to the counterpart 
remove_early_mappings rather than adding somewhere that make less sense.
Hence why I suggested to move the create_mappings function. Would you be 
fine with code motion for Xen 4.9?
Cheers,

--
Julien Grall

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