| Message ID | 20190325042929.27508-2-ruscur@russell.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] external/mambo: Populate kernel-base-address in the DT | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (b392d785eb49630b9f00fef8d17944ed82b2c1fe) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Mon, Mar 25, 2019 at 3:39 PM Russell Currey <ruscur@russell.cc> wrote: > > If you're trying to boot a gigantic kernel in mambo (which you can > reproduce by building a kernel with CONFIG_MODULES=n) you'll get > misleading errors like: > > WARNING: 0: (0): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] > WARNING: 0: (0): PC(EA): 0x0000000030000010 PC(RA):0x0000000030000010 MSR: 0x9000000000000000 LR: 0x0000000000000000 > WARNING: 0: (0): numInstructions = 0 > WARNING: 1: (1): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] > WARNING: 1: (1): PC(EA): 0x0000000000000E40 PC(RA):0x0000000000000E40 MSR: 0x9000000000000000 LR: 0x0000000000000000 > WARNING: 1: (1): numInstructions = 1 > WARNING: 1: (1): Interrupt to 0x0000000000000E40 from 0x0000000000000E40 > INFO: 1: (2): ** Execution stopped: Continuous Interrupt, Instruction caused exception, ** > > So add an error to skiboot.tcl to warn the user before this happens. > Making PAYLOAD_ADDR further back is one way to do this but if there's a > less gross way to generally work around this very niche problem, I can > suggest that instead. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- > external/mambo/skiboot.tcl | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/external/mambo/skiboot.tcl b/external/mambo/skiboot.tcl > index 096f1c34..7a939bfb 100644 > --- a/external/mambo/skiboot.tcl > +++ b/external/mambo/skiboot.tcl > @@ -552,6 +552,10 @@ mysim memory fread $mconf(boot_load) $boot_size $mconf(boot_image) > set payload_size [file size $mconf(payload)] > mysim memory fread $mconf(payload_addr) $payload_size $mconf(payload) > > +if { $payload_size > [expr $mconf(boot_load) - $mconf(payload_addr)] } { > + error "vmlinux is too large, consider adjusting PAYLOAD_ADDR" > +} Can we just load the kernel at zero? 768MB really ought to be enough for anyone... > + > # Flatten it > epapr::of2dtb mysim $mconf(epapr_dt_addr) > > -- > 2.21.0 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On Mon, 2019-03-25 at 15:49 +1100, Oliver wrote: > On Mon, Mar 25, 2019 at 3:39 PM Russell Currey <ruscur@russell.cc> > wrote: > > If you're trying to boot a gigantic kernel in mambo (which you can > > reproduce by building a kernel with CONFIG_MODULES=n) you'll get > > misleading errors like: > > > > WARNING: 0: (0): [0:0]: Invalid/unsupported instr > > 0x00000000[INVALID] > > WARNING: 0: (0): PC(EA): 0x0000000030000010 > > PC(RA):0x0000000030000010 MSR: 0x9000000000000000 LR: > > 0x0000000000000000 > > WARNING: 0: (0): numInstructions = 0 > > WARNING: 1: (1): [0:0]: Invalid/unsupported instr > > 0x00000000[INVALID] > > WARNING: 1: (1): PC(EA): 0x0000000000000E40 > > PC(RA):0x0000000000000E40 MSR: 0x9000000000000000 LR: > > 0x0000000000000000 > > WARNING: 1: (1): numInstructions = 1 > > WARNING: 1: (1): Interrupt to 0x0000000000000E40 from > > 0x0000000000000E40 > > INFO: 1: (2): ** Execution stopped: Continuous Interrupt, > > Instruction caused exception, ** > > > > So add an error to skiboot.tcl to warn the user before this > > happens. > > Making PAYLOAD_ADDR further back is one way to do this but if > > there's a > > less gross way to generally work around this very niche problem, I > > can > > suggest that instead. > > > > Signed-off-by: Russell Currey <ruscur@russell.cc> > > --- > > external/mambo/skiboot.tcl | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/external/mambo/skiboot.tcl > > b/external/mambo/skiboot.tcl > > index 096f1c34..7a939bfb 100644 > > --- a/external/mambo/skiboot.tcl > > +++ b/external/mambo/skiboot.tcl > > @@ -552,6 +552,10 @@ mysim memory fread $mconf(boot_load) > > $boot_size $mconf(boot_image) > > set payload_size [file size $mconf(payload)] > > mysim memory fread $mconf(payload_addr) $payload_size > > $mconf(payload) > > > > +if { $payload_size > [expr $mconf(boot_load) - > > $mconf(payload_addr)] } { > > + error "vmlinux is too large, consider adjusting > > PAYLOAD_ADDR" > > +} > > Can we just load the kernel at zero? 768MB really ought to be enough > for anyone... I didn't change the address since I don't know why it is the way it is currently. Even so we should keep the error just in case some mad lad makes a gigabyte kernel. > > > + > > # Flatten it > > epapr::of2dtb mysim $mconf(epapr_dt_addr) > > > > -- > > 2.21.0 > > > > _______________________________________________ > > Skiboot mailing list > > Skiboot@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/skiboot
On Mon, Mar 25, 2019 at 3:51 PM Russell Currey <ruscur@russell.cc> wrote: > > On Mon, 2019-03-25 at 15:49 +1100, Oliver wrote: > > On Mon, Mar 25, 2019 at 3:39 PM Russell Currey <ruscur@russell.cc> > > wrote: > > > If you're trying to boot a gigantic kernel in mambo (which you can > > > reproduce by building a kernel with CONFIG_MODULES=n) you'll get > > > misleading errors like: > > > > > > WARNING: 0: (0): [0:0]: Invalid/unsupported instr > > > 0x00000000[INVALID] > > > WARNING: 0: (0): PC(EA): 0x0000000030000010 > > > PC(RA):0x0000000030000010 MSR: 0x9000000000000000 LR: > > > 0x0000000000000000 > > > WARNING: 0: (0): numInstructions = 0 > > > WARNING: 1: (1): [0:0]: Invalid/unsupported instr > > > 0x00000000[INVALID] > > > WARNING: 1: (1): PC(EA): 0x0000000000000E40 > > > PC(RA):0x0000000000000E40 MSR: 0x9000000000000000 LR: > > > 0x0000000000000000 > > > WARNING: 1: (1): numInstructions = 1 > > > WARNING: 1: (1): Interrupt to 0x0000000000000E40 from > > > 0x0000000000000E40 > > > INFO: 1: (2): ** Execution stopped: Continuous Interrupt, > > > Instruction caused exception, ** > > > > > > So add an error to skiboot.tcl to warn the user before this > > > happens. > > > Making PAYLOAD_ADDR further back is one way to do this but if > > > there's a > > > less gross way to generally work around this very niche problem, I > > > can > > > suggest that instead. > > > > > > Signed-off-by: Russell Currey <ruscur@russell.cc> > > > --- > > > external/mambo/skiboot.tcl | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/external/mambo/skiboot.tcl > > > b/external/mambo/skiboot.tcl > > > index 096f1c34..7a939bfb 100644 > > > --- a/external/mambo/skiboot.tcl > > > +++ b/external/mambo/skiboot.tcl > > > @@ -552,6 +552,10 @@ mysim memory fread $mconf(boot_load) > > > $boot_size $mconf(boot_image) > > > set payload_size [file size $mconf(payload)] > > > mysim memory fread $mconf(payload_addr) $payload_size > > > $mconf(payload) > > > > > > +if { $payload_size > [expr $mconf(boot_load) - > > > $mconf(payload_addr)] } { > > > + error "vmlinux is too large, consider adjusting > > > PAYLOAD_ADDR" > > > +} > > > > Can we just load the kernel at zero? 768MB really ought to be enough > > for anyone... > > I didn't change the address since I don't know why it is the way it is > currently. Even so we should keep the error just in case some mad lad > makes a gigabyte kernel. Oh sure, I was just wondering why we don't do that currently. Actually, come to think of it, it's probably because if you load a zImage won't have space to extract the vmlinux into. <bikeshed> maybe mention in the error message that it's overlapping the skiboot image instead of saying "lolnope"</bikeshed> > > > > > > + > > > # Flatten it > > > epapr::of2dtb mysim $mconf(epapr_dt_addr) > > > > > > -- > > > 2.21.0 > > > > > > _______________________________________________ > > > Skiboot mailing list > > > Skiboot@lists.ozlabs.org > > > https://lists.ozlabs.org/listinfo/skiboot >
On Mon, 2019-03-25 at 15:29 +1100, Russell Currey wrote: > If you're trying to boot a gigantic kernel in mambo (which you can > reproduce by building a kernel with CONFIG_MODULES=n) you'll get > misleading errors like: > > WARNING: 0: (0): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] > WARNING: 0: (0): PC(EA): 0x0000000030000010 PC(RA):0x0000000030000010 MSR: > 0x9000000000000000 LR: 0x0000000000000000 > WARNING: 0: (0): numInstructions = 0 > WARNING: 1: (1): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] > WARNING: 1: (1): PC(EA): 0x0000000000000E40 PC(RA):0x0000000000000E40 MSR: > 0x9000000000000000 LR: 0x0000000000000000 > WARNING: 1: (1): numInstructions = 1 > WARNING: 1: (1): Interrupt to 0x0000000000000E40 from 0x0000000000000E40 > INFO: 1: (2): ** Execution stopped: Continuous Interrupt, Instruction caused > exception, ** > > So add an error to skiboot.tcl to warn the user before this happens. > Making PAYLOAD_ADDR further back is one way to do this but if there's a > less gross way to generally work around this very niche problem, I can > suggest that instead. I think this works! > Signed-off-by: Russell Currey <ruscur@russell.cc> Acked-by: Michael Neuling <mikey@neuling.org> > --- > external/mambo/skiboot.tcl | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/external/mambo/skiboot.tcl b/external/mambo/skiboot.tcl > index 096f1c34..7a939bfb 100644 > --- a/external/mambo/skiboot.tcl > +++ b/external/mambo/skiboot.tcl > @@ -552,6 +552,10 @@ mysim memory fread $mconf(boot_load) $boot_size > $mconf(boot_image) > set payload_size [file size $mconf(payload)] > mysim memory fread $mconf(payload_addr) $payload_size $mconf(payload) > > +if { $payload_size > [expr $mconf(boot_load) - $mconf(payload_addr)] } { > + > +} > + > # Flatten it > epapr::of2dtb mysim $mconf(epapr_dt_addr) >
diff --git a/external/mambo/skiboot.tcl b/external/mambo/skiboot.tcl index 096f1c34..7a939bfb 100644 --- a/external/mambo/skiboot.tcl +++ b/external/mambo/skiboot.tcl @@ -552,6 +552,10 @@ mysim memory fread $mconf(boot_load) $boot_size $mconf(boot_image) set payload_size [file size $mconf(payload)] mysim memory fread $mconf(payload_addr) $payload_size $mconf(payload) +if { $payload_size > [expr $mconf(boot_load) - $mconf(payload_addr)] } { + error "vmlinux is too large, consider adjusting PAYLOAD_ADDR" +} + # Flatten it epapr::of2dtb mysim $mconf(epapr_dt_addr)
If you're trying to boot a gigantic kernel in mambo (which you can reproduce by building a kernel with CONFIG_MODULES=n) you'll get misleading errors like: WARNING: 0: (0): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] WARNING: 0: (0): PC(EA): 0x0000000030000010 PC(RA):0x0000000030000010 MSR: 0x9000000000000000 LR: 0x0000000000000000 WARNING: 0: (0): numInstructions = 0 WARNING: 1: (1): [0:0]: Invalid/unsupported instr 0x00000000[INVALID] WARNING: 1: (1): PC(EA): 0x0000000000000E40 PC(RA):0x0000000000000E40 MSR: 0x9000000000000000 LR: 0x0000000000000000 WARNING: 1: (1): numInstructions = 1 WARNING: 1: (1): Interrupt to 0x0000000000000E40 from 0x0000000000000E40 INFO: 1: (2): ** Execution stopped: Continuous Interrupt, Instruction caused exception, ** So add an error to skiboot.tcl to warn the user before this happens. Making PAYLOAD_ADDR further back is one way to do this but if there's a less gross way to generally work around this very niche problem, I can suggest that instead. Signed-off-by: Russell Currey <ruscur@russell.cc> --- external/mambo/skiboot.tcl | 4 ++++ 1 file changed, 4 insertions(+)