Re: [Qemu-devel] [RFC QEMU PATCH v4 02/10] xen-hvm: create the hotplug memory region on Xen

[Anthony PERARD]

On Thu, Dec 07, 2017 at 06:18:04PM +0800, Haozhong Zhang wrote:
> The guest physical address of vNVDIMM is allocated from the hotplug
> memory region, which is not created when QEMU is used as Xen device
> model. In order to use vNVDIMM for Xen HVM domains, this commit reuses
> the code for pc machine type to create the hotplug memory region for
> Xen HVM domains.
> 
> Signed-off-by: Haozhong Zhang <address@hidden>
> ---
> Cc: "Michael S. Tsirkin" <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Richard Henderson <address@hidden>
> Cc: Eduardo Habkost <address@hidden>
> Cc: Stefano Stabellini <address@hidden>
> Cc: Anthony Perard <address@hidden>
> ---
>  hw/i386/pc.c          | 86 
> ++++++++++++++++++++++++++++-----------------------
>  hw/i386/xen/xen-hvm.c |  2 ++
>  include/hw/i386/pc.h  |  1 +
>  3 files changed, 51 insertions(+), 38 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 186545d2a4..9f46c8df79 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1315,6 +1315,53 @@ void xen_load_linux(PCMachineState *pcms)
>      pcms->fw_cfg = fw_cfg;
>  }
>  
> +void pc_memory_hotplug_init(PCMachineState *pcms, MemoryRegion 
> *system_memory)

It might be better to have a separate patch which move the code into a function.

> +{
> +    MachineState *machine = MACHINE(pcms);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> +    ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
> +
> +    if (!pcmc->has_reserved_memory || machine->ram_size >= 
> machine->maxram_size)
> +        return;
> +
> +    if (memory_region_size(&pcms->hotplug_memory.mr)) {

This new check looks like to catch programming error, rather than user
error. Would it be better to be an assert instead?

> +        error_report("hotplug memory region has been initialized");
> +        exit(EXIT_FAILURE);
> +    }
> +

-- 
Anthony PERARD