On Fri, 4 Apr 2014 14:14:04 -0300
Eduardo Habkost <ehabk...@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 <imamm...@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
> > 
> 


Reply via email to