From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area Date: Mon, 31 Aug 2015 14:23:43 +0800 Message-ID: <55E3F2EF.8070501@linux.intel.com> References: <1439563931-12352-1-git-send-email-guangrong.xiao@linux.intel.com> <1439563931-12352-9-git-send-email-guangrong.xiao@linux.intel.com> <20150825160353.GD8344@stefanha-thinkpad.redhat.com> <55DD979A.70804@linux.intel.com> <20150828115851.GM4917@stefanha-thinkpad.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Stefan Hajnoczi , pbonzini@redhat.com, imammedo@redhat.com, gleb@kernel.org, mtosatti@redhat.com, mst@redhat.com, rth@twiddle.net, ehabkost@redhat.com, kvm@vger.kernel.org, qemu-devel@nongnu.org To: Stefan Hajnoczi Return-path: Received: from mga14.intel.com ([192.55.52.115]:16173 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751476AbbHaG3n (ORCPT ); Mon, 31 Aug 2015 02:29:43 -0400 In-Reply-To: <20150828115851.GM4917@stefanha-thinkpad.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi Stefan, On 08/28/2015 07:58 PM, Stefan Hajnoczi wrote: > >>>> + goto do_unmap; >>>> + } >>>> + >>>> + nvdimm->device_index = new_device_index(); >>>> + sprintf(name, "NVDIMM-%d", nvdimm->device_index); >>>> + memory_region_init_ram_ptr(&nvdimm->mr, OBJECT(dev), name, nvdimm_size, >>>> + buf); >>> >>> How is the autogenerated name used? >>> >>> Why not just use "pc-nvdimm.memory"? >> >> Ah. Just for debug proposal :) and i am not sure if a name used for multiple >> MRs (MemoryRegion) is a good idea. > > Other devices use a constant name too (git grep > memory_region_init_ram_ptr) so it seems to be okay. The unique thing is > the OBJECT(dev) which differs for each NVDIMM instance. > When I was digging into live migration code, i noticed that the same MR name may cause the name "idstr", please refer to qemu_ram_set_idstr(). Since nvdimm devices do not have parent-bus, it will trigger the abort() in that function. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWIbD-0007m5-OS for qemu-devel@nongnu.org; Mon, 31 Aug 2015 02:29:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWIbA-0007nW-HT for qemu-devel@nongnu.org; Mon, 31 Aug 2015 02:29:47 -0400 Received: from mga02.intel.com ([134.134.136.20]:50765) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWIbA-0007m0-CW for qemu-devel@nongnu.org; Mon, 31 Aug 2015 02:29:44 -0400 References: <1439563931-12352-1-git-send-email-guangrong.xiao@linux.intel.com> <1439563931-12352-9-git-send-email-guangrong.xiao@linux.intel.com> <20150825160353.GD8344@stefanha-thinkpad.redhat.com> <55DD979A.70804@linux.intel.com> <20150828115851.GM4917@stefanha-thinkpad.redhat.com> From: Xiao Guangrong Message-ID: <55E3F2EF.8070501@linux.intel.com> Date: Mon, 31 Aug 2015 14:23:43 +0800 MIME-Version: 1.0 In-Reply-To: <20150828115851.GM4917@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, Stefan Hajnoczi , mtosatti@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, imammedo@redhat.com, rth@twiddle.net Hi Stefan, On 08/28/2015 07:58 PM, Stefan Hajnoczi wrote: > >>>> + goto do_unmap; >>>> + } >>>> + >>>> + nvdimm->device_index = new_device_index(); >>>> + sprintf(name, "NVDIMM-%d", nvdimm->device_index); >>>> + memory_region_init_ram_ptr(&nvdimm->mr, OBJECT(dev), name, nvdimm_size, >>>> + buf); >>> >>> How is the autogenerated name used? >>> >>> Why not just use "pc-nvdimm.memory"? >> >> Ah. Just for debug proposal :) and i am not sure if a name used for multiple >> MRs (MemoryRegion) is a good idea. > > Other devices use a constant name too (git grep > memory_region_init_ram_ptr) so it seems to be okay. The unique thing is > the OBJECT(dev) which differs for each NVDIMM instance. > When I was digging into live migration code, i noticed that the same MR name may cause the name "idstr", please refer to qemu_ram_set_idstr(). Since nvdimm devices do not have parent-bus, it will trigger the abort() in that function.