qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 5/8] nvdimm acpi: introduce patched dsm memor


From: Xiao Guangrong
Subject: Re: [Qemu-devel] [PATCH v3 5/8] nvdimm acpi: introduce patched dsm memory
Date: Tue, 1 Mar 2016 16:42:43 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1



On 02/29/2016 05:38 PM, Michael S. Tsirkin wrote:
On Sun, Feb 14, 2016 at 04:51:02PM +0800, Xiao Guangrong wrote:
The dsm memory is used to save the input parameters and store
the dsm result which is filled by QEMU.

The address of dsm memory is decided by bios and patched into
int32 object named "MEMA"

Signed-off-by: Xiao Guangrong <address@hidden>


This is a bit too hacky for my taste. First, I would prefer an explicit API
to add a DWORD. Second, I would like to avoid offset math hacks
and make API returning offsets.

Pls see below for a suggestion.


---
  hw/acpi/nvdimm.c | 34 +++++++++++++++++++++++++++++++++-
  1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 8568b20..bca36ae 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -29,6 +29,7 @@
  #include "qemu/osdep.h"
  #include "hw/acpi/acpi.h"
  #include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
  #include "hw/nvram/fw_cfg.h"
  #include "hw/mem/nvdimm.h"

@@ -406,6 +407,7 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, 
MemoryRegion *io,
  }

  #define NVDIMM_COMMON_DSM      "NCAL"
+#define NVDIMM_ACPI_MEM_ADDR   "MEMA"

  static void nvdimm_build_common_dsm(Aml *dev)
  {
@@ -470,7 +472,9 @@ static void nvdimm_build_nvdimm_devices(GSList 
*device_list, Aml *root_dev)
  static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
                                GArray *table_data, GArray *linker)
  {
-    Aml *ssdt, *sb_scope, *dev;
+    Aml *ssdt, *sb_scope, *dev, *mem_addr;
+    uint32_t zero_offset = 0;
+    int offset;

      acpi_add_table(table_offsets, table_data);

@@ -501,9 +505,37 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray 
*table_offsets,

      aml_append(sb_scope, dev);

+    /*
+     * leave it at the end of ssdt so that we can conveniently get the
+     * offset of int32 object which will be patched with the real address
+     * of the dsm memory by BIOS.
+     *
+     * 0x32000000 is the magic number to let aml_int() create int32 object.
+     * It will be zeroed later to make bios_linker_loader_add_pointer()
+     * happy.
+     */
+    mem_addr = aml_name_decl(NVDIMM_ACPI_MEM_ADDR, aml_int(0x32000000));
+
+    aml_append(sb_scope, mem_addr);
      aml_append(ssdt, sb_scope);
      /* copy AML table into ACPI tables blob and patch header there */
      g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
+
+    offset = table_data->len - 4;
+
+    /*
+     * zero the last 4 bytes, i.e, it is the offset of
+     * NVDIMM_ACPI_MEM_ADDR object.
+     */
+    g_array_remove_range(table_data, offset, 4);
+    g_array_append_vals(table_data, &zero_offset, 4);
+
+    bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE,
+                             false /* high memory */);
+    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
+                                   NVDIMM_DSM_MEM_FILE, table_data,
+                                   table_data->data + offset,
+                                   sizeof(uint32_t));
      build_header(linker, table_data,
          (void *)(table_data->data + table_data->len - ssdt->buf->len),
          "SSDT", ssdt->buf->len, 1, NULL, "NVDIMM");
--
1.8.3.1



--->

acpi: add build_append_named_dword, returning an offset in buffer

This is a very limited form of support for runtime patching -
similar in functionality to what we can do with ACPI_EXTRACT
macros in python, but implemented in C.

This is to allow ACPI code direct access to data tables -
which is exactly what DataTableRegion is there for, except
no known windows release so far implements DataTableRegion.

Signed-off-by: Michael S. Tsirkin <address@hidden>

---

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 1b632dc..f8998ea 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -286,4 +286,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre);
  void
  build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets);

+int
+build_append_named_dword(GArray *array, const char *name_format, ...);
+
  #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 0d4b324..7f9fa65 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -262,6 +262,32 @@ static void build_append_int(GArray *table, uint64_t value)
      }
  }

+/* Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
+ * and return the offset to 0x00000000 for runtime patching.
+ *
+ * Warning: runtime patching is best avoided. Only use this as
+ * a replacement for DataTableRegion (for guests that don't
+ * support it).
+ */
+int
+build_append_named_dword(GArray *array, const char *name_format, ...)
+{
+    int offset;
+    va_list ap;
+
+    va_start(ap, name_format);
+    build_append_namestringv(array, name_format, ap);
+    va_end(ap);
+

The NameOP was missed here...

The idea is great and i fixed and applied it on the top this patchset, the patch
is attached, would it be good to you?

Attachment: 0001-acpi-add-build_append_named_dword-returning-an-offse.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]