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

[Auger Eric <eric.auger@redhat.com>]

Hi

On 2/25/20 12:48 PM, Paolo Bonzini wrote:
> From: Igor Mammedov <imammedo@redhat.com>
> 
> 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 <imammedo@redhat.com>
> Message-Id: <20200219160953.13771-7-imammedo@redhat.com>

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.

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 &&
>