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

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 02/27/18 16:37 +0000, Anthony PERARD wrote:
> 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 <haozhong.zhang@xxxxxxxxx>
> > ---
> > Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Richard Henderson <rth@xxxxxxxxxxx>
> > Cc: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>
> > ---
> >  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.

will move it to a separate patch

> 
> > +{
> > +    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?

Well, this was a debugging check and I forgot to remove it before
sending the patch. I'll drop it in the next version.

Thanks,
Haozhong

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel