Re: [PATCH] tools/libxc: Fix construction of HVM guests with non-default firmware

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-11-11 at 20:18 +0000, Andrew Cooper wrote:
> c/s 1ee15d7 "libxl: switch HVM domain building to use xc_dom_* helpers"
> introduced a regression building HVM domains in combination with the
> libxl
> "firmware_override=" option.
> 
> The older HVM building code (now removed) had no 32bit ELF check, so would
> happily load ELF64 images which contained a stub to switch into long mode.

IOW a ELF64 with 32-bit code at its entry point? Is that entry point the
ELF entry point or the special Xen entry point located via the notes?

I think you likely mean the latter, in which case I'm ok with this change
if that entry point is explicitly documented to be 32-bit irrespective of
the containing ELF file (either the commit message should mention this is 
already the case or the patch should update the docs to make it so). 

I'm sure I saw this getting documented in patches at some point but I can't
actually find them. All I found was docs/misc/pvh-readme.txt which
discusses original pvh and not dmlite, so I'm pretty sure that is the wrong
place to be looking.

In any case I think for the purposes of supporting users who have done
something odd by accident rather than design it would be worthwhile to log
the type of ELF file somewhere, perhaps we already do, but I couldn't find
it if so.

> It is convenient for the ELF file to match the intended runmode rather
> than the starting runmode.  As such, don't make an arbitrary restriction.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> 
> My Xen Test Framework (which is almost ready to be formally presented on
> xen-devel) uses ELF32/ELF64 for the intended runmode to make
> compiling/disassembling easier.
> 
> At the point that the developer is specifying firmware_override, they are into
> "just do what I tell you" territory, and can keep both pieces if they have
> actually passed a broken firmware.
> 
> This change has actually regressed XenServers automated testing against
> xen-upstream, which does make use of the Test Framework.
> ---
>  tools/libxc/xc_dom_hvmloader.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_hvmloader.c
> b/tools/libxc/xc_dom_hvmloader.c
> index 79a3b99..0cf9887 100644
> --- a/tools/libxc/xc_dom_hvmloader.c
> +++ b/tools/libxc/xc_dom_hvmloader.c
> @@ -107,13 +107,6 @@ static elf_errorstatus
> xc_dom_parse_hvm_kernel(struct xc_dom_image *dom)
>          return rc;
>      }
>  
> -    if ( !elf_32bit(elf) )
> -    {
> -        xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: ELF image is not
> 32bit",
> -                     __func__);
> -        return -EINVAL;
> -    }
> -
>      /* parse binary and get xen meta info */
>      elf_parse_binary(elf);
>  

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