[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC QEMU PATCH v4 02/10] xen-hvm: create the hotplug m
From: |
Haozhong Zhang |
Subject: |
Re: [Qemu-devel] [RFC QEMU PATCH v4 02/10] xen-hvm: create the hotplug memory region on Xen |
Date: |
Wed, 28 Feb 2018 15:47:58 +0800 |
User-agent: |
NeoMutt/20171027 |
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 <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.
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