[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] xen/arm: Add support for booting gzip compressed uImages



Hi,

On 31/01/2023 15:13, Michal Orzel wrote:
At the moment, Xen does not support booting gzip compressed uImages.
This is because we are trying to decompress the kernel before probing
the u-boot header. This leads to a failure as the header always appears
at the top of the image (and therefore obscuring the gzip header).

Move the call to kernel_uimage_probe before kernel_decompress and make
the function self-containing by taking the following actions:
  - take a pointer to struct bootmodule as a parameter,
  - check the comp field of a u-boot header to determine compression type,
  - in case of compressed image, modify boot module start address and size
    by taking the header size into account and call kernel_decompress,
  - set up zimage.{kernel_addr,len} accordingly,
  - return -ENOENT in case of a u-boot header not found to distinguish it
    amongst other return values and make it the only case for falling
    through to try to probe other image types.

This is done to avoid splitting the uImage probing into 2 stages (executed
before and after decompression) which otherwise would be necessary to
properly update boot module start and size before decompression and
zimage.{kernel_addr,len} afterwards.

AFAIU, it would be possible to have a zImage/Image header embedded in the uImage. So any reason to only handle a compressed binary?


Remove the limitation from the booting.txt documentation.

Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
---
  docs/misc/arm/booting.txt |  3 ---
  xen/arch/arm/kernel.c     | 51 ++++++++++++++++++++++++++++++++++-----
  2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/docs/misc/arm/booting.txt b/docs/misc/arm/booting.txt
index bd7bfe7f284a..02f7bb65ec6d 100644
--- a/docs/misc/arm/booting.txt
+++ b/docs/misc/arm/booting.txt
@@ -50,9 +50,6 @@ Also, it is to be noted that if user provides the legacy 
image header on
  top of zImage or Image header, then Xen uses the attributes of legacy
  image header to determine the load address, entry point, etc.
-Known limitation: compressed kernels with a uboot headers are not
-working.
-
Firmware/bootloader requirements
  --------------------------------
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 068fbf88e492..ea5f9618169e 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -265,11 +265,14 @@ static __init int kernel_decompress(struct bootmodule 
*mod)
  #define IH_ARCH_ARM             2       /* ARM          */
  #define IH_ARCH_ARM64           22      /* ARM64        */
+/* uImage Compression Types */
+#define IH_COMP_GZIP            1
+
  /*
   * Check if the image is a uImage and setup kernel_info
   */
  static int __init kernel_uimage_probe(struct kernel_info *info,
-                                      paddr_t addr, paddr_t size)
+                                      struct bootmodule *mod)
  {
      struct {
          __be32 magic;   /* Image Header Magic Number */
@@ -287,6 +290,8 @@ static int __init kernel_uimage_probe(struct kernel_info 
*info,
      } uimage;
uint32_t len;
+    paddr_t addr = mod->start;
+    paddr_t size = mod->size;
if ( size < sizeof(uimage) )
          return -EINVAL;
@@ -294,13 +299,21 @@ static int __init kernel_uimage_probe(struct kernel_info 
*info,
      copy_from_paddr(&uimage, addr, sizeof(uimage));
if ( be32_to_cpu(uimage.magic) != UIMAGE_MAGIC )
-        return -EINVAL;
+        return -ENOENT;
len = be32_to_cpu(uimage.size); if ( len > size - sizeof(uimage) )
          return -EINVAL;
+ /* Only gzip compression is supported. */
+    if ( uimage.comp && uimage.comp != IH_COMP_GZIP )
+    {
+        printk(XENLOG_ERR
+               "Unsupported uImage compression type %"PRIu8"\n", uimage.comp);
+        return -EOPNOTSUPP;
+    }
+
      info->zimage.start = be32_to_cpu(uimage.load);
      info->entry = be32_to_cpu(uimage.ep);
@@ -330,8 +343,26 @@ static int __init kernel_uimage_probe(struct kernel_info *info,
          return -EINVAL;
      }
- info->zimage.kernel_addr = addr + sizeof(uimage);
-    info->zimage.len = len;
+    if ( uimage.comp )
+    {
+        int rc;
+
+        /* Prepare start and size for decompression. */
+        mod->start += sizeof(uimage);
+        mod->size -= sizeof(uimage);

kernel_decompress() will free the compressed module once it is decompressed. By updating the region it means the free page will be not be freed (assuming start was previously page-aligned).

+
+        rc = kernel_decompress(mod);
+        if ( rc )
+            return rc;
+
+        info->zimage.kernel_addr = mod->start;
+        info->zimage.len = mod->size;
+    }
+    else
+    {
+        info->zimage.kernel_addr = addr + sizeof(uimage);
+        info->zimage.len = len;
+    }
info->load = kernel_zimage_load; @@ -561,6 +592,16 @@ int __init kernel_probe(struct kernel_info *info,
          printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
                 info->initrd_bootmodule->start);
+ /*
+     * uImage header always appears at the top of the image (even compressed),
+     * so it needs to be probed first. Note that in case of compressed uImage,
+     * kernel_decompress is called from kernel_uimage_probe making the function
+     * self-containing (i.e. fall through only in case of a header not found).
+    */
+    rc = kernel_uimage_probe(info, mod);
+    if ( rc != -ENOENT )
+        return rc;
+
      /* if it is a gzip'ed image, 32bit or 64bit, uncompress it */
      rc = kernel_decompress(mod);
      if ( rc && rc != -EINVAL )
@@ -570,8 +611,6 @@ int __init kernel_probe(struct kernel_info *info,
      rc = kernel_zimage64_probe(info, mod->start, mod->size);
      if (rc < 0)
  #endif
-        rc = kernel_uimage_probe(info, mod->start, mod->size);
-    if (rc < 0)
          rc = kernel_zimage32_probe(info, mod->start, mod->size);
return rc;
Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.