Re: [Qemu-devel] [PATCH] ppc/pnv: Warn when using -initrd and low ram

[David Gibson]

On Tue, Jul 16, 2019 at 09:39:36AM +0200, Cédric Le Goater wrote:
> On 16/07/2019 06:56, Joel Stanley wrote:
> > When booting with the default amount of RAM the powernv machine will
> > load the initrd above the top of RAM and cause the Linux kernel to crash
> > when it attempts to access the initrd:
> > 
> >   Linux/PowerPC load:
> >   Finalizing device tree... flat tree at 0x202770c0
> >   [    0.070476] nvram: Failed to find or create lnx,oops-log partition, 
> > err -28
> >   [    0.073270] nvram: Failed to initialize oops partition!
> >   [    0.156302] BUG: Unable to handle kernel data access at 
> > 0xc000000060000000
> >   [    0.158009] Faulting instruction address: 0xc000000001002e5c
> >   cpu 0x0: Vector: 300 (Data Access) at [c00000003d1e3870]
> >       pc: c000000001002e5c: unpack_to_rootfs+0xdc/0x2f0
> >       lr: c000000001002df4: unpack_to_rootfs+0x74/0x2f0
> >       sp: c00000003d1e3b00
> >      msr: 9000000002009033
> >      dar: c000000060000000
> >    dsisr: 40000000
> >     current = 0xc00000003d1c0000
> >     paca    = 0xc000000001320000     irqmask: 0x03   irq_happened: 0x01
> >       pid   = 1, comm = swapper/0
> >   Linux version 5.2.0-10292-g040e2e618374 (joel@voyager) (gcc version 8.3.0 
> > (Debian 8.3.0-2)) #1 SMP Tue Jul 16 13:50:32 ACST 2019
> >   enter ? for help
> >   [c00000003d1e3bb0] c000000001003c90 populate_rootfs+0x84/0x1dc
> >   [c00000003d1e3c40] c00000000000f494 do_one_initcall+0x88/0x1d0
> >   [c00000003d1e3d10] c000000001000fc4 kernel_init_freeable+0x24c/0x250
> >   [c00000003d1e3db0] c00000000000f7a0 kernel_init+0x1c/0x150
> >   [c00000003d1e3e20] c00000000000b8a4 ret_from_kernel_thread+0x5c/0x78
> > 
> > Provide a helpful message for users so they don't go reporting bugs to
> > kernel developers.
> > 
> > Signed-off-by: Joel Stanley <address@hidden>
> > ---
> > We could solve this in other ways, such as warn when loading the initrd
> > outside of RAM, or load it within the known boundaries or RAM, but after
> > hitting this myself I wanted to start the discussion.
> 
> We should also increase : 
> 
>     mc->default_ram_size = 1 * GiB;
> 
> to 2 or 4 GiB. I always use 4.

It seems to be increasing the default addresses the real problem in
practice.  Putting in a warning but still letting you do it, rather
than relocating where we load the image based on the ram size seems
kind of roundabout.

> 
> > Signed-off-by: Joel Stanley <address@hidden>
> > ---
> >  hw/ppc/pnv.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index bd4531c82260..bbd596ab9eca 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -649,6 +649,13 @@ static void pnv_init(MachineState *machine)
> 
> 
> at the beginning of this routine we have :
> 
>     /* allocate RAM */
>     if (machine->ram_size < (1 * GiB)) {
>         warn_report("skiboot may not work with < 1GB of RAM");
>     }
> 
> and we should exit instead. 
> 
> >      /* load initrd */
> >      if (machine->initrd_filename) {
> > +        if (machine->ram_size <= (1.5 * GiB)) {
> > +            /* INITRD_LOAD_ADDR is at 1.5GB, so we require at least that 
> > much RAM
> > +             * when specifying the initrd on the command line */
> > +            warn_report("initrd load requires > %ld MB of RAM",
> > +                    INITRD_LOAD_ADDR / MiB);
> > +        }
> 
> Shouldn't we take into account the initrd size also ? I don't know if it is 
> relevant as it can be compressed.
> 
> >          pnv->initrd_base = INITRD_LOAD_ADDR;
> >          pnv->initrd_size = load_image_targphys(machine->initrd_filename,
> >                                    pnv->initrd_base, INITRD_MAX_SIZE);
> > 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature