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

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

[Joel Stanley]

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 <joel@jms.id.au>
---
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.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 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)
 
     /* 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);
+        }
+
         pnv->initrd_base = INITRD_LOAD_ADDR;
         pnv->initrd_size = load_image_targphys(machine->initrd_filename,
                                   pnv->initrd_base, INITRD_MAX_SIZE);
-- 
2.20.1

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

[Cédric Le Goater]

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 <joel@jms.id.au>
> ---
> 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.

> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  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);
> 

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3958 bytes --]

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 <joel@jms.id.au>
> > ---
> > 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 <joel@jms.id.au>
> > ---
> >  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Joel Stanley]

On Tue, 16 Jul 2019 at 08:53, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Tue, Jul 16, 2019 at 09:39:36AM +0200, Cédric Le Goater wrote:
> > > Provide a helpful message for users so they don't go reporting bugs to
> > > kernel developers.
> > >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > ---
> > > 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.

I agree. I'll send a patch to do that.

> > 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.

Yeah, perhaps. If someone is playing with some other bit of firmware
code then there's no reason not to continue. I'll leave this one alone
for now.

Cheers,

Joel