Re: [PATCH] xen/arm: Rework the way to compute dom0 DTB base address

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/05/13 14:50, Julien Grall wrote:
> If the DTB is loading right the after the kernel, on some setup, Linux will
> overwrite the DTB during the decompression step.
>
> To be sure the DTB won't be overwritten by the decrompression stage, load
decompression
> the DTB near the end of the first memory bank and below 4Gib (if memory range is
> greater).

Surely the correct solution is to make Linux aware that a DTB is present
so it wont overwrite it?

Sticking it at the end of memory in the hope that it wont be overwritten
is still going to fail in a somewhat memory-limited situation.

~Andrew

>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/domain_build.c |   19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f857e40..ca086a3 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -519,6 +519,7 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>      void *fdt;
>      int new_size;
>      int ret;
> +    paddr_t end;
>  
>      kinfo->unassigned_mem = dom0_mem;
>  
> @@ -543,11 +544,6 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>      if ( ret < 0 )
>          goto err;
>  
> -    /*
> -     * Put the device tree at the beginning of the first bank.  It
> -     * must be below 4 GiB.
> -     */
> -    kinfo->dtb_paddr = kinfo->mem.bank[0].start + 0x100;
>      if ( kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt) > (1ull << 32) )
>      {
>          printk("Not enough memory below 4 GiB for the device tree.");
> @@ -555,6 +551,16 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>          goto err;
>      }
>  
> +    /*
> +     * DTB must be load below 4GiB and far enough to linux (Linux use
> +     * the space after it to decompress)
> +     * Load the DTB at the end of the first bank or below 4Gib
> +     */
> +    end = kinfo->mem.bank[0].start + kinfo->mem.bank[0].size;
> +    kinfo->dtb_paddr = (MIN(1ull << 32, end) - fdt_totalsize(kinfo->fdt));
> +    /* Linux requires the address to be a multiple of 4 */
> +    kinfo->dtb_paddr &= ~3;
> +
>      return 0;
>  
>    err:
> @@ -566,6 +572,8 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>  static void dtb_load(struct kernel_info *kinfo)
>  {
>      void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
> +    printk("Loading dom0 DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
> +           kinfo->dtb_paddr, kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
>  
>      raw_copy_to_guest(dtb_virt, kinfo->fdt, fdt_totalsize(kinfo->fdt));
>      xfree(kinfo->fdt);
> @@ -604,7 +612,6 @@ int construct_dom0(struct domain *d)
>      /* The following loads use the domain's p2m */
>      p2m_load_VTTBR(d);
>  
> -    kinfo.dtb_paddr = kinfo.zimage.load_addr + kinfo.zimage.len;
>      kernel_load(&kinfo);
>      dtb_load(&kinfo);
>