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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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).

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



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