[Xen-devel,4/7] xen/arm: Drop support for loading ELF Dom0 kernel

Message ID 20180605171237.30601-5-julien.grall@arm.com
State New
Headers show
Series
  • xen/arm: Shrink down Xen on Arm
Related show

Commit Message

Julien Grall June 5, 2018, 5:12 p.m.
The code has been around since the beginning of Xen Arm. However, I am
not aware of any user and the code is pretty bogus:
    1) It is assuming virtual address == physical address.
    2) The cache is not cleaned after the Image is loaded but the Image
    is started with Cache disabled.
    3) There are not clear ABI with the guest.

Xen is currently supporting 3 other formats (zImage, Image, U-boot Image)
as well as gzip compressed version of each formats. All of them are well
documented and widely use.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Given the state, I doubt anyone is using the ELF format with Xen
    on Arm. By dropping this code, it also allows us to remove the
    built-in libelf (~1.2K lines) from Xen.
---
 xen/arch/arm/kernel.c | 77 ---------------------------------------------------
 xen/arch/arm/kernel.h | 10 +------
 2 files changed, 1 insertion(+), 86 deletions(-)

Comments

Stefano Stabellini June 12, 2018, 7:27 p.m. | #1
On Tue, 5 Jun 2018, Julien Grall wrote:
> The code has been around since the beginning of Xen Arm. However, I am
> not aware of any user and the code is pretty bogus:
>     1) It is assuming virtual address == physical address.
>     2) The cache is not cleaned after the Image is loaded but the Image
>     is started with Cache disabled.
>     3) There are not clear ABI with the guest.
> 
> Xen is currently supporting 3 other formats (zImage, Image, U-boot Image)
> as well as gzip compressed version of each formats. All of them are well
> documented and widely use.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Given the state, I doubt anyone is using the ELF format with Xen
>     on Arm. By dropping this code, it also allows us to remove the
>     built-in libelf (~1.2K lines) from Xen.
> ---
>  xen/arch/arm/kernel.c | 77 ---------------------------------------------------
>  xen/arch/arm/kernel.h | 10 +------
>  2 files changed, 1 insertion(+), 86 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index b29028f7d0..000d9397e1 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -421,81 +421,6 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
>      return 0;
>  }
>  
> -static void __init 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);
> -
> -    printk("Loading ELF image into guest memory\n");
> -    info->elf.elf.dest_base = (void*)(unsigned long)info->elf.parms.virt_kstart;
> -    info->elf.elf.dest_size =
> -         info->elf.parms.virt_kend - info->elf.parms.virt_kstart;
> -
> -    elf_load_binary(&info->elf.elf);
> -
> -    printk("Free temporary kernel buffer\n");
> -    free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
> -}
> -
> -static int __init kernel_elf_probe(struct kernel_info *info,
> -                                   paddr_t addr, paddr_t size)
> -{
> -    int rc;
> -
> -    memset(&info->elf.elf, 0, sizeof(info->elf.elf));
> -
> -    info->elf.kernel_order = get_order_from_bytes(size);
> -    info->elf.kernel_img = alloc_xenheap_pages(info->elf.kernel_order, 0);
> -    if ( info->elf.kernel_img == NULL )
> -        panic("Cannot allocate temporary buffer for kernel");
> -
> -    copy_from_paddr(info->elf.kernel_img, addr, size);
> -
> -    if ( (rc = elf_init(&info->elf.elf, info->elf.kernel_img, size )) != 0 )
> -        goto err;
> -#ifdef CONFIG_VERBOSE_DEBUG
> -    elf_set_verbose(&info->elf.elf);
> -#endif
> -    elf_parse_binary(&info->elf.elf);
> -    if ( (rc = elf_xen_parse(&info->elf.elf, &info->elf.parms)) != 0 )
> -        goto err;
> -
> -#ifdef CONFIG_ARM_64
> -    if ( elf_32bit(&info->elf.elf) )
> -        info->type = DOMAIN_32BIT;
> -    else if ( elf_64bit(&info->elf.elf) )
> -        info->type = DOMAIN_64BIT;
> -    else
> -    {
> -        printk("Unknown ELF class\n");
> -        rc = -EINVAL;
> -        goto err;
> -    }
> -#endif
> -
> -    info->load = kernel_elf_load;
> -
> -    if ( elf_check_broken(&info->elf.elf) )
> -        printk("Xen: warning: ELF kernel broken: %s\n",
> -               elf_check_broken(&info->elf.elf));
> -
> -    return 0;
> -err:
> -    if ( elf_check_broken(&info->elf.elf) )
> -        printk("Xen: ELF kernel broken: %s\n",
> -               elf_check_broken(&info->elf.elf));
> -
> -    free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
> -    return rc;
> -}
> -
>  int __init kernel_probe(struct kernel_info *info)
>  {
>      struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
> @@ -528,8 +453,6 @@ int __init kernel_probe(struct kernel_info *info)
>          rc = kernel_uimage_probe(info, mod->start, mod->size);
>      if (rc < 0)
>          rc = kernel_zimage32_probe(info, mod->start, mod->size);
> -    if (rc < 0)
> -        rc = kernel_elf_probe(info, mod->start, mod->size);
>  
>      return rc;
>  }
> diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> index 6d695097b5..47eacb5ba9 100644
> --- a/xen/arch/arm/kernel.h
> +++ b/xen/arch/arm/kernel.h
> @@ -6,7 +6,6 @@
>  #ifndef __ARCH_ARM_KERNEL_H__
>  #define __ARCH_ARM_KERNEL_H__
>  
> -#include <xen/libelf.h>
>  #include <xen/device_tree.h>
>  #include <asm/setup.h>
>  
> @@ -45,13 +44,6 @@ struct kernel_info {
>  #endif
>              paddr_t start; /* 32-bit zImage only */
>          } zimage;
> -
> -        struct {
> -            struct elf_binary elf;
> -            struct elf_dom_parms parms;
> -            unsigned kernel_order;
> -            void *kernel_img;
> -        } elf;
>      };
>  };
>  
> @@ -60,7 +52,7 @@ struct kernel_info {
>   *
>   * Sets in info:
>   *  ->type
> - *  ->load hook, and sets loader specific variables ->{zimage,elf}
> + *  ->load hook, and sets loader specific variables ->zimage
>   */
>  int kernel_probe(struct kernel_info *info);
>  
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index b29028f7d0..000d9397e1 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -421,81 +421,6 @@  static int __init kernel_zimage32_probe(struct kernel_info *info,
     return 0;
 }
 
-static void __init 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);
-
-    printk("Loading ELF image into guest memory\n");
-    info->elf.elf.dest_base = (void*)(unsigned long)info->elf.parms.virt_kstart;
-    info->elf.elf.dest_size =
-         info->elf.parms.virt_kend - info->elf.parms.virt_kstart;
-
-    elf_load_binary(&info->elf.elf);
-
-    printk("Free temporary kernel buffer\n");
-    free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
-}
-
-static int __init kernel_elf_probe(struct kernel_info *info,
-                                   paddr_t addr, paddr_t size)
-{
-    int rc;
-
-    memset(&info->elf.elf, 0, sizeof(info->elf.elf));
-
-    info->elf.kernel_order = get_order_from_bytes(size);
-    info->elf.kernel_img = alloc_xenheap_pages(info->elf.kernel_order, 0);
-    if ( info->elf.kernel_img == NULL )
-        panic("Cannot allocate temporary buffer for kernel");
-
-    copy_from_paddr(info->elf.kernel_img, addr, size);
-
-    if ( (rc = elf_init(&info->elf.elf, info->elf.kernel_img, size )) != 0 )
-        goto err;
-#ifdef CONFIG_VERBOSE_DEBUG
-    elf_set_verbose(&info->elf.elf);
-#endif
-    elf_parse_binary(&info->elf.elf);
-    if ( (rc = elf_xen_parse(&info->elf.elf, &info->elf.parms)) != 0 )
-        goto err;
-
-#ifdef CONFIG_ARM_64
-    if ( elf_32bit(&info->elf.elf) )
-        info->type = DOMAIN_32BIT;
-    else if ( elf_64bit(&info->elf.elf) )
-        info->type = DOMAIN_64BIT;
-    else
-    {
-        printk("Unknown ELF class\n");
-        rc = -EINVAL;
-        goto err;
-    }
-#endif
-
-    info->load = kernel_elf_load;
-
-    if ( elf_check_broken(&info->elf.elf) )
-        printk("Xen: warning: ELF kernel broken: %s\n",
-               elf_check_broken(&info->elf.elf));
-
-    return 0;
-err:
-    if ( elf_check_broken(&info->elf.elf) )
-        printk("Xen: ELF kernel broken: %s\n",
-               elf_check_broken(&info->elf.elf));
-
-    free_xenheap_pages(info->elf.kernel_img, info->elf.kernel_order);
-    return rc;
-}
-
 int __init kernel_probe(struct kernel_info *info)
 {
     struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
@@ -528,8 +453,6 @@  int __init kernel_probe(struct kernel_info *info)
         rc = kernel_uimage_probe(info, mod->start, mod->size);
     if (rc < 0)
         rc = kernel_zimage32_probe(info, mod->start, mod->size);
-    if (rc < 0)
-        rc = kernel_elf_probe(info, mod->start, mod->size);
 
     return rc;
 }
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 6d695097b5..47eacb5ba9 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -6,7 +6,6 @@ 
 #ifndef __ARCH_ARM_KERNEL_H__
 #define __ARCH_ARM_KERNEL_H__
 
-#include <xen/libelf.h>
 #include <xen/device_tree.h>
 #include <asm/setup.h>
 
@@ -45,13 +44,6 @@  struct kernel_info {
 #endif
             paddr_t start; /* 32-bit zImage only */
         } zimage;
-
-        struct {
-            struct elf_binary elf;
-            struct elf_dom_parms parms;
-            unsigned kernel_order;
-            void *kernel_img;
-        } elf;
     };
 };
 
@@ -60,7 +52,7 @@  struct kernel_info {
  *
  * Sets in info:
  *  ->type
- *  ->load hook, and sets loader specific variables ->{zimage,elf}
+ *  ->load hook, and sets loader specific variables ->zimage
  */
 int kernel_probe(struct kernel_info *info);