From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42786) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ab9ML-0004Yy-WD for qemu-devel@nongnu.org; Wed, 02 Mar 2016 11:10:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ab9MH-00028b-Bk for qemu-devel@nongnu.org; Wed, 02 Mar 2016 11:10:45 -0500 Received: from mga11.intel.com ([192.55.52.93]:4543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ab9MG-000284-SQ for qemu-devel@nongnu.org; Wed, 02 Mar 2016 11:10:41 -0500 References: <1456919441-101204-1-git-send-email-guangrong.xiao@linux.intel.com> <1456919441-101204-2-git-send-email-guangrong.xiao@linux.intel.com> <20160302135630-mutt-send-email-mst@redhat.com> From: Xiao Guangrong Message-ID: <56D71071.8050802@linux.intel.com> Date: Thu, 3 Mar 2016 00:10:25 +0800 MIME-Version: 1.0 In-Reply-To: <20160302135630-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 1/5] nvdimm acpi: initialize the resource used by NVDIMM ACPI 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 03/02/2016 07:58 PM, Michael S. Tsirkin wrote: > On Wed, Mar 02, 2016 at 07:50:37PM +0800, Xiao Guangrong wrote: >> 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM >> ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into >> NVDIMM ACPI binary code >> >> OSPM uses this port to tell QEMU the final address of the DSM memory >> and notify QEMU to emulate the DSM method >> >> Signed-off-by: Xiao Guangrong > > > two minor comments: don't respin just for these, can > be addressed later if necessary. Okay. > >> --- >> hw/acpi/Makefile.objs | 2 +- >> hw/acpi/nvdimm.c | 35 +++++++++++++++++++++++++++++++++++ >> hw/i386/acpi-build.c | 10 +--------- >> hw/i386/pc.c | 6 +++--- >> hw/i386/pc_piix.c | 5 +++++ >> hw/i386/pc_q35.c | 8 +++++++- >> include/hw/i386/pc.h | 4 +++- >> include/hw/mem/nvdimm.h | 28 +++++++++++++++++++++++++++- >> 8 files changed, 82 insertions(+), 16 deletions(-) > > How about a spec document to document the interface? Sure, good to me. Actually, there was a spec file in the old versions (e.g, https://github.com/xiaogr/qemu/commit/3e30799182ec53fd56af1e753a24ccf9f6a8f047), I will add it in the last part which will implement the real DSM functions. >> +/* >> + * AcpiNVDIMMState: >> + * @is_enabled: detect if NVDIMM support is enabled. >> + * >> + * @dsm_mem: the data of the fw_cfg file NVDIMM_DSM_MEM_FILE. >> + * @io_mr: the IO region used by OSPM to transfer control to QEMU. >> + */ > > this is not the way we document structures normally. > comments belong near fields. > Okay. keep that in my mind. ;)