| Message ID | 56D6656A.9060501@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 02, 2016 at 12:00:42PM +0800, Xiao Guangrong wrote: > > > On 03/02/2016 01:12 AM, Michael S. Tsirkin wrote: > >On Tue, Mar 01, 2016 at 06:56:10PM +0800, Xiao Guangrong wrote: > >>Emulate dsm method after IO VM-exit > >> > >>Currently, we only introduce the framework and no function is actually > >>supported > >> > >>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > >>--- > >> hw/acpi/aml-build.c | 2 +- > >> hw/acpi/nvdimm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > >> include/hw/acpi/aml-build.h | 1 + > >> include/hw/mem/nvdimm.h | 8 ++++++++ > >> 4 files changed, 54 insertions(+), 1 deletion(-) > >> > >>diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > >>index ab89ca6..da11bf8 100644 > >>--- a/hw/acpi/aml-build.c > >>+++ b/hw/acpi/aml-build.c > >>@@ -227,7 +227,7 @@ static void build_extop_package(GArray *package, uint8_t op) > >> build_prepend_byte(package, 0x5B); /* ExtOpPrefix */ > >> } > >> > >>-static void build_append_int_noprefix(GArray *table, uint64_t value, int size) > >>+void build_append_int_noprefix(GArray *table, uint64_t value, int size) > >> { > >> int i; > >> > >>diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > >>index 781f6c1..e0b483a 100644 > >>--- a/hw/acpi/nvdimm.c > >>+++ b/hw/acpi/nvdimm.c > >>@@ -393,12 +393,56 @@ typedef struct NvdimmDsmOut NvdimmDsmOut; > >> static uint64_t > >> nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size) > >> { > >>+ fprintf(stderr, "BUG: we never read _DSM IO Port.\n"); > >> return 0; > >> } > >> > >> 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)); > >>+ cpu_physical_memory_write(dsm_mem_addr + sizeof(buf_size), out->data, > >>+ out->len); > > > >BTW, how do we know buffer is big enough? Add assert here? > > I planed to do it when we introduce the real handler of NVDIMM command, but yes, > it is better doing it in this patchset. Will follow it in the next version. > > > > >Also, you have a packed structure with the layout, correct? > >Can't you use that instead of open-coding it? > > > Okay, how about do it like this: > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index a6359cc..2812f7a 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -401,7 +401,7 @@ static void > nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) > { > NvdimmDsmIn *in; > - GArray *out; > + NvdimmDsmOut *out; > uint32_t buf_size; > hwaddr dsm_mem_addr = val; > > @@ -422,27 +422,33 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) > nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision, > in->handle, in->function); > > - out = g_array_new(false, true /* clear */, 1); > + out = g_malloc(TARGET_PAGE_SIZE); > + > + out->len = sizeof(out); > > /* > * 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)); > + /* No function Supported */ > + uint32_t cmd_list = cpu_to_le32(0); > + > + out->len += sizeof(cmd_list); > } else { > - /* No function is supported yet. */ > - build_append_int_noprefix(out, 1 /* Not Supported */, > - sizeof(uint8_t)); > + /* Not Supported */ > + uint32_t status = cpu_to_le32(1); > + > + out->len = sizeof(status); > } > > - buf_size = cpu_to_le32(out->len); > - cpu_physical_memory_write(dsm_mem_addr, &buf_size, sizeof(buf_size)); > - cpu_physical_memory_write(dsm_mem_addr + sizeof(buf_size), out->data, > - out->len); > + buf_size = out->len; > + assert(buf_size <= TARGET_PAGE_SIZE); > + > + out->len = cpu_to_le32(out->len); > + cpu_physical_memory_write(dsm_mem_addr, out, buf_size); > g_free(in); > - g_array_free(out, true); > + g_free(out); > } > > static const MemoryRegionOps nvdimm_dsm_ops = { Not really much better. I suggested a better version in another thread. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index a6359cc..2812f7a 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -401,7 +401,7 @@ static void nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { NvdimmDsmIn *in; - GArray *out; + NvdimmDsmOut *out; uint32_t buf_size; hwaddr dsm_mem_addr = val; @@ -422,27 +422,33 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision, in->handle, in->function); - out = g_array_new(false, true /* clear */, 1); + out = g_malloc(TARGET_PAGE_SIZE); + + out->len = sizeof(out); /* * 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)); + /* No function Supported */ + uint32_t cmd_list = cpu_to_le32(0); + + out->len += sizeof(cmd_list); } else { - /* No function is supported yet. */ - build_append_int_noprefix(out, 1 /* Not Supported */, - sizeof(uint8_t)); + /* Not Supported */ + uint32_t status = cpu_to_le32(1); + + out->len = sizeof(status); } - buf_size = cpu_to_le32(out->len); - cpu_physical_memory_write(dsm_mem_addr, &buf_size, sizeof(buf_size)); - cpu_physical_memory_write(dsm_mem_addr + sizeof(buf_size), out->data, - out->len); + buf_size = out->len; + assert(buf_size <= TARGET_PAGE_SIZE); + + out->len = cpu_to_le32(out->len); + cpu_physical_memory_write(dsm_mem_addr, out, buf_size); g_free(in); - g_array_free(out, true); + g_free(out); } static const MemoryRegionOps nvdimm_dsm_ops = {