From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38115) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaxUx-0006CE-P7 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 22:30:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaxUt-0004zc-OQ for qemu-devel@nongnu.org; Tue, 01 Mar 2016 22:30:51 -0500 Received: from mga09.intel.com ([134.134.136.24]:42339) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaxUt-0004zB-IP for qemu-devel@nongnu.org; Tue, 01 Mar 2016 22:30:47 -0500 References: <1456829771-71553-1-git-send-email-guangrong.xiao@linux.intel.com> <1456829771-71553-9-git-send-email-guangrong.xiao@linux.intel.com> <20160301190240-mutt-send-email-mst@redhat.com> From: Xiao Guangrong Message-ID: <56D65E42.1050108@linux.intel.com> Date: Wed, 2 Mar 2016 11:30:10 +0800 MIME-Version: 1.0 In-Reply-To: <20160301190240-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 8/9] nvdimm acpi: emulate dsm method 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 01:09 AM, Michael S. Tsirkin wrote: > > Can't guest trigger this? > If yes, don't put such code in production please: > this will fill up disk on the host. > Okay, the evil guest can read the IO port freely. I will use nvdimm_debug() instead. > >> >> static void >> nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) >> { >> + NvdimmDsmIn *in; >> + GArray *out; >> + uint32_t buf_size; >> + hwaddr dsm_mem_addr = val; >> + >> + nvdimm_debug("dsm memory address %#lx.\n", dsm_mem_addr); >> + >> + /* >> + * 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(TARGET_PAGE_SIZE); >> + cpu_physical_memory_read(dsm_mem_addr, in, TARGET_PAGE_SIZE); >> + >> + 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); >> + >> + /* >> + * function 0 is called to inquire what functions are supported by >> + * OSPM >> + */ >> + if (in->function == 0) { >> + build_append_int_noprefix(out, 0 /* No function Supported */, >> + sizeof(uint8_t)); >> + } else { >> + /* No function is supported yet. */ >> + build_append_int_noprefix(out, 1 /* Not Supported */, >> + sizeof(uint8_t)); >> + } >> + >> + buf_size = cpu_to_le32(out->len); >> + cpu_physical_memory_write(dsm_mem_addr, &buf_size, sizeof(buf_size)); > > is there a race here? > can guest read this before data is written? I think no. It is the SERIALIZED DSM so there is no race in guest. And the CPU has exited from guest mode when we fill the buffer in the same CPU-context so the guest can not read the buffer at this point also memory-barrier is not needed here. > >> + cpu_physical_memory_write(dsm_mem_addr + sizeof(buf_size), out->data, >> + out->len); > > What is this doing? > Is this actually writing AML bytecode into guest memory? The layout of result written into the buffer is like this: struct NvdimmDsmOut { /* the size of buffer filled by QEMU. */ uint32_t len; uint8_t data[0]; } QEMU_PACKED; typedef struct NvdimmDsmOut NvdimmDsmOut; So the first cpu_physical_memory_write() writes the @len and the second one you pointed out writes the real payload.