Re: [PATCH v3 5/8] nvdimm acpi: introduce patched dsm memory

["Michael S. Tsirkin" <mst@redhat.com>]

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 <guangrong.xiao@linux.intel.com>


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 <mst@redhat.com>

---

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);
+
+    build_append_byte(array, 0x0C); /* DWordPrefix */
+
+    offset = array->len;
+    build_append_int_noprefix(array, 0x00000000, 4);
+    assert(array->len == offset + 4);
+
+    return offset;
+}
+
 static GPtrArray *alloc_list;
 
 static Aml *aml_alloc(void)