[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.9 4/4] xen/arm: Properly map the FDT in the boot page table



On Wed, 19 Apr 2017, Julien Grall wrote:
> Currently, Xen is assuming the FDT will always fit in a 2MB section.
> Recently, I noticed an early crash on Xen when using GRUB with the
> following call trace:
> 
> (XEN) Hypervisor Trap. HSR=0x96000006 EC=0x25 IL=1 Syndrome=0x6
> (XEN) CPU0: Unexpected Trap: Hypervisor
> (XEN) ----[ Xen-4.9-unstable  arm64  debug=y   Not tainted ]----
> (XEN) CPU:    0
> (XEN) PC:     0000000000264140 strlen+0x10/0x84
> (XEN) LR:     00000000002401c0
> (XEN) SP:     00000000002cfc20
> (XEN) CPSR:   400003c9 MODE:64-bit EL2h (Hypervisor, handler)
> (XEN)      X0: 0000000000801230  X1: 0000000000801230  X2: 0000000000005230
> (XEN)      X3: 0000000000000030  X4: 0000000000000030  X5: 0000000000000038
> (XEN)      X6: 0000000000000034  X7: 0000000000000000  X8: 7f7f7f7f7f7f7f7f
> (XEN)      X9: 64622c6479687222 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101
> (XEN)     X12: 0000000000000030 X13: ffffff00ff000000 X14: 0800000003000000
> (XEN)     X15: ffffffffffffffff X16: 00000000fefff610 X17: 00000000000000f0
> (XEN)     X18: 0000000000000004 X19: 0000000000000008 X20: 00000000007fc040
> (XEN)     X21: 00000000007fc000 X22: 000000000000000e X23: 0000000000000000
> (XEN)     X24: 00000000002a9f58 X25: 0000000000801230 X26: 00000000002a9f68
> (XEN)     X27: 00000000002a9f58 X28: 0000000000298910  FP: 00000000002cfc20
> (XEN)
> (XEN)   VTCR_EL2: 80010c40
> (XEN)  VTTBR_EL2: 0000082800203000
> (XEN)
> (XEN)  SCTLR_EL2: 30c5183d
> (XEN)    HCR_EL2: 000000000038663f
> (XEN)  TTBR0_EL2: 00000000f4912000
> (XEN)
> (XEN)    ESR_EL2: 96000006
> (XEN)  HPFAR_EL2: 00000000e8071000
> (XEN)    FAR_EL2: 0000000000801230
> (XEN)
> (XEN) Xen stack trace from sp=00000000002cfc20:
> (XEN)    00000000002cfc70 0000000000240254 00000000002a9f58 00000000007fc000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 00000000007fc03c
> (XEN)    00000000002cfd78 0000000000000000 00000000002cfca0 00000000002986fc
> (XEN)    0000000000000000 00000000007fc000 0000000000000000 0000000000000000
> (XEN)    00000000002cfcc0 0000000000298f1c 0000000000000000 00000000007fc000
> (XEN)    00000000002cfdc0 000000000029904c 00000000f47fc000 00000000f4604000
> (XEN)    00000000f47fc000 00000000007fc000 0000000000400000 0000000000000100
> (XEN)    00000000f4604000 0000000000000001 0000000000000001 8000000000000002
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 00000000002cfdc0 0000000000299038
> (XEN)    00000000f47fc000 00000000f4604000 00000000f47fc000 0000000000000000
> (XEN)    00000000002cfe20 000000000029c420 00000000002d8000 00000000f4604000
> (XEN)    00000000f47fc000 0000000000000000 0000000000400000 0000000000000100
> (XEN)    00000000f4604000 0000000000000001 00000000f47fc000 000000000029c404
> (XEN)    00000000fefff510 0000000000200624 00000000f4804000 00000000f4604000
> (XEN)    00000000f47fc000 0000000000000000 0000000000400000 0000000000000100
> (XEN)    0000000000000001 0000000000000001 0000000000000001 8000000000000002
> (XEN)    00000000f47fc000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) Xen call trace:
> (XEN)    [<0000000000264140>] strlen+0x10/0x84 (PC)
> (XEN)    [<00000000002401c0>] fdt_get_property_namelen+0x9c/0xf0 (LR)
> (XEN)    [<0000000000240254>] fdt_get_property+0x40/0x50
> (XEN)    [<00000000002986fc>] bootfdt.c#device_tree_get_u32+0x18/0x5c
> (XEN)    [<0000000000298f1c>] device_tree_for_each_node+0x84/0x144
> (XEN)    [<000000000029904c>] boot_fdt_info+0x70/0x23c
> (XEN)    [<000000000029c420>] start_xen+0x9c/0xd30
> (XEN)    [<0000000000200624>] arm64/head.o#paging+0x84/0xbc
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) CPU0: Unexpected Trap: Hypervisor
> (XEN)
> (XEN) ****************************************
> 
> Indeed, the booting documentation for AArch32 and AArch64 only requires
> the FDT to be placed on a 8-byte boundary. This means the Device-Tree can
> cross a 2MB boundary.
> 
> Given that Xen limits the size of the FDT to 2MB, it will always fit in
> a 4MB slot. So extend the fixmap slot for FDT from 2MB to 4MB.
> 
> The second 2MB superpage will only be mapped if the FDT is cross the 2MB
> boundary.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/mm.c            | 16 ++++++++++++++--
>  xen/include/asm-arm/config.h | 14 +++++++-------
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 53d36e2ce2..9cff1619c6 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -451,6 +451,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>      paddr_t base_paddr = fdt_paddr & SECOND_MASK;
>      paddr_t offset;
>      void *fdt_virt;
> +    uint32_t size;
>  
>      /*
>       * Check whether the physical FDT address is set and meets the minimum
> @@ -475,9 +476,17 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>      if ( fdt_magic(fdt_virt) != FDT_MAGIC )
>          return NULL;
>  
> -    if ( fdt_totalsize(fdt_virt) > MAX_FDT_SIZE )
> +    size = fdt_totalsize(fdt_virt);
> +    if ( size > MAX_FDT_SIZE )
>          return NULL;
>  
> +    if ( (offset + size) > SZ_2M )
> +    {
> +        create_mappings(boot_second, BOOT_FDT_VIRT_START + SZ_2M,
> +                        paddr_to_pfn(base_paddr + SZ_2M),
> +                        SZ_2M >> PAGE_SHIFT, SZ_2M);
> +    }
> +
>      return fdt_virt;
>  }
>  
> @@ -485,7 +494,8 @@ void __init remove_early_mappings(void)
>  {
>      lpae_t pte = {0};
>      write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
> -    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
> +    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);

Aren't you writing the same pte twice at the same address?


> +    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
>  }
>  
>  extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
> @@ -544,6 +554,8 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset, paddr_t xen_paddr)
>      /* ... DTB */
>      pte = boot_second[second_table_offset(BOOT_FDT_VIRT_START)];
>      xen_second[second_table_offset(BOOT_FDT_VIRT_START)] = pte;
> +    pte = boot_second[second_table_offset(BOOT_FDT_VIRT_START + SZ_2M)];
> +    xen_second[second_table_offset(BOOT_FDT_VIRT_START + SZ_2M)] = pte;
>  
>      /* ... Boot Misc area for xen relocation */
>      dest_va = BOOT_RELOC_VIRT_START;
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 9c14a385e7..5b6f3c985d 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -77,12 +77,12 @@
>   *   0  -   2M   Unmapped
>   *   2M -   4M   Xen text, data, bss
>   *   4M -   6M   Fixmap: special-purpose 4K mapping slots
> - *   6M -   8M   Early boot mapping of FDT
> - *   8M -  10M   Early relocation address (used when relocating Xen)
> + *   6M -  10M   Early boot mapping of FDT
> + *   10M - 12M   Early relocation address (used when relocating Xen)
>   *               and later for livepatch vmap (if compiled in)
>   *
>   * ARM32 layout:
> - *   0  -  10M   <COMMON>
> + *   0  -  12M   <COMMON>
>   *
>   *  32M - 128M   Frametable: 24 bytes per page for 16GB of RAM
>   * 256M -   1G   VMAP: ioremap and early_ioremap use this virtual address
> @@ -93,7 +93,7 @@
>   *
>   * ARM64 layout:
>   * 0x0000000000000000 - 0x0000007fffffffff (512GB, L0 slot [0])
> - *   0  -  10M   <COMMON>
> + *   0  -  12M   <COMMON>
>   *
>   *   1G -   2G   VMAP: ioremap and early_ioremap
>   *
> @@ -113,12 +113,12 @@
>  #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
>  
>  #define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
> -#define BOOT_FDT_SLOT_SIZE     MB(2)
> +#define BOOT_FDT_SLOT_SIZE     MB(4)
>  #define BOOT_FDT_VIRT_END      (BOOT_FDT_VIRT_START + BOOT_FDT_SLOT_SIZE)
>  
> -#define BOOT_RELOC_VIRT_START  _AT(vaddr_t,0x00800000)
> +#define BOOT_RELOC_VIRT_START  _AT(vaddr_t,0x00a00000)

Wouldn't it be better to define it as BOOT_FDT_VIRT_END?


>  #ifdef CONFIG_LIVEPATCH
> -#define LIVEPATCH_VMAP_START   _AT(vaddr_t,0x00800000)
> +#define LIVEPATCH_VMAP_START   _AT(vaddr_t,0x00a00000)

same here


>  #define LIVEPATCH_VMAP_END     (LIVEPATCH_VMAP_START + MB(2))
>  #endif
>  
> -- 
> 2.11.0
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.