diff mbox

[Xen-devel,2/3] xen: arm: probe the kernel to determine the guest type earlier

Message ID 1396616219-32219-2-git-send-email-ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell April 4, 2014, 12:56 p.m. UTC
We need to know if the kernel is 32- or 64- bit capable sooner so that we know
what sort of domain we are building when allocating memory to it (so we can
place appropriate limits when allocating RAM to the guest). At the moment
kernel_prepare() decides this but it needs the memory to have already been
allocated.

Therefore split the probing functionality of kernel_prepare() and call it much
earlier. The remainder of kernel_prepare() deals with where to place the
kernel in RAM which can be deferred until kernel_load() so do so.

Document the input and output of kernel_probe() and _load().

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |   10 ++---
 xen/arch/arm/kernel.c       |   95 ++++++++++++++++++++++++-------------------
 xen/arch/arm/kernel.h       |   26 +++++++++++-
 3 files changed, 83 insertions(+), 48 deletions(-)

Comments

Julien Grall April 4, 2014, 2:44 p.m. UTC | #1
Hi Ian,

On 04/04/2014 01:56 PM, Ian Campbell wrote:
> +static paddr_t kernel_zimage_place(struct kernel_info *info)
> +{
> +    paddr_t load_addr;
> +
> +#ifdef CONFIG_ARM_64
> +    if ( info->type == DOMAIN_64BIT )
> +        return info->mem.bank[0].start + info->zimage.text_offset;
> +#endif
> +
> +    /*
> +     * If start is zero, the zImage is position independent, in this
> +     * case Documentation/arm/Booting recommends loading below 128MiB
> +     * and above 32MiB. Load it as high as possible within these
> +     * constraints, while also avoiding the DTB.
> +     */
> +    if (info->zimage.start == 0)

if  ( ... )

[..]

> -int kernel_prepare(struct kernel_info *info);
> +/*
> + * Probe the kernel to detemine its type and select a loader.
> + *
> + * Sets in info:
> + *  ->type
> + *  ->load hook, and sets loader specific variables ->{zimage,elf}
> + */
> +int kernel_probe(struct kernel_info *info);

I would add a newline here.

With theses 2 minor changes, the code looks good to me:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 502db84..4d6b26b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1002,9 +1002,7 @@  int construct_dom0(struct domain *d)
 
     kinfo.unassigned_mem = dom0_mem;
 
-    allocate_memory(d, &kinfo);
-
-    rc = kernel_prepare(&kinfo);
+    rc = kernel_probe(&kinfo);
     if ( rc < 0 )
         return rc;
 
@@ -1012,6 +1010,8 @@  int construct_dom0(struct domain *d)
     d->arch.type = kinfo.type;
 #endif
 
+    allocate_memory(d, &kinfo);
+
     rc = prepare_dtb(d, &kinfo);
     if ( rc < 0 )
         return rc;
@@ -1024,8 +1024,8 @@  int construct_dom0(struct domain *d)
     p2m_restore_state(v);
 
     /*
-     * kernel_load will determine the placement of the initrd & fdt in
-     * RAM, so call it first.
+     * kernel_load will determine the placement of the kernel as well
+     * as the initrd & fdt in RAM, so call it first.
      */
     kernel_load(&kinfo);
     /* initrd_load will fix up the fdt, so call it before dtb_load */
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d1fd307..a7a3e0c 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -116,13 +116,47 @@  static void place_modules(struct kernel_info *info,
     info->initrd_paddr = info->dtb_paddr + dtb_len;
 }
 
+static paddr_t kernel_zimage_place(struct kernel_info *info)
+{
+    paddr_t load_addr;
+
+#ifdef CONFIG_ARM_64
+    if ( info->type == DOMAIN_64BIT )
+        return info->mem.bank[0].start + info->zimage.text_offset;
+#endif
+
+    /*
+     * If start is zero, the zImage is position independent, in this
+     * case Documentation/arm/Booting recommends loading below 128MiB
+     * and above 32MiB. Load it as high as possible within these
+     * constraints, while also avoiding the DTB.
+     */
+    if (info->zimage.start == 0)
+    {
+        paddr_t load_end;
+
+        load_end = info->mem.bank[0].start + info->mem.bank[0].size;
+        load_end = MIN(info->mem.bank[0].start + MB(128), load_end);
+
+        load_addr = load_end - info->zimage.len;
+        /* Align to 2MB */
+        load_addr &= ~((2 << 20) - 1);
+    }
+    else
+        load_addr = info->zimage.start;
+
+    return load_addr;
+}
+
 static void kernel_zimage_load(struct kernel_info *info)
 {
-    paddr_t load_addr = info->zimage.load_addr;
+    paddr_t load_addr = kernel_zimage_place(info);
     paddr_t paddr = info->zimage.kernel_addr;
     paddr_t len = info->zimage.len;
     unsigned long offs;
 
+    info->entry = load_addr;
+
     place_modules(info, load_addr, load_addr + len);
 
     printk("Loading zImage from %"PRIpaddr" to %"PRIpaddr"-%"PRIpaddr"\n",
@@ -154,10 +188,10 @@  static void kernel_zimage_load(struct kernel_info *info)
 
 #ifdef CONFIG_ARM_64
 /*
- * Check if the image is a 64-bit zImage and setup kernel_info
+ * Check if the image is a 64-bit Image.
  */
-static int kernel_try_zimage64_prepare(struct kernel_info *info,
-                                     paddr_t addr, paddr_t size)
+static int kernel_zimage64_probe(struct kernel_info *info,
+                                 paddr_t addr, paddr_t size)
 {
     /* linux/Documentation/arm64/booting.txt */
     struct {
@@ -196,12 +230,9 @@  static int kernel_try_zimage64_prepare(struct kernel_info *info,
         return -EINVAL;
 
     info->zimage.kernel_addr = addr;
-
-    info->zimage.load_addr = info->mem.bank[0].start
-        + zimage.text_offset;
     info->zimage.len = end - start;
+    info->zimage.text_offset = zimage.text_offset;
 
-    info->entry = info->zimage.load_addr;
     info->load = kernel_zimage_load;
 
     info->type = DOMAIN_64BIT;
@@ -213,8 +244,8 @@  static int kernel_try_zimage64_prepare(struct kernel_info *info,
 /*
  * Check if the image is a 32-bit zImage and setup kernel_info
  */
-static int kernel_try_zimage32_prepare(struct kernel_info *info,
-                                     paddr_t addr, paddr_t size)
+static int kernel_zimage32_probe(struct kernel_info *info,
+                                 paddr_t addr, paddr_t size)
 {
     uint32_t zimage[ZIMAGE32_HEADER_LEN/4];
     uint32_t start, end;
@@ -250,28 +281,9 @@  static int kernel_try_zimage32_prepare(struct kernel_info *info,
 
     info->zimage.kernel_addr = addr;
 
-    /*
-     * If start is zero, the zImage is position independent, in this
-     * case Documentation/arm/Booting recommends loading below 128MiB
-     * and above 32MiB. Load it as high as possible within these
-     * constraints, while also avoiding the DTB.
-     */
-    if (start == 0)
-    {
-        paddr_t load_end;
-
-        load_end = info->mem.bank[0].start + info->mem.bank[0].size;
-        load_end = MIN(info->mem.bank[0].start + MB(128), load_end);
-
-        info->zimage.load_addr = load_end - end;
-        /* Align to 2MB */
-        info->zimage.load_addr &= ~((2 << 20) - 1);
-    }
-    else
-        info->zimage.load_addr = start;
+    info->zimage.start = start;
     info->zimage.len = end - start;
 
-    info->entry = info->zimage.load_addr;
     info->load = kernel_zimage_load;
 
 #ifdef CONFIG_ARM_64
@@ -283,6 +295,12 @@  static int kernel_try_zimage32_prepare(struct kernel_info *info,
 
 static void kernel_elf_load(struct kernel_info *info)
 {
+    /*
+     * TODO: can the ELF header be used to find the physical address
+     * to load the image to?  Instead of assuming virt == phys.
+     */
+    info->entry = info->elf.parms.virt_entry;
+
     place_modules(info,
                   info->elf.parms.virt_kstart,
                   info->elf.parms.virt_kend);
@@ -298,8 +316,8 @@  static void kernel_elf_load(struct kernel_info *info)
     free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
 }
 
-static int kernel_try_elf_prepare(struct kernel_info *info,
-                                  paddr_t addr, paddr_t size)
+static int kernel_elf_probe(struct kernel_info *info,
+                            paddr_t addr, paddr_t size)
 {
     int rc;
 
@@ -334,11 +352,6 @@  static int kernel_try_elf_prepare(struct kernel_info *info,
     }
 #endif
 
-    /*
-     * TODO: can the ELF header be used to find the physical address
-     * to load the image to?  Instead of assuming virt == phys.
-     */
-    info->entry = info->elf.parms.virt_entry;
     info->load = kernel_elf_load;
 
     if ( elf_check_broken(&info->elf.elf) )
@@ -355,7 +368,7 @@  err:
     return rc;
 }
 
-int kernel_prepare(struct kernel_info *info)
+int kernel_probe(struct kernel_info *info)
 {
     int rc;
 
@@ -373,12 +386,12 @@  int kernel_prepare(struct kernel_info *info)
     printk("Loading kernel from boot module %d\n", MOD_KERNEL);
 
 #ifdef CONFIG_ARM_64
-    rc = kernel_try_zimage64_prepare(info, start, size);
+    rc = kernel_zimage64_probe(info, start, size);
     if (rc < 0)
 #endif
-        rc = kernel_try_zimage32_prepare(info, start, size);
+        rc = kernel_zimage32_probe(info, start, size);
     if (rc < 0)
-        rc = kernel_try_elf_prepare(info, start, size);
+        rc = kernel_elf_probe(info, start, size);
 
     return rc;
 }
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 2c27c64..2f3d2d4 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -31,8 +31,11 @@  struct kernel_info {
     union {
         struct {
             paddr_t kernel_addr;
-            paddr_t load_addr;
             paddr_t len;
+#ifdef CONFIG_ARM_64
+            paddr_t text_offset; /* 64-bit Image only */
+#endif
+            paddr_t start; /* 32-bit zImage only */
         } zimage;
 
         struct {
@@ -44,7 +47,26 @@  struct kernel_info {
     };
 };
 
-int kernel_prepare(struct kernel_info *info);
+/*
+ * Probe the kernel to detemine its type and select a loader.
+ *
+ * Sets in info:
+ *  ->type
+ *  ->load hook, and sets loader specific variables ->{zimage,elf}
+ */
+int kernel_probe(struct kernel_info *info);
+/*
+ * Loads the kernel into guest RAM.
+ *
+ * Expects to be set in info when called:
+ *  ->mem
+ *  ->fdt
+ *
+ * Sets in info:
+ *  ->entry
+ *  ->dtb_paddr
+ *  ->initrd_paddr
+ */
 void kernel_load(struct kernel_info *info);
 
 #endif /* #ifdef __ARCH_ARM_KERNEL_H__ */