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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-05-27 at 14:50 +0100, 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).

I've been considering something like this too. I have a feeling we might
end up just pushing things around in memory continually breaking one
platform in favour of another. However this new choice location seems
likely to be more compatible than what we have now...

> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  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) )

Isn't this check now misplaced?

>      {
>          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;

What do you think of aligning to e.g. a 2MB boundary?

"multiple of 4" would be more usually expressed in this context as
"aligned to 4 bytes".

> +
>      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);
>  



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