Re: [xen-unstable test] 18128: regressions - FAIL

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

On Sat, 2013-06-15 at 12:15 +0100, Andrew Cooper wrote:
> On 15/06/2013 09:35, Ian Campbell wrote:
> > On Sat, 2013-06-15 at 07:16 +0100, xen.org wrote:
> >> flight 18128 xen-unstable real [real]
> >> http://www.chiark.greenend.org.uk/~xensrcts/logs/18128/
> >>
> >> Regressions :-(
> >>
> >> Tests which did not succeed and are blocking,
> >> including tests which could not be run:
> >>  build-armhf                   4 xen-build                 fail REGR. vs. 18121
> > kernel.c: In function 'kernel_elf_load':
> > kernel.c:162:18: error: 'struct elf_binary' has no member named 'dest'
> > make[4]: *** [kernel.o] Error 1
> >
> > The bisector has separately fingered ed65808a8ed4 "libelf: check all
> > pointer accesses".
> >
> > 8<-----------------------
> >
> > From d1b3125b7e4d9ad3b1d8bb78781dd95fc2af2fee Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ian.campbell@citrix.com>
> > Date: Sat, 15 Jun 2013 09:30:47 +0100
> > Subject: [PATCH] xen: arm: fix build after libelf changes.
> >
> > ed65808a8ed4 "libelf: check all pointer accesses" caused:
> >     kernel.c: In function 'kernel_elf_load':
> >     kernel.c:162:18: error: 'struct elf_binary' has no member named 'dest'
> >     make[4]: *** [kernel.o] Error 1
> >
> > The field is now called dest_base. We also need to populate dest_size.
> >
> > This fixes the build for me although have not tested it. I have a feeling that
> > loading the kernel from an ELF file on ARM doesn't currently work anyway
> > (everyone uses the zImage loader as far as I am aware).
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

> Although it might perhaps be better to fail kernel_elf_load() with
> panic("Not implemented yet") if it really has never been tried.  It is
> likely to be less subtle than what actually does break.

It did used to work, we used it before we got zImage support, and I
don't know for sure its broken, I just suspect it is.

We should probably just rip it out at some point, but for now lets just
get the build error fixed.

> 
> > Cc: ian.jackson@eu.citrix.com
> > Cc: stefano.stabellini@citrix.com
> > Cc: julien.grall@citrix.com
> > Cc: tim@xen.org
> > ---
> >  xen/arch/arm/kernel.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index 43cf2ab..aba5441 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -159,7 +159,9 @@ static int kernel_try_zimage_prepare(struct kernel_info *info,
> >  static void kernel_elf_load(struct kernel_info *info)
> >  {
> >      printk("Loading ELF image into guest memory\n");
> > -    info->elf.elf.dest = (void*)(unsigned long)info->elf.parms.virt_kstart;
> > +    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");
>