From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zo4x5-0006v7-GH for qemu-devel@nongnu.org; Mon, 19 Oct 2015 03:33:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zo4x1-00041f-Ab for qemu-devel@nongnu.org; Mon, 19 Oct 2015 03:33:51 -0400 Received: from mga01.intel.com ([192.55.52.88]:41526) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zo4x0-00041b-W0 for qemu-devel@nongnu.org; Mon, 19 Oct 2015 03:33:47 -0400 References: <1444535584-18220-1-git-send-email-guangrong.xiao@linux.intel.com> <1444535584-18220-23-git-send-email-guangrong.xiao@linux.intel.com> <20151019095612-mutt-send-email-mst@redhat.com> From: Xiao Guangrong Message-ID: <56249B59.1040103@linux.intel.com> Date: Mon, 19 Oct 2015 15:27:21 +0800 MIME-Version: 1.0 In-Reply-To: <20151019095612-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region 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 10/19/2015 02:56 PM, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2015 at 11:52:54AM +0800, Xiao Guangrong wrote: >> We reserve the memory region 0xFF00000 ~ 0xFFF00000 for NVDIMM ACPI >> which is used as: >> - the first page is mapped as MMIO, ACPI write data to this page to >> transfer the control to QEMU >> >> - the second page is RAM-based which used to save the input info of >> _DSM method and QEMU reuse it store output info >> >> - the left is mapped as RAM, it's the buffer returned by _FIT method, >> this is needed by NVDIMM hotplug >> > > Isn't there some way to document this in code, e.g. with > macros? > Yes. It's also documented when DSM memory is created, see nvdimm_build_dsm_memory() introduced in [PATCH v4 25/33] nvdimm acpi: init the address region used by DSM > Adding text under docs/specs would also be a good idea. > Yes. A doc has been added in v4. > >> Signed-off-by: Xiao Guangrong >> --- >> hw/i386/pc.c | 3 ++ >> hw/mem/Makefile.objs | 2 +- >> hw/mem/nvdimm/acpi.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/i386/pc.h | 2 + >> include/hw/mem/nvdimm.h | 19 ++++++++ >> 5 files changed, 145 insertions(+), 1 deletion(-) >> create mode 100644 hw/mem/nvdimm/acpi.c >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 6694b18..8fea4c3 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1360,6 +1360,9 @@ FWCfgState *pc_memory_init(PCMachineState *pcms, >> exit(EXIT_FAILURE); >> } >> >> + nvdimm_init_memory_state(&pcms->nvdimm_memory, system_memory, machine, >> + TARGET_PAGE_SIZE); >> + > > Shouldn't this be conditional on presence of the nvdimm device? > We will enable hotplug on nvdimm devices in the near future once Linux driver is ready. I'd keep it here for future development. > >> pcms->hotplug_memory.base = >> ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, 1ULL << 30); >> >> diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs >> index e0ff328..7310bac 100644 >> --- a/hw/mem/Makefile.objs >> +++ b/hw/mem/Makefile.objs >> @@ -1,3 +1,3 @@ >> common-obj-$(CONFIG_DIMM) += dimm.o >> common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o >> -common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o >> +common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o nvdimm/acpi.o >> diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c >> new file mode 100644 >> index 0000000..b640874 >> --- /dev/null >> +++ b/hw/mem/nvdimm/acpi.c >> @@ -0,0 +1,120 @@ >> +/* >> + * NVDIMM ACPI Implementation >> + * >> + * Copyright(C) 2015 Intel Corporation. >> + * >> + * Author: >> + * Xiao Guangrong >> + * >> + * NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table (NFIT) >> + * and the DSM specfication can be found at: >> + * http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf >> + * >> + * Currently, it only supports PMEM Virtualization. >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2 of the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, see >> + */ >> + >> +#include "qemu-common.h" >> +#include "hw/acpi/acpi.h" >> +#include "hw/acpi/aml-build.h" >> +#include "hw/mem/nvdimm.h" >> +#include "internal.h" >> + >> +/* System Physical Address Range Structure */ >> +struct nfit_spa { >> + uint16_t type; >> + uint16_t length; >> + uint16_t spa_index; >> + uint16_t flags; >> + uint32_t reserved; >> + uint32_t proximity_domain; >> + uint8_t type_guid[16]; >> + uint64_t spa_base; >> + uint64_t spa_length; >> + uint64_t mem_attr; >> +} QEMU_PACKED; >> +typedef struct nfit_spa nfit_spa; >> + >> +/* Memory Device to System Physical Address Range Mapping Structure */ >> +struct nfit_memdev { >> + uint16_t type; >> + uint16_t length; >> + uint32_t nfit_handle; >> + uint16_t phys_id; >> + uint16_t region_id; >> + uint16_t spa_index; >> + uint16_t dcr_index; >> + uint64_t region_len; >> + uint64_t region_offset; >> + uint64_t region_dpa; >> + uint16_t interleave_index; >> + uint16_t interleave_ways; >> + uint16_t flags; >> + uint16_t reserved; >> +} QEMU_PACKED; >> +typedef struct nfit_memdev nfit_memdev; >> + >> +/* NVDIMM Control Region Structure */ >> +struct nfit_dcr { >> + uint16_t type; >> + uint16_t length; >> + uint16_t dcr_index; >> + uint16_t vendor_id; >> + uint16_t device_id; >> + uint16_t revision_id; >> + uint16_t sub_vendor_id; >> + uint16_t sub_device_id; >> + uint16_t sub_revision_id; >> + uint8_t reserved[6]; >> + uint32_t serial_number; >> + uint16_t fic; >> + uint16_t num_bcw; >> + uint64_t bcw_size; >> + uint64_t cmd_offset; >> + uint64_t cmd_size; >> + uint64_t status_offset; >> + uint64_t status_size; >> + uint16_t flags; >> + uint8_t reserved2[6]; >> +} QEMU_PACKED; >> +typedef struct nfit_dcr nfit_dcr; > > Struct naming violates the QEMU coding style. > Pls fix it. I got it. Will add nvdimm_ prefix.