From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvkV0-0001er-B4 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 06:20:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZvkUx-000781-N3 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 06:20:34 -0500 Received: from mga14.intel.com ([192.55.52.115]:32819) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvkUx-00077U-8F for qemu-devel@nongnu.org; Mon, 09 Nov 2015 06:20:31 -0500 References: <1446184587-142784-1-git-send-email-guangrong.xiao@linux.intel.com> <1446184587-142784-20-git-send-email-guangrong.xiao@linux.intel.com> <20151109110439.GB3324@redhat.com> From: Xiao Guangrong Message-ID: <56407FE6.1010906@linux.intel.com> Date: Mon, 9 Nov 2015 19:13:42 +0800 MIME-Version: 1.0 In-Reply-To: <20151109110439.GB3324@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 19/33] dimm: keep the state of the whole backend memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: ehabkost@redhat.com, kvm@vger.kernel.org, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, imammedo@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 11/09/2015 07:04 PM, Michael S. Tsirkin wrote: > On Fri, Oct 30, 2015 at 01:56:13PM +0800, Xiao Guangrong wrote: >> QEMU keeps the state of memory of dimm device during live migration, >> however, it is not enough for nvdimm device as its memory does not >> contain its label data, so that we should protect the whole backend >> memory instead >> >> Signed-off-by: Xiao Guangrong > > It looks like there's now a difference between > host_memory_backend_get_memory and get_memory_region, > whereas previously they were exactly interchangeable. Yes, it is. host_memory_backend_get_memory() is used to get the whole backend memory region. get_memory_region() is used to get the memory region which will be mapped to guest's address space. They are on the same page for pc-dimm, but it's different for nvdimm since part of backend memory used as label data which is not directly mapped to guest. > > This needs some thought, in particular the theoretically > generic dimm.c has to do tricks to accomodate nvdimm. > > The missing piece for NVDIMM is the 128k label space at the end, > right? Can't nvdimm specific code just register that as a > separate RAM chunk in order to migrate it? Yes, it is. I thought this before, then we need to have addition callbackes: - plug(), which is called when the dimm device is plugged, then nvdimm has the chance to do it own migration things. - unplug(), let nvdimm undo the thing of migration. It needs more codes but the logic seems more clear. If you like this way, i will do. > >> --- >> hw/mem/dimm.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c >> index 498d380..44447d1 100644 >> --- a/hw/mem/dimm.c >> +++ b/hw/mem/dimm.c >> @@ -134,9 +134,16 @@ void dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, >> } >> >> memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); >> - vmstate_register_ram(mr, dev); >> numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); >> >> + /* >> + * save the state only for @mr is not enough as it does not contain >> + * the label data of NVDIMM device, so that we keep the state of >> + * whole hostmem instead. >> + */ >> + vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp), >> + dev); >> + >> out: >> error_propagate(errp, local_err); >> } >> @@ -145,10 +152,13 @@ void dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms, >> MemoryRegion *mr) >> { >> DIMMDevice *dimm = DIMM(dev); >> + MemoryRegion *backend_mr; >> + >> + backend_mr = host_memory_backend_get_memory(dimm->hostmem, &error_abort); >> >> numa_unset_mem_node_id(dimm->addr, memory_region_size(mr), dimm->node); >> memory_region_del_subregion(&hpms->mr, mr); >> - vmstate_unregister_ram(mr, dev); >> + vmstate_unregister_ram(backend_mr, dev); >> } >> >> int qmp_dimm_device_list(Object *obj, void *opaque) >> -- >> 1.8.3.1 >