hw/ppc/pnv.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
The memory layout of the PowerNV machine is defined as :
#define KERNEL_LOAD_BASE ((void *)0x20000000)
#define KERNEL_LOAD_SIZE 0x08000000
#define INITRAMFS_LOAD_BASE KERNEL_LOAD_BASE + KERNEL_LOAD_SIZE
#define INITRAMFS_LOAD_SIZE 0x08000000
#define SKIBOOT_BASE 0x30000000
#define SKIBOOT_SIZE 0x01c10000
#define CPU_STACKS_BASE (SKIBOOT_BASE + SKIBOOT_SIZE)
#define STACK_SHIFT 15
#define STACK_SIZE (1 << STACK_SHIFT)
The overall size of the CPU stacks is (max PIR + 1) * 32K and the
machine easily reaches 800MB of minimum required RAM.
Any value below will result in a skiboot crash :
[ 0.034949905,3] MEM: Partial overlap detected between regions:
[ 0.034959039,3] MEM: ibm,firmware-stacks [0x31c10000-0x3a450000] (new)
[ 0.034968576,3] MEM: ibm,firmware-allocs-memory@0 [0x31c10000-0x38400000]
[ 0.034980367,3] Out of memory adding skiboot reserved areas
[ 0.035074945,3] ***********************************************
[ 0.035093627,3] < assert failed at core/mem_region.c:1129 >
[ 0.035104247,3] .
[ 0.035108025,3] .
[ 0.035111651,3] .
[ 0.035115231,3] OO__)
[ 0.035119198,3] <"__/
[ 0.035122980,3] ^ ^
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/ppc/pnv.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 50810df83815..77af846cdfea 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "qemu/datadir.h"
#include "qemu/units.h"
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "sysemu/qtest.h"
#include "sysemu/sysemu.h"
@@ -725,8 +726,11 @@ static void pnv_init(MachineState *machine)
DeviceState *dev;
/* allocate RAM */
- if (machine->ram_size < (1 * GiB)) {
- warn_report("skiboot may not work with < 1GB of RAM");
+ if (machine->ram_size < mc->default_ram_size) {
+ char *sz = size_to_str(mc->default_ram_size);
+ error_report("Invalid RAM size, should be bigger than %s", sz);
+ g_free(sz);
+ exit(EXIT_FAILURE);
}
memory_region_add_subregion(get_system_memory(), 0, machine->ram);
@@ -1994,7 +1998,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
* RAM defaults to less than 2048 for 32-bit hosts, and large
* enough to fit the maximum initrd size at it's load address
*/
- mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE;
+ mc->default_ram_size = 1 * GiB;
mc->default_ram_id = "pnv.ram";
ispc->print_info = pnv_pic_print_info;
nc->nmi_monitor_handler = pnv_nmi;
--
2.26.2
On Fri, 29 Jan 2021 12:17:19 +0100 Cédric Le Goater <clg@kaod.org> wrote: > The memory layout of the PowerNV machine is defined as : > > #define KERNEL_LOAD_BASE ((void *)0x20000000) > #define KERNEL_LOAD_SIZE 0x08000000 > > #define INITRAMFS_LOAD_BASE KERNEL_LOAD_BASE + KERNEL_LOAD_SIZE > #define INITRAMFS_LOAD_SIZE 0x08000000 > > #define SKIBOOT_BASE 0x30000000 > #define SKIBOOT_SIZE 0x01c10000 > > #define CPU_STACKS_BASE (SKIBOOT_BASE + SKIBOOT_SIZE) > #define STACK_SHIFT 15 > #define STACK_SIZE (1 << STACK_SHIFT) > > The overall size of the CPU stacks is (max PIR + 1) * 32K and the > machine easily reaches 800MB of minimum required RAM. > > Any value below will result in a skiboot crash : > > [ 0.034949905,3] MEM: Partial overlap detected between regions: > [ 0.034959039,3] MEM: ibm,firmware-stacks [0x31c10000-0x3a450000] (new) > [ 0.034968576,3] MEM: ibm,firmware-allocs-memory@0 [0x31c10000-0x38400000] > [ 0.034980367,3] Out of memory adding skiboot reserved areas > [ 0.035074945,3] *********************************************** > [ 0.035093627,3] < assert failed at core/mem_region.c:1129 > > [ 0.035104247,3] . > [ 0.035108025,3] . > [ 0.035111651,3] . > [ 0.035115231,3] OO__) > [ 0.035119198,3] <"__/ > [ 0.035122980,3] ^ ^ > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/pnv.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 50810df83815..77af846cdfea 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -21,6 +21,7 @@ > #include "qemu-common.h" > #include "qemu/datadir.h" > #include "qemu/units.h" > +#include "qemu/cutils.h" > #include "qapi/error.h" > #include "sysemu/qtest.h" > #include "sysemu/sysemu.h" > @@ -725,8 +726,11 @@ static void pnv_init(MachineState *machine) > DeviceState *dev; > > /* allocate RAM */ > - if (machine->ram_size < (1 * GiB)) { > - warn_report("skiboot may not work with < 1GB of RAM"); > + if (machine->ram_size < mc->default_ram_size) { > + char *sz = size_to_str(mc->default_ram_size); > + error_report("Invalid RAM size, should be bigger than %s", sz); > + g_free(sz); FWIW you could even not call g_free() at all since this is an exit path and this code is unlikely to be reused differently, or stay clean and tag 'sz' with g_autofree as recommended in CODING_STYLE. Anyway, this is definitely an improvement so: Reviewed-by: Greg Kurz <groug@kaod.org> > + exit(EXIT_FAILURE); > } > memory_region_add_subregion(get_system_memory(), 0, machine->ram); > > @@ -1994,7 +1998,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > * RAM defaults to less than 2048 for 32-bit hosts, and large > * enough to fit the maximum initrd size at it's load address > */ > - mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > + mc->default_ram_size = 1 * GiB; > mc->default_ram_id = "pnv.ram"; > ispc->print_info = pnv_pic_print_info; > nc->nmi_monitor_handler = pnv_nmi;
On 1/29/21 12:41 PM, Greg Kurz wrote: > On Fri, 29 Jan 2021 12:17:19 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> The memory layout of the PowerNV machine is defined as : >> >> #define KERNEL_LOAD_BASE ((void *)0x20000000) >> #define KERNEL_LOAD_SIZE 0x08000000 >> >> #define INITRAMFS_LOAD_BASE KERNEL_LOAD_BASE + KERNEL_LOAD_SIZE >> #define INITRAMFS_LOAD_SIZE 0x08000000 >> >> #define SKIBOOT_BASE 0x30000000 >> #define SKIBOOT_SIZE 0x01c10000 >> >> #define CPU_STACKS_BASE (SKIBOOT_BASE + SKIBOOT_SIZE) >> #define STACK_SHIFT 15 >> #define STACK_SIZE (1 << STACK_SHIFT) >> >> The overall size of the CPU stacks is (max PIR + 1) * 32K and the >> machine easily reaches 800MB of minimum required RAM. >> >> Any value below will result in a skiboot crash : >> >> [ 0.034949905,3] MEM: Partial overlap detected between regions: >> [ 0.034959039,3] MEM: ibm,firmware-stacks [0x31c10000-0x3a450000] (new) >> [ 0.034968576,3] MEM: ibm,firmware-allocs-memory@0 [0x31c10000-0x38400000] >> [ 0.034980367,3] Out of memory adding skiboot reserved areas >> [ 0.035074945,3] *********************************************** >> [ 0.035093627,3] < assert failed at core/mem_region.c:1129 > >> [ 0.035104247,3] . >> [ 0.035108025,3] . >> [ 0.035111651,3] . >> [ 0.035115231,3] OO__) >> [ 0.035119198,3] <"__/ >> [ 0.035122980,3] ^ ^ >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/pnv.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 50810df83815..77af846cdfea 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -21,6 +21,7 @@ >> #include "qemu-common.h" >> #include "qemu/datadir.h" >> #include "qemu/units.h" >> +#include "qemu/cutils.h" >> #include "qapi/error.h" >> #include "sysemu/qtest.h" >> #include "sysemu/sysemu.h" >> @@ -725,8 +726,11 @@ static void pnv_init(MachineState *machine) >> DeviceState *dev; >> >> /* allocate RAM */ >> - if (machine->ram_size < (1 * GiB)) { >> - warn_report("skiboot may not work with < 1GB of RAM"); >> + if (machine->ram_size < mc->default_ram_size) { >> + char *sz = size_to_str(mc->default_ram_size); >> + error_report("Invalid RAM size, should be bigger than %s", sz); >> + g_free(sz); > > FWIW you could even not call g_free() at all since this is an > exit path and this code is unlikely to be reused differently, > or stay clean and tag 'sz' with g_autofree as recommended in > CODING_STYLE. Indeed. I copied that part from another machine and it should disappear if we introduce a 'min_ram_size' MachineClass attribute. > Anyway, this is definitely an improvement so: > > Reviewed-by: Greg Kurz <groug@kaod.org> Thanks, C. >> + exit(EXIT_FAILURE); >> } >> memory_region_add_subregion(get_system_memory(), 0, machine->ram); >> >> @@ -1994,7 +1998,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) >> * RAM defaults to less than 2048 for 32-bit hosts, and large >> * enough to fit the maximum initrd size at it's load address >> */ >> - mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; >> + mc->default_ram_size = 1 * GiB; >> mc->default_ram_id = "pnv.ram"; >> ispc->print_info = pnv_pic_print_info; >> nc->nmi_monitor_handler = pnv_nmi; >
On 1/29/21 12:41 PM, Greg Kurz wrote: > On Fri, 29 Jan 2021 12:17:19 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> The memory layout of the PowerNV machine is defined as : >> >> #define KERNEL_LOAD_BASE ((void *)0x20000000) >> #define KERNEL_LOAD_SIZE 0x08000000 >> >> #define INITRAMFS_LOAD_BASE KERNEL_LOAD_BASE + KERNEL_LOAD_SIZE >> #define INITRAMFS_LOAD_SIZE 0x08000000 >> >> #define SKIBOOT_BASE 0x30000000 >> #define SKIBOOT_SIZE 0x01c10000 >> >> #define CPU_STACKS_BASE (SKIBOOT_BASE + SKIBOOT_SIZE) >> #define STACK_SHIFT 15 >> #define STACK_SIZE (1 << STACK_SHIFT) >> >> The overall size of the CPU stacks is (max PIR + 1) * 32K and the >> machine easily reaches 800MB of minimum required RAM. >> >> Any value below will result in a skiboot crash : >> >> [ 0.034949905,3] MEM: Partial overlap detected between regions: >> [ 0.034959039,3] MEM: ibm,firmware-stacks [0x31c10000-0x3a450000] (new) >> [ 0.034968576,3] MEM: ibm,firmware-allocs-memory@0 [0x31c10000-0x38400000] >> [ 0.034980367,3] Out of memory adding skiboot reserved areas >> [ 0.035074945,3] *********************************************** >> [ 0.035093627,3] < assert failed at core/mem_region.c:1129 > >> [ 0.035104247,3] . >> [ 0.035108025,3] . >> [ 0.035111651,3] . >> [ 0.035115231,3] OO__) >> [ 0.035119198,3] <"__/ >> [ 0.035122980,3] ^ ^ >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/pnv.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) ... >> /* allocate RAM */ >> - if (machine->ram_size < (1 * GiB)) { >> - warn_report("skiboot may not work with < 1GB of RAM"); >> + if (machine->ram_size < mc->default_ram_size) { >> + char *sz = size_to_str(mc->default_ram_size); >> + error_report("Invalid RAM size, should be bigger than %s", sz); >> + g_free(sz); > > FWIW you could even not call g_free() at all since this is an > exit path and this code is unlikely to be reused differently, Well, some tests check for invalid configs, and when run with --enable-sanitizer they'd complain for memory leak, so better free IMHO. > or stay clean and tag 'sz' with g_autofree as recommended in > CODING_STYLE. Preferably using g_autofree: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Anyway, this is definitely an improvement so: > > Reviewed-by: Greg Kurz <groug@kaod.org>
On Fri, Jan 29, 2021 at 12:17:19PM +0100, Cédric Le Goater wrote: 65;6201;1c> The memory layout of the PowerNV machine is defined as : > > #define KERNEL_LOAD_BASE ((void *)0x20000000) > #define KERNEL_LOAD_SIZE 0x08000000 > > #define INITRAMFS_LOAD_BASE KERNEL_LOAD_BASE + KERNEL_LOAD_SIZE > #define INITRAMFS_LOAD_SIZE 0x08000000 > > #define SKIBOOT_BASE 0x30000000 > #define SKIBOOT_SIZE 0x01c10000 > > #define CPU_STACKS_BASE (SKIBOOT_BASE + SKIBOOT_SIZE) > #define STACK_SHIFT 15 > #define STACK_SIZE (1 << STACK_SHIFT) > > The overall size of the CPU stacks is (max PIR + 1) * 32K and the > machine easily reaches 800MB of minimum required RAM. > > Any value below will result in a skiboot crash : > > [ 0.034949905,3] MEM: Partial overlap detected between regions: > [ 0.034959039,3] MEM: ibm,firmware-stacks [0x31c10000-0x3a450000] (new) > [ 0.034968576,3] MEM: ibm,firmware-allocs-memory@0 [0x31c10000-0x38400000] > [ 0.034980367,3] Out of memory adding skiboot reserved areas > [ 0.035074945,3] *********************************************** > [ 0.035093627,3] < assert failed at core/mem_region.c:1129 > > [ 0.035104247,3] . > [ 0.035108025,3] . > [ 0.035111651,3] . > [ 0.035115231,3] OO__) > [ 0.035119198,3] <"__/ > [ 0.035122980,3] ^ ^ > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Applied to ppc-for-6.0, thanks. > --- > hw/ppc/pnv.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 50810df83815..77af846cdfea 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -21,6 +21,7 @@ > #include "qemu-common.h" > #include "qemu/datadir.h" > #include "qemu/units.h" > +#include "qemu/cutils.h" > #include "qapi/error.h" > #include "sysemu/qtest.h" > #include "sysemu/sysemu.h" > @@ -725,8 +726,11 @@ static void pnv_init(MachineState *machine) > DeviceState *dev; > > /* allocate RAM */ > - if (machine->ram_size < (1 * GiB)) { > - warn_report("skiboot may not work with < 1GB of RAM"); > + if (machine->ram_size < mc->default_ram_size) { > + char *sz = size_to_str(mc->default_ram_size); > + error_report("Invalid RAM size, should be bigger than %s", sz); > + g_free(sz); > + exit(EXIT_FAILURE); > } > memory_region_add_subregion(get_system_memory(), 0, machine->ram); > > @@ -1994,7 +1998,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > * RAM defaults to less than 2048 for 32-bit hosts, and large > * enough to fit the maximum initrd size at it's load address > */ > - mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > + mc->default_ram_size = 1 * GiB; > mc->default_ram_id = "pnv.ram"; > ispc->print_info = pnv_pic_print_info; > nc->nmi_monitor_handler = pnv_nmi; -- 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
© 2016 - 2024 Red Hat, Inc.