| Message ID | 1396618620-27823-15-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 04, 2014 at 03:36:39PM +0200, Igor Mammedov wrote: > ... which is current ACPI implementation limit. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> With this we will have yet another hardcoded limit/assumption scattererd in different places around the code, which we won't know how/if it can be changed in the future. I would prefer to have a #define and an explanation on the code for the specific limit, so people know if/when/how it is safe to change it a few years from now. Something like: hw/acpi/acpi.h: /* current device naming scheme dosen't support * more that 256 memory devices */ #define ACPI_MAX_RAM_SLOTS 256 hw/acpi/acpi-build.c:build_ssdt(): assert(nr_mem <= ACPI_MAX_RAM_SLOTS) hw/i386/pc.c: if (machine->init_args.ram_slots > ACPI_MAX_RAM_SLOTS) { error_report("unsupported amount of memory slots: %"PRIu64, machine->init_args.ram_slots); exit(EXIT_FAILURE); } > --- > hw/i386/pc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 69e4225..6fe1803 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1204,6 +1204,12 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, > ram_addr_t hotplug_mem_size = > machine->init_args.maxram_size - ram_size; > > + if (machine->init_args.ram_slots > 256) { > + error_report("unsupported amount of memory slots: %"PRIu64, > + machine->init_args.ram_slots); > + exit(EXIT_FAILURE); > + } > + > pcms->hotplug_memory_base = > ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30); > > -- > 1.9.0 >
On Fri, 4 Apr 2014 14:14:04 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Fri, Apr 04, 2014 at 03:36:39PM +0200, Igor Mammedov wrote: > > ... which is current ACPI implementation limit. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > With this we will have yet another hardcoded limit/assumption scattererd > in different places around the code, which we won't know how/if it can > be changed in the future. > > I would prefer to have a #define and an explanation on the code for the > specific limit, so people know if/when/how it is safe to change it a few > years from now. > > Something like: > > hw/acpi/acpi.h: > /* current device naming scheme dosen't support > * more that 256 memory devices > */ > #define ACPI_MAX_RAM_SLOTS 256 > > hw/acpi/acpi-build.c:build_ssdt(): > assert(nr_mem <= ACPI_MAX_RAM_SLOTS) > > hw/i386/pc.c: > if (machine->init_args.ram_slots > ACPI_MAX_RAM_SLOTS) { > error_report("unsupported amount of memory slots: %"PRIu64, > machine->init_args.ram_slots); > exit(EXIT_FAILURE); > } sure, I'll fix it for the next series respin. > > > --- > > hw/i386/pc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 69e4225..6fe1803 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1204,6 +1204,12 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, > > ram_addr_t hotplug_mem_size = > > machine->init_args.maxram_size - ram_size; > > > > + if (machine->init_args.ram_slots > 256) { > > + error_report("unsupported amount of memory slots: %"PRIu64, > > + machine->init_args.ram_slots); > > + exit(EXIT_FAILURE); > > + } > > + > > pcms->hotplug_memory_base = > > ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30); > > > > -- > > 1.9.0 > > >
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 69e4225..6fe1803 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1204,6 +1204,12 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, ram_addr_t hotplug_mem_size = machine->init_args.maxram_size - ram_size; + if (machine->init_args.ram_slots > 256) { + error_report("unsupported amount of memory slots: %"PRIu64, + machine->init_args.ram_slots); + exit(EXIT_FAILURE); + } + pcms->hotplug_memory_base = ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30);
... which is current ACPI implementation limit. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/pc.c | 6 ++++++ 1 file changed, 6 insertions(+)