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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel