From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zs9Q4-0002pU-FH for qemu-devel@nongnu.org; Fri, 30 Oct 2015 09:08:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zs9Q0-0002uf-EN for qemu-devel@nongnu.org; Fri, 30 Oct 2015 09:08:36 -0400 Received: from mga03.intel.com ([134.134.136.65]:30287) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zs9Q0-0002uD-8L for qemu-devel@nongnu.org; Fri, 30 Oct 2015 09:08:32 -0400 References: <1446184587-142784-1-git-send-email-guangrong.xiao@linux.intel.com> <1446184587-142784-28-git-send-email-guangrong.xiao@linux.intel.com> <20151030101436.GH4227@stefanha-x1.localdomain> From: Xiao Guangrong Message-ID: <56336A18.5040107@linux.intel.com> Date: Fri, 30 Oct 2015 21:01:12 +0800 MIME-Version: 1.0 In-Reply-To: <20151030101436.GH4227@stefanha-x1.localdomain> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 27/33] nvdimm acpi: support function 0 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, mtosatti@redhat.com, qemu-devel@nongnu.org, imammedo@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 10/30/2015 06:14 PM, Stefan Hajnoczi wrote: > On Fri, Oct 30, 2015 at 01:56:21PM +0800, Xiao Guangrong wrote: >> static uint64_t >> nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size) >> { >> - return 0; >> + AcpiNVDIMMState *state = opaque; >> + MemoryRegion *dsm_ram_mr = &state->ram_mr; >> + NvdimmDsmIn *in; >> + GArray *out; >> + void *dsm_ram_addr; >> + uint32_t buf_size; >> + >> + assert(memory_region_size(dsm_ram_mr) >= sizeof(NvdimmDsmIn)); >> + dsm_ram_addr = memory_region_get_ram_ptr(dsm_ram_mr); >> + >> + /* >> + * The DSM memory is mapped to guest address space so an evil guest >> + * can change its content while we are doing DSM emulation. Avoid >> + * this by copying DSM memory to QEMU local memory. >> + */ >> + in = g_malloc(memory_region_size(dsm_ram_mr)); >> + memcpy(in, dsm_ram_addr, memory_region_size(dsm_ram_mr)); >> + >> + le32_to_cpus(&in->revision); >> + le32_to_cpus(&in->function); >> + le32_to_cpus(&in->handle); >> + >> + nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision, >> + in->handle, in->function); >> + >> + out = g_array_new(false, true /* clear */, 1); >> + >> + if (in->revision != 0x1 /* Current we support DSM Spec Rev1. */) { >> + nvdimm_debug("Revision %#x is not supported, expect %#x.\n", >> + in->revision, 0x1); >> + nvdimm_dsm_write_status(out, NVDIMM_DSM_STATUS_NOT_SUPPORTED); >> + goto exit; >> + } >> + >> + /* Handle 0 is reserved for NVDIMM Root Device. */ >> + if (!in->handle) { >> + nvdimm_dsm_root(in, out); >> + goto exit; >> + } >> + >> + nvdimm_dsm_device(in, out); >> + >> +exit: >> + /* Write output result to dsm memory. */ >> + memcpy(dsm_ram_addr, out->data, out->len); >> + memory_region_set_dirty(dsm_ram_mr, 0, out->len); > > If you respin this series, please add this before the memcpy out: > > assert(out->len <= memory_region_size(dsm_ram_mr)) > > That way we can catch situations where too much output data was > generated by mistake. > Okay. If this patchset is okay to be merged, i will add this in future development. Thanks for your continuous and active review, Stefan!