Re: [Xen-devel] [PATCH 09/12] arm: load dom0 kernel from first boot module

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-11-29 at 17:15 +0000, Tim Deegan wrote:
> At 16:23 +0000 on 13 Nov (1352823801), Ian Campbell wrote:
> > -static int kernel_try_zimage_prepare(struct kernel_info *info)
> > +static int kernel_try_zimage_prepare(struct kernel_info *info,
> > +                                     paddr_t addr, paddr_t size)
> >  {
> >      uint32_t *zimage = (void *)FIXMAP_ADDR(FIXMAP_MISC);
> >      uint32_t start, end;
> >      struct minimal_dtb_header dtb_hdr;
> >  
> > -    set_fixmap(FIXMAP_MISC, KERNEL_FLASH_ADDRESS >> PAGE_SHIFT, 
> > DEV_SHARED);
> > +    set_fixmap(FIXMAP_MISC, addr >> PAGE_SHIFT, DEV_SHARED);
> > +
> > +    zimage += addr & ~PAGE_MASK;
> >  
> >      if (zimage[ZIMAGE_MAGIC_OFFSET/4] != ZIMAGE_MAGIC)
> > +    {
> > +        clear_fixmap(FIXMAP_MISC);
> >          return -EINVAL;
> > +    }
> >  
> >      start = zimage[ZIMAGE_START_OFFSET/4];
> >      end = zimage[ZIMAGE_END_OFFSET/4];
> >  
> >      clear_fixmap(FIXMAP_MISC);
> >  
> > +    if ( end > addr + size )
> > +        return -EINVAL;
> 
> Should this also check for start == 0 && end > size?

Possibly ought to be checking for (end - start) > size which covers
both?

Looking at it now comparing addr + size with end seems a bit nonsensical
since addr is where it is now and end is the end of where it would like
to be loaded (or the size if start == 0, which is what has saved us so
far).

> 
> >  int kernel_prepare(struct kernel_info *info)
> >  {
> >      int rc;
> >  
> > -    rc = kernel_try_zimage_prepare(info);
> > +    paddr_t start, size;
> > +
> > +    if ( early_info.modules.nr_mods > 1 )
> > +        panic("Cannot handle dom0 initrd yet\n");
> > +
> > +    if ( early_info.modules.nr_mods < 1 )
> > +    {
> > +        printk("No boot modules found, trying flash\n");
> > +        start = KERNEL_FLASH_ADDRESS;
> > +        size = KERNEL_FLASH_SIZE;
> > +    }
> > +    else
> > +    {
> > +        printk("Loading kernel from boot module 1\n");
> > +        start = early_info.modules.module[1].start;
> > +        size = early_info.modules.module[1].size;
> 
> Do we want (here or elsewhere) to check that start is page-aligned?

I think kernel_try_zimage_prepare tries to do the right thing

Although you've made me look and I suspect it is buggy if start is <
sizeof(zimage header) from the end of a page. It should probably just
use copy_from_paddr into a local buffer.

Ian.


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