Re: [Xen-devel] [PATCH] xen: arm: load FDT below 0.5G

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 29 July 2013 15:46, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Wed, 2013-07-24 at 15:37 +0100, Ian Campbell wrote:
>> BTW I asked for confirmation on linux-arm-kernel but I don't see that in
>> my l-a-k folder or the archives -- did you get the copy I CCd to you?
>> Subject is "Requirements for FDT load address on ARM".
>
> There was some discussion on L-A-K and I think we know what to do, see
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/254538 and
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/255716
>
> The updated patch would look like the below (although I may try and
> combine place+check_overlap+load into a single function, which tries a
> bit harder to find a good address by looping until overlap == false).

Do you plan to support position-dependent DOM0 kernel? If no, as the
kernel will be relocated to the same address, I don't think we need to
automatically find the DTB base address.

> However since this has grown a dependency on the 64-bit guest series I'm
> going to hold off until that goes in (it looks very close) and then
> rebase + resend this one. Hopefully in the meantime the Linux patch will
> gain some sort of consensus.
>
> Ian.
>
> commit 176dafbc2f916f369450e647a33806c13270be15
> Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Date:   Tue Jul 23 06:05:29 2013 +0100
>
>     xen: arm: load FDT at correct address
>
>     The 64-bit Documentation/arm64/booting.txt requires that the FDT be below
>     512MiB. On 32-bit things are less clear but discussion at
>     http://thread.gmane.org/gmane.linux.ports.arm.kernel/254538 suggests that
>     below 256MiB should be good. I've submitted a patch to
>     Documentation/arm/Booting to cover this.
>
>     Also note that 64-bit requires 2MiB alignment for the FDT. We already do 
> this
>     but update the comment.
>
>     Locating the fdt needs to be called after prepare_kernel so we know which 
> sort
>     of kernel it is, so split out into a separate "place_fdt".
>
>     Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
       Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
>     ---
>     v2: Load below 256MiB on 32-bit and below 512MiB on 64-bit.
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 155b436..6d92686 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -477,7 +477,6 @@ 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;
>
> @@ -502,33 +501,50 @@ static int prepare_dtb(struct domain *d, struct 
> kernel_info *kinfo)
>      if ( ret < 0 )
>          goto err;
>
> +    return 0;
> +
> +  err:
> +    printk("Device tree generation failed (%d).\n", ret);
> +    xfree(kinfo->fdt);
> +    return -EINVAL;
> +}
> +
> +static int place_dtb(struct domain *d, struct kernel_info *kinfo)
> +{
> +    paddr_t end;
> +
>      /*
> -     * DTB must be load below 4GiB and far enough from linux (Linux uses
> -     * the space after it to decompress)
> -     * Load the DTB at the end of the first bank, while ensuring it is
> -     * also below 4G
> +     * DTB must be loaded such that it does not conflict with the
> +     * kernel decompressor. For 32-bit Linux Documentation/arm/Booting
> +     * recommends just below 256MB while for 64-bit Linux the
> +     * recommendation in Documentation/arm64/booting.txt is below
> +     * 512MB.
>       */
>      end = kinfo->mem.bank[0].start + kinfo->mem.bank[0].size;
> -    end = MIN(1ull << 32, end);
> +    switch (kinfo->type) {
> +    case DOMAIN_PV32:
> +        end = MIN(kinfo->mem.bank[0].start + (256<<20), end);
> +        break;
> +    case DOMAIN_PV64:
> +        end = MIN(kinfo->mem.bank[0].start + (512<<20), end);
> +        break;
> +    }
> +
>      kinfo->dtb_paddr = end - fdt_totalsize(kinfo->fdt);
> -    /* Align the address to 2Mb. Linux only requires 4 byte alignment */
> +    /*
> +     * Align the address to 2Mb, as required by 64-bit ARM64
> +     * Linux. 32-bit ARM Linux only requires 4 byte alignment
> +     */
>      kinfo->dtb_paddr &= ~((2 << 20) - 1);
>
>      if ( fdt_totalsize(kinfo->fdt) > end )
>      {
>          printk(XENLOG_ERR "Not enough memory in the first bank for "
>                 "the device tree.");
> -        ret = -FDT_ERR_XEN(EINVAL);
> -        goto err;
> +        return -ENOMEM;
>      }
>
> -
>      return 0;
> -
> -  err:
> -    printk("Device tree generation failed (%d).\n", ret);
> -    xfree(kinfo->fdt);
> -    return -EINVAL;
>  }
>
>  static void dtb_load(struct kernel_info *kinfo)
> @@ -567,6 +583,10 @@ int construct_dom0(struct domain *d)
>      if ( rc < 0 )
>          return rc;
>
> +    rc = place_dtb(d, &kinfo);
> +    if ( rc < 0 )
> +        return rc;
> +
>      map_devices_from_device_tree(d);
>      rc = platform_specific_mapping(d);
>      if ( rc < 0 )
>
>



-- 
Julien Grall

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