Re: [PULL 006/136] vl.c: move -m parsing after memory backends has been processed

[Igor Mammedov]

On Thu, 26 Mar 2020 10:20:31 +0100
Auger Eric <address@hidden> wrote:

> Hi
> 
> On 2/25/20 12:48 PM, Paolo Bonzini wrote:
> > From: Igor Mammedov <address@hidden>
> > 
> > It will be possible for main RAM to come from memory-backend
> > and we should check that size specified in -m matches the size
> > of the backend and [MachineState::]ram_size also matches
> > backend's size.
> > 
> > However -m parsing (set_memory_options()) happens before backends
> > are intialized (object_create_delayed()) which complicates it.
> > Consolidate set_memory_options() and assigning parsed results to
> > current_machine after backends are initialized, so it would be
> > possible access the initialized backend instance to compare
> > sizes.
> > 
> > This patch only consolidates scattered places touching ram_size
> > within vl.c. And follow up patch will integrate backend handling
> > to set_memory_options().
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > Message-Id: <address@hidden>  
> 
> Unfortunately this patch breaks ARM virt memory map setting in KVM mode.
> If you launch ARM virt with PCDIMM you now get
> 
> qemu-system-aarch64: -device
> pc-dimm,memdev=mem1,id=dimm1,driver=pc-dimm: memory devices (e.g. for
> memory hotplug) are not supported by the machine
> 
> With machvirt/KVM
> configure_accelerators() calls ARM virt_kvm_type(). This function
> freezes the machine memory map and computes the size of the IPA to be
> supported by KVM. See:
> c9650222b8 ("hw/arm/virt: Implement kvm_type function for 4.0 machine")
> 
> virt_kvm_type() uses machine memory settings. Igor's patch moves those
> after the mem backend init, so the virt memory map does not properly
> take into account the machine memory settings anymore.
> 
> So we need to find a way to rework this.
set_memory_options() was already reverted, so  safest way to fix it during
freeze is to revert back lines

-    current_machine->ram_size = ram_size;
-    current_machine->maxram_size = maxram_size;
-    current_machine->ram_slots = ram_slots;

I'll post a patch

> 
> Thanks
> 
> Eric
> 
> 
> > ---
> >  vl.c | 27 ++++++++++++++-------------
> >  1 file changed, 14 insertions(+), 13 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index 2103804..72ffc06 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2655,6 +2655,14 @@ static void set_memory_options(uint64_t *ram_slots, 
> > ram_addr_t *maxram_size,
> >          exit(EXIT_FAILURE);
> >      }
> >  
> > +    if (!xen_enabled()) {
> > +        /* On 32-bit hosts, QEMU is limited by virtual address space */
> > +        if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
> > +            error_report("at most 2047 MB RAM can be simulated");
> > +            exit(1);
> > +        }
> > +    }
> > +
> >      loc_pop(&loc);
> >  }
> >  
> > @@ -3819,8 +3827,6 @@ int main(int argc, char **argv, char **envp)
> >      machine_class = select_machine();
> >      object_set_machine_compat_props(machine_class->compat_props);
> >  
> > -    set_memory_options(&ram_slots, &maxram_size, machine_class);
> > -
> >      os_daemonize();
> >      rcu_disable_atfork();
> >  
> > @@ -4122,9 +4128,6 @@ int main(int argc, char **argv, char **envp)
> >      machine_opts = qemu_get_machine_opts();
> >      qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
> >                       &error_fatal);
> > -    current_machine->ram_size = ram_size;
> > -    current_machine->maxram_size = maxram_size;
> > -    current_machine->ram_slots = ram_slots;
> >  
> >      /*
> >       * Note: uses machine properties such as kernel-irqchip, must run
> > @@ -4235,14 +4238,6 @@ int main(int argc, char **argv, char **envp)
> >  
> >      tpm_init();
> >  
> > -    if (!xen_enabled()) {
> > -        /* On 32-bit hosts, QEMU is limited by virtual address space */
> > -        if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
> > -            error_report("at most 2047 MB RAM can be simulated");
> > -            exit(1);
> > -        }
> > -    }
> > -
> >      blk_mig_init();
> >      ram_mig_init();
> >      dirty_bitmap_mig_init();
> > @@ -4287,6 +4282,12 @@ int main(int argc, char **argv, char **envp)
> >      if (cpu_option) {
> >          current_machine->cpu_type = parse_cpu_option(cpu_option);
> >      }
> > +
> > +    set_memory_options(&ram_slots, &maxram_size, machine_class);
> > +    current_machine->ram_size = ram_size;
> > +    current_machine->maxram_size = maxram_size;
> > +    current_machine->ram_slots = ram_slots;
> > +
> >      parse_numa_opts(current_machine);
> >  
> >      if (machine_class->default_ram_id && current_machine->ram_size &&
> >