Re: [Qemu-devel] [PATCH 21/27] pc: add memory hotplug 440fx machine

[Michael S. Tsirkin]

On Thu, Nov 21, 2013 at 03:34:53PM +0100, Igor Mammedov wrote:
> On Thu, 21 Nov 2013 15:13:12 +0100
> Andreas Färber <address@hidden> wrote:
> 
> > Am 21.11.2013 06:48, schrieb Li Guang:
> > > Hi, Igor
> > > 
> > > Igor Mammedov wrote:
> > >> Add DimmBus for memory hotplug below 4Gb or above 4Gb depending
> > >> on initial memory size and hotplug memory size.
> > >>    
> > > ...
> > >> +static
> > >> +void pc_hotplug_memory_init_impl(Object *owner,
> > >> +                                 MemoryRegion *system_memory,
> > >> +                                 ram_addr_t low_hotplug_mem_start,
> > >> +                                 ram_addr_t low_hotplug_mem_end,
> > >> +                                 DimmBus *hotplug_mem_bus,
> > >> +                                 ram_addr_t *high_mem_end)
> > >> +{
> > >> +    QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory-opts"),
> > >> NULL);
> > >> +    ram_addr_t ram_size = qemu_opt_get_size(opts, "mem", 0);
> > >> +    ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
> > >> +    ram_addr_t hotplug_mem_size;
> > >> +
> > >> +    if (maxmem<= ram_size) {
> > >> +        /* Disable ACPI migration code and creation of memory devices
> > >> in SSDT */
> > >>    
> > > 
> > > Why not give the memory that not be hot-added a chance to be placed on
> > > one memory slot?
> > 
> > Seconded, I believe I requested that on the previous version already.
> Because current initial memory allocation is a mess and this already
> large series would become even more large and intrusive, so far series
> it relatively not intrusive and self contained.
> 
> I believe re-factoring of initial memory to use Dimm devices should be
> done later on top of infrastructure this series provides.

Kind of makes sense, it looks like a feature request rather than
a bug report.
Maybe add some comments in code/commit logs?

> > Andreas
> > 
> > > if all memory can be hot-added and hot-removed, then we can bring in
> > > more flexibility for
> > > memory hotplug feature.
> >