Re: [Xen-devel] [PATCH] xen: arm: rework placement of fdt in initial dom0 memory map

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-08-28 at 11:43 +0100, Julien Grall wrote:
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index f12f895..e8f3f3b 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -211,11 +211,30 @@ static int kernel_try_zimage32_prepare(struct 
> > kernel_info *info,
> >      info->zimage.kernel_addr = addr;
> >  
> >      /*
> > -     * If start is zero, the zImage is position independent -- load it
> > -     * at 32k from start of RAM.
> > +     * If start is zero, the zImage is position independent, in this
> > +     * case Documentation/arm/Booting recommends loading below 128MiB
> > +     * and above 32MiB. Load it as high as possible within these
> > +     * constraints, while also avoiding the DTB.
> >       */
> >      if (start == 0)
> > -        info->zimage.load_addr = info->mem.bank[0].start + 0x8000;
> > +    {
> > +        paddr_t load_end;
> > +
> > +        load_end = info->mem.bank[0].start + info->mem.bank[0].size;
> > +        load_end = MIN(info->mem.bank[0].start + (128<<20), load_end);
> > +
> > +        /*
> > +         * FDT is loaded above 128M or as high as possible, so the
> > +         * only way we can clash is if we have <=128MB, in which case
> 
> Is it possible to start an ARM64 dom0 with only 128MB of RAM?

Yes, but...

> If yes, we need the same code in kernel_try_zimage64_prepare.

On arm64 the load address is mandated in the zImage head (text_offset
field) so we don't get to choose.

> 
> > +         * FDT will be right at the end and so dtb_paddr will be below
> > +         * the proposed kernel load address. Move the kernel down if
> > +         * necessary.
> > +         */
> > +        if ( load_end >= info->dtb_paddr )
> > +            load_end = info->dtb_paddr;
> > +
> > +        info->zimage.load_addr = load_end - size;
> 
> With this solution, it's possible to have size > load_end. load_addr
> will have a wrong address.

load_end is absolute (i.e. it incorporates the ram base address) while
size is relative. I think the appropriate check is that load_addr is >=
bank[0].start. I can incorporate this into the check that the kernel is
within the dom0 memory which you suggest below.

> Furthermore, I think you need to use (end - start) instead of size. On
> the current implementation size can be greater than the real image size.
> If you want to stick on your solution, then you must update zimage.len.

Right, I don't know whether end-start or load_addr is better (since the
former will cut of an appended dtb -- but that might be a feature) but
since the len is already end-start I will make this consistent and if we
want to change we can do so later.

> > +    }
> >      else
> >          info->zimage.load_addr = start;
> 
> This is not a part of the patch. The line above is wrong, Xen needs to
> check if start belongs to the dom0 RAM.

I think we will end up doing this when it tries to load the kernel into
non-existent ram...

Ian.


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