All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT)
@ 2019-01-11 15:34 Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 1/9] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT Tao Xu
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

This series of patches will build Heterogeneous Memory Attribute Table (HMAT)
according to the command line. The ACPI HMAT describes the memory attributes,
such as memory side cache attributes and bandwidth and latency details,
related to the System Physical Address (SPA) Memory Ranges.
The software is expected to use this information as hint for optimization.

OSPM evaluates HMAT only during system initialization. Any changes to the HMAT
state at runtime or information regarding HMAT for hot plug are communicated
using the _HMA method.

>From 1 to 6 patches are the former V1 patches. From 7 to 9 patches are the
updates per Igor and Eric's comments.

The V1 RESEND patches link:
http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg05368.html
The V1 patches link:
http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg01927.html

Changelog:
v2:
  Per Igor and Eric's comments, fix some coding style and small issues:
    - update the version number in qapi/misc.json
    - including the expansion of the acronym HMAT in qapi/misc.json
    - correct spell mistakes in qapi/misc.json and qemu-options.hx
    - fix the comment syle in hw/i386/acpi-build.c
    and hw/acpi/hmat.h
   - remove some unnecessary head files in hw/acpi/hmat.c
   - use hardcoded numbers from spec to generate
   Memory Subsystem Address Range Structure in hw/acpi/hmat.c
   - drop the struct AcpiHmat and AcpiHmatSpaRange
    in hw/acpi/hmat.h
  Per Igor's comment, rewrite NFIT code to build _HMA method.

Liu Jingqi (6):
  hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI
    HMAT
  hmat acpi: Build System Locality Latency and Bandwidth Information
    Structure(s) in ACPI HMAT
  hmat acpi: Build Memory Side Cache Information Structure(s) in ACPI
    HMAT
  Extend the command-line to provide memory latency and bandwidth
    information
  numa: Extend the command-line to provide memory side cache information
  hmat acpi: Implement _HMA method to update HMAT at runtime

Tao Xu (3):
  hmat acpi: fix some coding style and small issues
  hmat acpi: move some function inside of the caller
  acpi: rewrite the _FIT method to use it in _HMA method

 default-configs/i386-softmmu.mak |   1 +
 hw/acpi/Makefile.objs            |   1 +
 hw/acpi/hmat.c                   | 424 +++++++++++++++++++++++++++++++
 hw/acpi/hmat.h                   | 244 ++++++++++++++++++
 hw/acpi/nvdimm.c                 | 389 ++++++++++++++++++----------
 hw/i386/acpi-build.c             | 125 +++++----
 hw/i386/acpi-build.h             |  10 +
 hw/i386/pc.c                     |   2 +
 hw/i386/pc_piix.c                |   3 +
 hw/i386/pc_q35.c                 |   3 +
 include/hw/i386/pc.h             |   2 +
 include/hw/mem/nvdimm.h          |  11 +
 include/sysemu/numa.h            |   2 +
 numa.c                           | 202 +++++++++++++++
 qapi/misc.json                   | 162 +++++++++++-
 qemu-options.hx                  |  28 +-
 16 files changed, 1426 insertions(+), 183 deletions(-)
 create mode 100644 hw/acpi/hmat.c
 create mode 100644 hw/acpi/hmat.h

-- 
2.17.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 1/9] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 2/9] hmat acpi: Build System Locality Latency and Bandwidth Information " Tao Xu
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

HMAT is defined in ACPI 6.2: 5.2.27 Heterogeneous Memory Attribute Table (HMAT).
The specification references below link:
http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf

It describes the memory attributes, such as memory side cache
attributes and bandwidth and latency details, related to the
System Physical Address (SPA) Memory Ranges. The software is
expected to use this information as hint for optimization.

This structure describes the System Physical Address(SPA) range
occupied by memory subsystem and its associativity with processor
proximity domain as well as hint for memory usage.

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 default-configs/i386-softmmu.mak |   1 +
 hw/acpi/Makefile.objs            |   1 +
 hw/acpi/hmat.c                   | 139 +++++++++++++++++++++++++++++++
 hw/acpi/hmat.h                   |  73 ++++++++++++++++
 hw/i386/acpi-build.c             | 121 +++++++++++++++++----------
 hw/i386/acpi-build.h             |  10 +++
 include/sysemu/numa.h            |   2 +
 numa.c                           |   6 ++
 8 files changed, 308 insertions(+), 45 deletions(-)
 create mode 100644 hw/acpi/hmat.c
 create mode 100644 hw/acpi/hmat.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 64c998c4c8..3b77640f9d 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -67,3 +67,4 @@ CONFIG_I2C=y
 CONFIG_SEV=$(CONFIG_KVM)
 CONFIG_VTD=y
 CONFIG_AMD_IOMMU=y
+CONFIG_ACPI_HMAT=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 11c35bcb44..21889fd80a 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
+common-obj-$(CONFIG_ACPI_HMAT) += hmat.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
new file mode 100644
index 0000000000..d4e586d4f5
--- /dev/null
+++ b/hw/acpi/hmat.c
@@ -0,0 +1,139 @@
+/*
+ * HMAT ACPI Implementation
+ *
+ * Copyright(C) 2018 Intel Corporation.
+ *
+ * Author:
+ *  Liu jingqi <jingqi.liu@linux.intel.com>
+ *
+ * HMAT is defined in ACPI 6.2.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "unistd.h"
+#include "fcntl.h"
+#include "qemu/osdep.h"
+#include "sysemu/numa.h"
+#include "hw/i386/pc.h"
+#include "hw/i386/acpi-build.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/hmat.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/nvram/fw_cfg.h"
+#include "hw/acpi/bios-linker-loader.h"
+
+/* Build Memory Subsystem Address Range Structure */
+static void hmat_build_spa_info(GArray *table_data,
+                                uint64_t base, uint64_t length, int node)
+{
+    uint16_t flags = 0;
+
+    if (numa_info[node].is_initiator) {
+        flags |= HMAT_SPA_PROC_VALID;
+    }
+    if (numa_info[node].is_target) {
+        flags |= HMAT_SPA_MEM_VALID;
+    }
+
+    /* Type */
+    build_append_int_noprefix(table_data, ACPI_HMAT_SPA, sizeof(uint16_t));
+    /* Reserved0 */
+    build_append_int_noprefix(table_data, 0, sizeof(uint16_t));
+    /* Length */
+    build_append_int_noprefix(table_data, sizeof(AcpiHmatSpaRange),
+                              sizeof(uint32_t));
+    /* Flags */
+    build_append_int_noprefix(table_data, flags, sizeof(uint16_t));
+    /* Reserved1 */
+    build_append_int_noprefix(table_data, 0, sizeof(uint16_t));
+    /* Process Proximity Domain */
+    build_append_int_noprefix(table_data, node, sizeof(uint32_t));
+    /* Memory Proximity Domain */
+    build_append_int_noprefix(table_data, node, sizeof(uint32_t));
+    /* Reserved2 */
+    build_append_int_noprefix(table_data, 0, sizeof(uint32_t));
+    /* System Physical Address Range Base */
+    build_append_int_noprefix(table_data, base, sizeof(uint64_t));
+    /* System Physical Address Range Length */
+    build_append_int_noprefix(table_data, length, sizeof(uint64_t));
+}
+
+static int pc_dimm_device_list(Object *obj, void *opaque)
+{
+    GSList **list = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        *list = g_slist_append(*list, DEVICE(obj));
+    }
+
+    object_child_foreach(obj, pc_dimm_device_list, opaque);
+    return 0;
+}
+
+/*
+ * The Proximity Domain of System Physical Address ranges defined
+ * in the HMAT, NFIT and SRAT tables shall match each other.
+ */
+static void hmat_build_spa(GArray *table_data, PCMachineState *pcms)
+{
+    GSList *device_list = NULL;
+    uint64_t mem_base, mem_len;
+    int i;
+
+    if (pcms->numa_nodes && !mem_ranges_number) {
+        build_mem_ranges(pcms);
+    }
+
+    for (i = 0; i < mem_ranges_number; i++) {
+        hmat_build_spa_info(table_data, mem_ranges[i].base,
+                            mem_ranges[i].length, mem_ranges[i].node);
+    }
+
+    /* Build HMAT SPA structures for PC-DIMM devices. */
+    object_child_foreach(qdev_get_machine(), pc_dimm_device_list, &device_list);
+
+    for (; device_list; device_list = device_list->next) {
+        PCDIMMDevice *dimm = device_list->data;
+        mem_base = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
+                                            NULL);
+        mem_len = object_property_get_uint(OBJECT(dimm), PC_DIMM_SIZE_PROP,
+                                           NULL);
+        i = object_property_get_uint(OBJECT(dimm), PC_DIMM_NODE_PROP, NULL);
+        hmat_build_spa_info(table_data, mem_base, mem_len, i);
+    }
+}
+
+static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
+{
+    /* Build HMAT Memory Subsystem Address Range. */
+    hmat_build_spa(hma, pcms);
+}
+
+void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
+                     MachineState *machine)
+{
+    PCMachineState *pcms = PC_MACHINE(machine);
+    uint64_t hmat_start, hmat_len;
+
+    hmat_start = table_data->len;
+    acpi_data_push(table_data, sizeof(AcpiHmat));
+
+    hmat_build_hma(table_data, pcms);
+    hmat_len = table_data->len - hmat_start;
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + hmat_start),
+                 "HMAT", hmat_len, 1, NULL, NULL);
+}
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
new file mode 100644
index 0000000000..096415df8a
--- /dev/null
+++ b/hw/acpi/hmat.h
@@ -0,0 +1,73 @@
+/*
+ * HMAT ACPI Implementation Header
+ *
+ * Copyright(C) 2018 Intel Corporation.
+ *
+ * Author:
+ *  Liu jingqi <jingqi.liu@linux.intel.com>
+ *
+ * HMAT is defined in ACPI 6.2.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef HMAT_H
+#define HMAT_H
+
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/aml-build.h"
+
+#define ACPI_HMAT_SPA               0
+
+/* ACPI HMAT sub-structure header */
+#define ACPI_HMAT_SUB_HEADER_DEF    \
+    uint16_t  type;                 \
+    uint16_t  reserved0;            \
+    uint32_t  length;
+
+/* the values of AcpiHmatSpaRange flag */
+enum {
+    HMAT_SPA_PROC_VALID = 0x1,
+    HMAT_SPA_MEM_VALID  = 0x2,
+    HMAT_SPA_RESERVATION_HINT = 0x4,
+};
+
+/*
+ * HMAT (Heterogeneous Memory Attributes Table)
+ */
+struct AcpiHmat {
+    ACPI_TABLE_HEADER_DEF
+    uint32_t    reserved;
+} QEMU_PACKED;
+typedef struct AcpiHmat AcpiHmat;
+
+struct AcpiHmatSpaRange {
+    ACPI_HMAT_SUB_HEADER_DEF
+    uint16_t    flags;
+    uint16_t    reserved1;
+    uint32_t    proc_proximity;
+    uint32_t    mem_proximity;
+    uint32_t    reserved2;
+    uint64_t    spa_base;
+    uint64_t    spa_length;
+} QEMU_PACKED;
+typedef struct AcpiHmatSpaRange AcpiHmatSpaRange;
+
+void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
+                     MachineState *machine);
+
+#endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 14f757fc36..a93d437175 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -64,6 +64,7 @@
 #include "hw/i386/intel_iommu.h"
 
 #include "hw/acpi/ipmi.h"
+#include "hw/acpi/hmat.h"
 
 /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
  * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
@@ -119,6 +120,14 @@ typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+/* The memory contains at least one hole
+ * from 640k-1M and possibly another one from 3.5G-4G.
+ * So far, the number of memory ranges is up to 2
+ * more than the number of numa nodes.
+ */
+MemoryRange mem_ranges[MAX_NODES + 2];
+uint32_t mem_ranges_number;
+
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
     uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -2251,6 +2260,63 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 #define HOLE_640K_START  (640 * KiB)
 #define HOLE_640K_END   (1 * MiB)
 
+void build_mem_ranges(PCMachineState *pcms)
+{
+    uint64_t mem_len, mem_base, next_base;
+    int i;
+
+    /* the memory map is a bit tricky, it contains at least one hole
+     * from 640k-1M and possibly another one from 3.5G-4G.
+     */
+    mem_ranges_number = 0;
+    next_base = 0;
+
+    for (i = 0; i < pcms->numa_nodes; ++i) {
+        mem_base = next_base;
+        mem_len = pcms->node_mem[i];
+        next_base = mem_base + mem_len;
+
+        /* Cut out the 640K hole */
+        if (mem_base <= HOLE_640K_START &&
+            next_base > HOLE_640K_START) {
+            mem_len -= next_base - HOLE_640K_START;
+            if (mem_len > 0) {
+                mem_ranges[mem_ranges_number].base = mem_base;
+                mem_ranges[mem_ranges_number].length = mem_len;
+                mem_ranges[mem_ranges_number].node = i;
+                mem_ranges_number++;
+            }
+
+            /* Check for the rare case: 640K < RAM < 1M */
+            if (next_base <= HOLE_640K_END) {
+                next_base = HOLE_640K_END;
+                continue;
+            }
+            mem_base = HOLE_640K_END;
+            mem_len = next_base - HOLE_640K_END;
+        }
+
+        /* Cut out the ACPI_PCI hole */
+        if (mem_base <= pcms->below_4g_mem_size &&
+            next_base > pcms->below_4g_mem_size) {
+            mem_len -= next_base - pcms->below_4g_mem_size;
+            if (mem_len > 0) {
+                mem_ranges[mem_ranges_number].base = mem_base;
+                mem_ranges[mem_ranges_number].length = mem_len;
+                mem_ranges[mem_ranges_number].node = i;
+                mem_ranges_number++;
+            }
+            mem_base = 1ULL << 32;
+            mem_len = next_base - pcms->below_4g_mem_size;
+            next_base = mem_base + mem_len;
+        }
+        mem_ranges[mem_ranges_number].base = mem_base;
+        mem_ranges[mem_ranges_number].length = mem_len;
+        mem_ranges[mem_ranges_number].node = i;
+        mem_ranges_number++;
+    }
+}
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
@@ -2259,7 +2325,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 
     int i;
     int srat_start, numa_start, slots;
-    uint64_t mem_len, mem_base, next_base;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
     PCMachineState *pcms = PC_MACHINE(machine);
@@ -2299,54 +2364,18 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
         }
     }
 
+    if (pcms->numa_nodes && !mem_ranges_number) {
+        build_mem_ranges(pcms);
+    }
 
-    /* the memory map is a bit tricky, it contains at least one hole
-     * from 640k-1M and possibly another one from 3.5G-4G.
-     */
-    next_base = 0;
     numa_start = table_data->len;
 
-    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
-        mem_base = next_base;
-        mem_len = pcms->node_mem[i - 1];
-        next_base = mem_base + mem_len;
-
-        /* Cut out the 640K hole */
-        if (mem_base <= HOLE_640K_START &&
-            next_base > HOLE_640K_START) {
-            mem_len -= next_base - HOLE_640K_START;
-            if (mem_len > 0) {
-                numamem = acpi_data_push(table_data, sizeof *numamem);
-                build_srat_memory(numamem, mem_base, mem_len, i - 1,
-                                  MEM_AFFINITY_ENABLED);
-            }
-
-            /* Check for the rare case: 640K < RAM < 1M */
-            if (next_base <= HOLE_640K_END) {
-                next_base = HOLE_640K_END;
-                continue;
-            }
-            mem_base = HOLE_640K_END;
-            mem_len = next_base - HOLE_640K_END;
-        }
-
-        /* Cut out the ACPI_PCI hole */
-        if (mem_base <= pcms->below_4g_mem_size &&
-            next_base > pcms->below_4g_mem_size) {
-            mem_len -= next_base - pcms->below_4g_mem_size;
-            if (mem_len > 0) {
-                numamem = acpi_data_push(table_data, sizeof *numamem);
-                build_srat_memory(numamem, mem_base, mem_len, i - 1,
-                                  MEM_AFFINITY_ENABLED);
-            }
-            mem_base = 1ULL << 32;
-            mem_len = next_base - pcms->below_4g_mem_size;
-            next_base = mem_base + mem_len;
-        }
-
-        if (mem_len > 0) {
+    for (i = 0; i < mem_ranges_number; i++) {
+        if (mem_ranges[i].length > 0) {
             numamem = acpi_data_push(table_data, sizeof *numamem);
-            build_srat_memory(numamem, mem_base, mem_len, i - 1,
+            build_srat_memory(numamem, mem_ranges[i].base,
+                              mem_ranges[i].length,
+                              mem_ranges[i].node,
                               MEM_AFFINITY_ENABLED);
         }
     }
@@ -2669,6 +2698,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
             acpi_add_table(table_offsets, tables_blob);
             build_slit(tables_blob, tables->linker);
         }
+        acpi_add_table(table_offsets, tables_blob);
+        hmat_build_acpi(tables_blob, tables->linker, machine);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 007332e51c..f17de6af6a 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -2,6 +2,16 @@
 #ifndef HW_I386_ACPI_BUILD_H
 #define HW_I386_ACPI_BUILD_H
 
+typedef struct memory_range {
+    uint64_t base;
+    uint64_t length;
+    uint32_t node;
+} MemoryRange;
+
+extern MemoryRange mem_ranges[];
+extern uint32_t mem_ranges_number;
+
+void build_mem_ranges(PCMachineState *pcms);
 void acpi_setup(void);
 
 #endif
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index b6ac7de43e..d41be00b92 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -13,6 +13,8 @@ struct NodeInfo {
     uint64_t node_mem;
     struct HostMemoryBackend *node_memdev;
     bool present;
+    bool is_initiator;
+    bool is_target;
     uint8_t distance[MAX_NODES];
 };
 
diff --git a/numa.c b/numa.c
index 50ec016013..9ee4f6f258 100644
--- a/numa.c
+++ b/numa.c
@@ -105,6 +105,10 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
         }
     }
 
+    if (node->cpus) {
+        numa_info[nodenr].is_initiator = true;
+    }
+
     if (node->has_mem && node->has_memdev) {
         error_setg(errp, "cannot specify both mem= and memdev=");
         return;
@@ -121,6 +125,7 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
 
     if (node->has_mem) {
         numa_info[nodenr].node_mem = node->mem;
+        numa_info[nodenr].is_target = true;
     }
     if (node->has_memdev) {
         Object *o;
@@ -133,6 +138,7 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
         object_ref(o);
         numa_info[nodenr].node_mem = object_property_get_uint(o, "size", NULL);
         numa_info[nodenr].node_memdev = MEMORY_BACKEND(o);
+        numa_info[nodenr].is_target = true;
     }
     numa_info[nodenr].present = true;
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 2/9] hmat acpi: Build System Locality Latency and Bandwidth Information Structure(s) in ACPI HMAT
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 1/9] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 3/9] hmat acpi: Build Memory Side Cache " Tao Xu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

This structure describes the memory access latency and bandwidth
information from various memory access initiator proximity domains.
The latency and bandwidth numbers represented in this structure
correspond to rated latency and bandwidth for the platform.
The software could use this information as hint for optimization.

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/hmat.h | 76 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index d4e586d4f5..214f150fe6 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -34,6 +34,11 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 
+struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES] = {0};
+
+static uint32_t initiator_pxm[MAX_NODES], target_pxm[MAX_NODES];
+static uint32_t num_initiator, num_target;
+
 /* Build Memory Subsystem Address Range Structure */
 static void hmat_build_spa_info(GArray *table_data,
                                 uint64_t base, uint64_t length, int node)
@@ -115,10 +120,103 @@ static void hmat_build_spa(GArray *table_data, PCMachineState *pcms)
     }
 }
 
+static void classify_proximity_domains(void)
+{
+    int node;
+
+    for (node = 0; node < nb_numa_nodes; node++) {
+        if (numa_info[node].is_initiator) {
+            initiator_pxm[num_initiator++] = node;
+        }
+        if (numa_info[node].is_target) {
+            target_pxm[num_target++] = node;
+        }
+    }
+}
+
+static void hmat_build_lb(GArray *table_data)
+{
+    AcpiHmatLBInfo *hmat_lb;
+    struct numa_hmat_lb_info *numa_hmat_lb;
+    int i, j, hrchy, type;
+
+    if (!num_initiator && !num_target) {
+        classify_proximity_domains();
+    }
+
+    for (hrchy = HMAT_LB_MEM_MEMORY;
+         hrchy <= HMAT_LB_MEM_CACHE_3RD_LEVEL; hrchy++) {
+        for (type = HMAT_LB_DATA_ACCESS_LATENCY;
+             type <= HMAT_LB_DATA_WRITE_BANDWIDTH; type++) {
+            numa_hmat_lb = hmat_lb_info[hrchy][type];
+
+            if (numa_hmat_lb) {
+                uint64_t start;
+                uint32_t *list_entry;
+                uint16_t *entry, *entry_start;
+                uint32_t size;
+                uint8_t m, n;
+
+                start = table_data->len;
+                hmat_lb = acpi_data_push(table_data, sizeof(*hmat_lb));
+
+                hmat_lb->type          = cpu_to_le16(ACPI_HMAT_LB_INFO);
+                hmat_lb->flags         = numa_hmat_lb->hierarchy;
+                hmat_lb->data_type     = numa_hmat_lb->data_type;
+                hmat_lb->num_initiator = cpu_to_le32(num_initiator);
+                hmat_lb->num_target    = cpu_to_le32(num_target);
+
+                if (type <= HMAT_LB_DATA_WRITE_LATENCY) {
+                    hmat_lb->base_unit = cpu_to_le32(numa_hmat_lb->base_lat);
+                } else {
+                    hmat_lb->base_unit = cpu_to_le32(numa_hmat_lb->base_bw);
+                }
+                if (!hmat_lb->base_unit) {
+                    hmat_lb->base_unit = cpu_to_le32(1);
+                }
+
+                /* the initiator proximity domain list */
+                for (i = 0; i < num_initiator; i++) {
+                    list_entry = acpi_data_push(table_data, sizeof(uint32_t));
+                    *list_entry = cpu_to_le32(initiator_pxm[i]);
+                }
+
+                /* the target proximity domain list */
+                for (i = 0; i < num_target; i++) {
+                    list_entry = acpi_data_push(table_data, sizeof(uint32_t));
+                    *list_entry = cpu_to_le32(target_pxm[i]);
+                }
+
+                /* latency or bandwidth entries */
+                size = sizeof(uint16_t) * num_initiator * num_target;
+                entry_start = acpi_data_push(table_data, size);
+
+                for (i = 0; i < num_initiator; i++) {
+                    m = initiator_pxm[i];
+                    for (j = 0; j < num_target; j++) {
+                        n = target_pxm[j];
+                        entry = entry_start + i * num_target + j;
+                        if (type <= HMAT_LB_DATA_WRITE_LATENCY) {
+                            *entry = cpu_to_le16(numa_hmat_lb->latency[m][n]);
+                        } else {
+                            *entry = cpu_to_le16(numa_hmat_lb->bandwidth[m][n]);
+                        }
+                    }
+                }
+                hmat_lb = (AcpiHmatLBInfo *)(table_data->data + start);
+                hmat_lb->length = cpu_to_le16(table_data->len - start);
+            }
+        }
+    }
+}
+
 static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
 {
     /* Build HMAT Memory Subsystem Address Range. */
     hmat_build_spa(hma, pcms);
+
+    /* Build HMAT System Locality Latency and Bandwidth Information. */
+    hmat_build_lb(hma);
 }
 
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index 096415df8a..fddd05e0d1 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -32,6 +32,7 @@
 #include "hw/acpi/aml-build.h"
 
 #define ACPI_HMAT_SPA               0
+#define ACPI_HMAT_LB_INFO           1
 
 /* ACPI HMAT sub-structure header */
 #define ACPI_HMAT_SUB_HEADER_DEF    \
@@ -46,6 +47,28 @@ enum {
     HMAT_SPA_RESERVATION_HINT = 0x4,
 };
 
+/* the value of AcpiHmatLBInfo flags */
+enum {
+    HMAT_LB_MEM_MEMORY           = 0,
+    HMAT_LB_MEM_CACHE_LAST_LEVEL = 1,
+    HMAT_LB_MEM_CACHE_1ST_LEVEL  = 2,
+    HMAT_LB_MEM_CACHE_2ND_LEVEL  = 3,
+    HMAT_LB_MEM_CACHE_3RD_LEVEL  = 4,
+};
+
+/* the value of AcpiHmatLBInfo data type */
+enum {
+    HMAT_LB_DATA_ACCESS_LATENCY = 0,
+    HMAT_LB_DATA_READ_LATENCY = 1,
+    HMAT_LB_DATA_WRITE_LATENCY = 2,
+    HMAT_LB_DATA_ACCESS_BANDWIDTH = 3,
+    HMAT_LB_DATA_READ_BANDWIDTH = 4,
+    HMAT_LB_DATA_WRITE_BANDWIDTH = 5,
+};
+
+#define HMAT_LB_LEVELS    (HMAT_LB_MEM_CACHE_3RD_LEVEL + 1)
+#define HMAT_LB_TYPES     (HMAT_LB_DATA_WRITE_BANDWIDTH + 1)
+
 /*
  * HMAT (Heterogeneous Memory Attributes Table)
  */
@@ -67,6 +90,59 @@ struct AcpiHmatSpaRange {
 } QEMU_PACKED;
 typedef struct AcpiHmatSpaRange AcpiHmatSpaRange;
 
+struct AcpiHmatLBInfo {
+    ACPI_HMAT_SUB_HEADER_DEF
+    uint8_t     flags;
+    uint8_t     data_type;
+    uint16_t    reserved1;
+    uint32_t    num_initiator;
+    uint32_t    num_target;
+    uint32_t    reserved2;
+    uint64_t    base_unit;
+} QEMU_PACKED;
+typedef struct AcpiHmatLBInfo AcpiHmatLBInfo;
+
+struct numa_hmat_lb_info {
+    /*
+     * Indicates total number of Proximity Domains
+     * that can initiate memory access requests.
+     */
+    uint32_t    num_initiator;
+    /*
+     * Indicates total number of Proximity Domains
+     * that can act as target.
+     */
+    uint32_t    num_target;
+    /*
+     * Indicates it's memory or
+     * the specified level memory side cache.
+     */
+    uint8_t     hierarchy;
+    /*
+     * Present the type of data,
+     * access/read/write latency or bandwidth.
+     */
+    uint8_t     data_type;
+    /* The base unit for latency in nanoseconds. */
+    uint64_t    base_lat;
+    /* The base unit for bandwidth in megabytes per second(MB/s). */
+    uint64_t    base_bw;
+    /*
+     * latency[i][j]:
+     * Indicates the latency based on base_lat
+     * from Initiator Proximity Domain i to Target Proximity Domain j.
+     */
+    uint16_t    latency[MAX_NODES][MAX_NODES];
+    /*
+     * bandwidth[i][j]:
+     * Indicates the bandwidth based on base_bw
+     * from Initiator Proximity Domain i to Target Proximity Domain j.
+     */
+    uint16_t    bandwidth[MAX_NODES][MAX_NODES];
+};
+
+extern struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES];
+
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                      MachineState *machine);
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 3/9] hmat acpi: Build Memory Side Cache Information Structure(s) in ACPI HMAT
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 1/9] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 2/9] hmat acpi: Build System Locality Latency and Bandwidth Information " Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information Tao Xu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

This structure describes memory side cache information for memory
proximity domains if the memory side cache is present and the
physical device(SMBIOS handle) forms the memory side cache.
The software could use this information to effectively place
the data in memory to maximize the performance of the system
memory that use the memory side cache.

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/hmat.h | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index 214f150fe6..9d29ef7929 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -35,6 +35,8 @@
 #include "hw/acpi/bios-linker-loader.h"
 
 struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES] = {0};
+struct numa_hmat_cache_info
+       *hmat_cache_info[MAX_NODES][MAX_HMAT_CACHE_LEVEL + 1] = {0};
 
 static uint32_t initiator_pxm[MAX_NODES], target_pxm[MAX_NODES];
 static uint32_t num_initiator, num_target;
@@ -210,6 +212,57 @@ static void hmat_build_lb(GArray *table_data)
     }
 }
 
+static void hmat_build_cache(GArray *table_data)
+{
+    AcpiHmatCacheInfo *hmat_cache;
+    struct numa_hmat_cache_info *numa_hmat_cache;
+    int i, level;
+
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (level = 0; level <= MAX_HMAT_CACHE_LEVEL; level++) {
+            numa_hmat_cache = hmat_cache_info[i][level];
+            if (numa_hmat_cache) {
+                uint64_t start = table_data->len;
+
+                hmat_cache = acpi_data_push(table_data, sizeof(*hmat_cache));
+                hmat_cache->length = cpu_to_le32(sizeof(*hmat_cache));
+                hmat_cache->type = cpu_to_le16(ACPI_HMAT_CACHE_INFO);
+                hmat_cache->mem_proximity =
+                            cpu_to_le32(numa_hmat_cache->mem_proximity);
+                hmat_cache->cache_size  = cpu_to_le64(numa_hmat_cache->size);
+                hmat_cache->cache_attr  = HMAT_CACHE_TOTAL_LEVEL(
+                                          numa_hmat_cache->total_levels);
+                hmat_cache->cache_attr |= HMAT_CACHE_CURRENT_LEVEL(
+                                          numa_hmat_cache->level);
+                hmat_cache->cache_attr |= HMAT_CACHE_ASSOC(
+                                          numa_hmat_cache->associativity);
+                hmat_cache->cache_attr |= HMAT_CACHE_WRITE_POLICY(
+                                          numa_hmat_cache->write_policy);
+                hmat_cache->cache_attr |= HMAT_CACHE_LINE_SIZE(
+                                          numa_hmat_cache->line_size);
+                hmat_cache->cache_attr = cpu_to_le32(hmat_cache->cache_attr);
+
+                if (numa_hmat_cache->num_smbios_handles != 0) {
+                    uint16_t *smbios_handles;
+                    int size;
+
+                    size = hmat_cache->num_smbios_handles * sizeof(uint16_t);
+                    smbios_handles = acpi_data_push(table_data, size);
+
+                    hmat_cache = (AcpiHmatCacheInfo *)
+                                 (table_data->data + start);
+                    hmat_cache->length += size;
+
+                    /* TBD: set smbios handles */
+                    memset(smbios_handles, 0, size);
+                }
+                hmat_cache->num_smbios_handles =
+                            cpu_to_le16(numa_hmat_cache->num_smbios_handles);
+            }
+        }
+    }
+}
+
 static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
 {
     /* Build HMAT Memory Subsystem Address Range. */
@@ -217,6 +270,9 @@ static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
 
     /* Build HMAT System Locality Latency and Bandwidth Information. */
     hmat_build_lb(hma);
+
+    /* Build HMAT Memory Side Cache Information. */
+    hmat_build_cache(hma);
 }
 
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index fddd05e0d1..f9fdcdcd33 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -33,6 +33,15 @@
 
 #define ACPI_HMAT_SPA               0
 #define ACPI_HMAT_LB_INFO           1
+#define ACPI_HMAT_CACHE_INFO        2
+
+#define MAX_HMAT_CACHE_LEVEL        3
+
+#define HMAT_CACHE_TOTAL_LEVEL(level)      (level & 0xF)
+#define HMAT_CACHE_CURRENT_LEVEL(level)    ((level & 0xF) << 4)
+#define HMAT_CACHE_ASSOC(assoc)            ((assoc & 0xF) << 8)
+#define HMAT_CACHE_WRITE_POLICY(policy)    ((policy & 0xF) << 12)
+#define HMAT_CACHE_LINE_SIZE(size)         ((size & 0xFFFF) << 16)
 
 /* ACPI HMAT sub-structure header */
 #define ACPI_HMAT_SUB_HEADER_DEF    \
@@ -102,6 +111,17 @@ struct AcpiHmatLBInfo {
 } QEMU_PACKED;
 typedef struct AcpiHmatLBInfo AcpiHmatLBInfo;
 
+struct AcpiHmatCacheInfo {
+    ACPI_HMAT_SUB_HEADER_DEF
+    uint32_t    mem_proximity;
+    uint32_t    reserved;
+    uint64_t    cache_size;
+    uint32_t    cache_attr;
+    uint16_t    reserved2;
+    uint16_t    num_smbios_handles;
+} QEMU_PACKED;
+typedef struct AcpiHmatCacheInfo AcpiHmatCacheInfo;
+
 struct numa_hmat_lb_info {
     /*
      * Indicates total number of Proximity Domains
@@ -141,7 +161,31 @@ struct numa_hmat_lb_info {
     uint16_t    bandwidth[MAX_NODES][MAX_NODES];
 };
 
+struct numa_hmat_cache_info {
+    /* The memory proximity domain to which the memory belongs. */
+    uint32_t    mem_proximity;
+    /* Size of memory side cache in bytes. */
+    uint64_t    size;
+    /* Total cache levels for this memory proximity domain. */
+    uint8_t     total_levels;
+    /* Cache level described in this structure. */
+    uint8_t     level;
+    /* Cache Associativity: None/Direct Mapped/Comple Cache Indexing */
+    uint8_t     associativity;
+    /* Write Policy: None/Write Back(WB)/Write Through(WT) */
+    uint8_t     write_policy;
+    /* Cache Line size in bytes. */
+    uint16_t    line_size;
+    /*
+     * Number of SMBIOS handles that contributes to
+     * the memory side cache physical devices.
+     */
+    uint16_t    num_smbios_handles;
+};
+
 extern struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES];
+extern struct numa_hmat_cache_info
+              *hmat_cache_info[MAX_NODES][MAX_HMAT_CACHE_LEVEL + 1];
 
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                      MachineState *machine);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (2 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 3/9] hmat acpi: Build Memory Side Cache " Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-14 19:38   ` Eric Blake
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 5/9] numa: Extend the command-line to provide memory side cache information Tao Xu
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

Add -numa hmat-lb option to provide System Locality Latency and
Bandwidth Information. These memory attributes help to build
System Locality Latency and Bandwidth Information Structure(s)
in ACPI Heterogeneous Memory Attribute Table (HMAT).

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 numa.c          | 124 ++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/misc.json  |  92 ++++++++++++++++++++++++++++++++++-
 qemu-options.hx |  28 ++++++++++-
 3 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/numa.c b/numa.c
index 9ee4f6f258..97b77356ad 100644
--- a/numa.c
+++ b/numa.c
@@ -40,6 +40,7 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qemu/cutils.h"
+#include "hw/acpi/hmat.h"
 
 QemuOptsList qemu_numa_opts = {
     .name = "numa",
@@ -180,6 +181,123 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp)
     have_numa_distance = true;
 }
 
+static void parse_numa_hmat_lb(MachineState *ms, NumaHmatLBOptions *node,
+                               Error **errp)
+{
+    struct numa_hmat_lb_info *hmat_lb = 0;
+
+    if (node->data_type <= HMATLB_DATA_TYPE_WRITE_LATENCY) {
+        if (!node->has_latency) {
+            error_setg(errp, "Please specify the latency.");
+            return;
+        }
+        if (node->has_bandwidth) {
+            error_setg(errp, "Please do not specify the bandwidth "
+                       "since the data type is latency.");
+            return;
+        }
+        if (node->has_base_bw) {
+            error_setg(errp, "Please do not specify the base-bw "
+                       "since the data type is latency.");
+            return;
+        }
+    }
+
+    if (node->data_type >= HMATLB_DATA_TYPE_ACCESS_BANDWIDTH) {
+        if (!node->has_bandwidth) {
+            error_setg(errp, "Please specify the bandwidth.");
+            return;
+        }
+        if (node->has_latency) {
+            error_setg(errp, "Please do not specify the latency "
+                       "since the data type is bandwidth.");
+            return;
+        }
+        if (node->has_base_lat) {
+            error_setg(errp, "Please do not specify the base-lat "
+                       "since the data type is bandwidth.");
+            return;
+        }
+    }
+
+    if (node->initiator >= nb_numa_nodes) {
+        error_setg(errp, "Invalid initiator=%"
+                   PRIu16 ", it should be less than %d.",
+                   node->initiator, nb_numa_nodes);
+        return;
+    }
+    if (!numa_info[node->initiator].is_initiator) {
+        error_setg(errp, "Invalid initiator=%"
+                   PRIu16 ", it isn't an initiator proximity domain.",
+                   node->initiator);
+        return;
+    }
+
+    if (node->target >= nb_numa_nodes) {
+        error_setg(errp, "Invalid initiator=%"
+                   PRIu16 ", it should be less than %d.",
+                   node->target, nb_numa_nodes);
+        return;
+    }
+    if (!numa_info[node->target].is_target) {
+        error_setg(errp, "Invalid target=%"
+                   PRIu16 ", it isn't a target proximity domain.",
+                   node->target);
+        return;
+    }
+
+    if (node->has_latency) {
+        hmat_lb = hmat_lb_info[node->hierarchy][node->data_type];
+        if (!hmat_lb) {
+            hmat_lb = g_malloc0(sizeof(*hmat_lb));
+            hmat_lb_info[node->hierarchy][node->data_type] = hmat_lb;
+        } else if (hmat_lb->latency[node->initiator][node->target]) {
+            error_setg(errp, "Duplicate configuration of the latency for "
+                       "initiator=%" PRIu16 " and target=%" PRIu16 ".",
+                       node->initiator, node->target);
+            return;
+        }
+
+        /* Only the first time of setting the base unit is valid. */
+        if ((hmat_lb->base_lat == 0) && (node->has_base_lat)) {
+            hmat_lb->base_lat = node->base_lat;
+        }
+
+        hmat_lb->latency[node->initiator][node->target] = node->latency;
+    }
+
+    if (node->has_bandwidth) {
+        hmat_lb = hmat_lb_info[node->hierarchy][node->data_type];
+
+        if (!hmat_lb) {
+            hmat_lb = g_malloc0(sizeof(*hmat_lb));
+            hmat_lb_info[node->hierarchy][node->data_type] = hmat_lb;
+        } else if (hmat_lb->bandwidth[node->initiator][node->target]) {
+            error_setg(errp, "Duplicate configuration of the bandwidth for "
+                       "initiator=%" PRIu16 " and target=%" PRIu16 ".",
+                       node->initiator, node->target);
+            return;
+        }
+
+        /* Only the first time of setting the base unit is valid. */
+        if (hmat_lb->base_bw == 0) {
+            if (!node->has_base_bw) {
+                error_setg(errp, "Please provide the base-bw!");
+                return;
+            } else {
+                hmat_lb->base_bw = node->base_bw;
+            }
+        }
+
+        hmat_lb->bandwidth[node->initiator][node->target] = node->bandwidth;
+    }
+
+    if (hmat_lb) {
+        hmat_lb->hierarchy = node->hierarchy;
+        hmat_lb->data_type = node->data_type;
+    }
+}
+
 static
 void set_numa_options(MachineState *ms, NumaOptions *object, Error **errp)
 {
@@ -213,6 +331,12 @@ void set_numa_options(MachineState *ms, NumaOptions *object, Error **errp)
         machine_set_cpu_numa_node(ms, qapi_NumaCpuOptions_base(&object->u.cpu),
                                   &err);
         break;
+    case NUMA_OPTIONS_TYPE_HMAT_LB:
+        parse_numa_hmat_lb(ms, &object->u.hmat_lb, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }
diff --git a/qapi/misc.json b/qapi/misc.json
index 24d20a880a..b18eb28459 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2746,10 +2746,12 @@
 #
 # @cpu: property based CPU(s) to node mapping (Since: 2.10)
 #
+# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
+#
 # Since: 2.1
 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node', 'dist', 'cpu' ] }
+  'data': [ 'node', 'dist', 'cpu', 'hmat-lb' ] }
 
 ##
 # @NumaOptions:
@@ -2764,7 +2766,8 @@
   'data': {
     'node': 'NumaNodeOptions',
     'dist': 'NumaDistOptions',
-    'cpu': 'NumaCpuOptions' }}
+    'cpu': 'NumaCpuOptions',
+    'hmat-lb': 'NumaHmatLBOptions' }}
 
 ##
 # @NumaNodeOptions:
@@ -2827,6 +2830,91 @@
    'base': 'CpuInstanceProperties',
    'data' : {} }
 
+##
+# @HmatLBMemoryHierarchy:
+#
+# The memory hierarchy in the System Locality Latency
+# and Bandwidth Information Structure of HMAT
+#
+# @memory: the structure represents the memory performance
+#
+# @last-level: last level memory of memory side cached memory
+#
+# @1st-level: first level memory of memory side cached memory
+#
+# @2nd-level: second level memory of memory side cached memory
+#
+# @3rd-level: third level memory of memory side cached memory
+#
+# Since: 2.13
+##
+{ 'enum': 'HmatLBMemoryHierarchy',
+  'data': [ 'memory', 'last-level', '1st-level',
+            '2nd-level', '3rd-level' ] }
+
+##
+# @HmatLBDataType:
+#
+# Data type in the System Locality Latency
+# and Bandwidth Information Structure of HMAT
+#
+# @access-latency: access latency
+#
+# @read-latency: read latency
+#
+# @write-latency: write latency
+#
+# @access-bandwidth: access bandwitch
+#
+# @read-bandwidth: read bandwidth
+#
+# @write-bandwidth: write bandwidth
+#
+# Since: 2.13
+##
+{ 'enum': 'HmatLBDataType',
+  'data': [ 'access-latency', 'read-latency', 'write-latency',
+            'access-bandwidth', 'read-bandwidth', 'write-bandwidth' ] }
+
+##
+# @NumaHmatLBOptions:
+#
+# Set the system locality latency and bandwidth information
+# between Initiator and Target proximity Domains.
+#
+# @initiator: the Initiator Proximity Domain.
+#
+# @target: the Target Proximity Domain.
+#
+# @hierarchy: the Memory Hierarchy. Indicates the performance
+#             of memory or side cache.
+#
+# @data-type: presents the type of data, access/read/write
+#             latency or hit latency.
+#
+# @base-lat: the base unit for latency in nanoseconds.
+#
+# @base-bw: the base unit for bandwidth in megabytes per second(MB/s).
+#
+# @latency: the value of latency based on Base Unit from @initiator
+#           to @target proximity domain.
+#
+# @bandwidth: the value of bandwidth based on Base Unit between
+#             @initiator and @target proximity domain.
+#
+# Since: 2.13
+##
+{ 'struct': 'NumaHmatLBOptions',
+  'data': {
+   'initiator': 'uint16',
+   'target': 'uint16',
+   'hierarchy': 'HmatLBMemoryHierarchy',
+   'data-type': 'HmatLBDataType',
+   '*base-lat': 'uint64',
+   '*base-bw': 'uint64',
+   '*latency': 'uint16',
+   '*bandwidth': 'uint16' }}
+
 ##
 # @HostMemPolicy:
 #
diff --git a/qemu-options.hx b/qemu-options.hx
index d4f3564b78..88f078c846 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -163,16 +163,19 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
     "-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
     "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
     "-numa dist,src=source,dst=destination,val=distance\n"
-    "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n",
+    "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
+    "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|last-level,data-type=access-latency|read-latency|write-latency[,base-lat=blat][,base-bw=bbw][,latency=lat][,bandwidth=bw]\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -numa node[,mem=@var{size}][,cpus=@var{firstcpu}[-@var{lastcpu}]][,nodeid=@var{node}]
 @itemx -numa node[,memdev=@var{id}][,cpus=@var{firstcpu}[-@var{lastcpu}]][,nodeid=@var{node}]
 @itemx -numa dist,src=@var{source},dst=@var{destination},val=@var{distance}
 @itemx -numa cpu,node-id=@var{node}[,socket-id=@var{x}][,core-id=@var{y}][,thread-id=@var{z}]
+@itemx -numa hmat-lb,initiator=@var{node},target=@var{node},hierarchy=@var{str},data-type=@var{str}[,base-lat=@var{blat}][,base-bw=@var{bbw}][,latency=@var{lat}][,bandwidth=@var{bw}]
 @findex -numa
 Define a NUMA node and assign RAM and VCPUs to it.
 Set the NUMA distance from a source node to a destination node.
+Set the ACPI Heterogeneous Memory Attribute for the given nodes.
 
 Legacy VCPU assignment uses @samp{cpus} option where
 @var{firstcpu} and @var{lastcpu} are CPU indexes. Each
@@ -230,6 +233,29 @@ specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},
 @option{-smp} options to allocate RAM and VCPUs respectively.
 
+Use 'hmat-lb' to set System Locality Latency and Bandwidth Information
+between initiator NUMA node and target NUMA node to build ACPI Heterogeneous Attribute Memory Table (HMAT).
+Initiator NUMA node can create memory requests, usually including one or more processors.
+Target NUMA node contains addressable memory.
+
+For example:
+@example
+-m 2G \
+-smp 3,sockets=2,maxcpus=3 \
+-numa node,cpus=0-1,nodeid=0 \
+-numa node,mem=1G,cpus=2,nodeid=1 \
+-numa node,mem=1G,nodeid=2 \
+-numa hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,base-lat=10,base-bw=20,latency=10,bandwidth=10 \
+-numa hmat-lb,initiator=1,target=2,hierarchy=1st-level,data-type=access-latency,base-bw=10,bandwidth=20
+@end example
+
+When the processors in NUMA node 0 access memory in NUMA node 1,
+the first line containing 'hmat-lb' sets the latency and bandwidth information.
+The latency is @var{lat} multiplied by @var{blat} and the bandwidth is @var{bw} multiplied by @var{bbw}.
+
+When the processors in NUMA node 1 access memory in NUMA node 2 that acts as 2nd level memory side cache,
+the second line containing 'hmat-lb' sets the access hit bandwidth information.
+
 ETEXI
 
 DEF("add-fd", HAS_ARG, QEMU_OPTION_add_fd,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 5/9] numa: Extend the command-line to provide memory side cache information
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (3 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 6/9] hmat acpi: Implement _HMA method to update HMAT at runtime Tao Xu
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

Add -numa hmat-cache option to provide Memory Side Cache Information.
These memory attributes help to build Memory Side Cache Information
Structure(s) in ACPI Heterogeneous Memory Attribute Table (HMAT).

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 numa.c         | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/misc.json | 72 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/numa.c b/numa.c
index 97b77356ad..c2f4049689 100644
--- a/numa.c
+++ b/numa.c
@@ -298,6 +298,72 @@ static void parse_numa_hmat_lb(MachineState *ms, NumaHmatLBOptions *node,
     }
 }
 
+static void parse_numa_hmat_cache(MachineState *ms, NumaHmatCacheOptions *node,
+                            Error **errp)
+{
+    struct numa_hmat_cache_info *hmat_cache;
+
+    if (node->node_id >= nb_numa_nodes) {
+        error_setg(errp, "Invalid node-id=%" PRIu32
+                   ", it should be less than %d.",
+                   node->node_id, nb_numa_nodes);
+        return;
+    }
+    if (!numa_info[node->node_id].is_target) {
+        error_setg(errp, "Invalid node-id=%" PRIu32
+                   ", it isn't a target proximity domain.",
+                   node->node_id);
+        return;
+    }
+
+    if (node->total > MAX_HMAT_CACHE_LEVEL) {
+        error_setg(errp, "Invalid total=%" PRIu8
+                   ", it should be less than or equal to %d.",
+                   node->total, MAX_HMAT_CACHE_LEVEL);
+        return;
+    }
+    if (node->level > node->total) {
+        error_setg(errp, "Invalid level=%" PRIu8
+                   ", it should be less than or equal to"
+                   " total=%" PRIu8 ".",
+                   node->level, node->total);
+        return;
+    }
+    if (hmat_cache_info[node->node_id][node->level]) {
+        error_setg(errp, "Duplicate configuration of the side cache for "
+                   "node-id=%" PRIu32 " and level=%" PRIu8 ".",
+                   node->node_id, node->level);
+        return;
+    }
+
+    if ((node->level > 1) &&
+        hmat_cache_info[node->node_id][node->level - 1] &&
+        (node->size >=
+            hmat_cache_info[node->node_id][node->level - 1]->size)) {
+        error_setg(errp, "Invalid size=0x%" PRIx64
+                   ", the size of level=%" PRIu8
+                   " should be less than the size(0x%" PRIx64
+                   ") of level=%" PRIu8 ".",
+                   node->size, node->level,
+                   hmat_cache_info[node->node_id][node->level - 1]->size,
+                   node->level - 1);
+        return;
+    }
+
+    hmat_cache = g_malloc0(sizeof(*hmat_cache));
+
+    hmat_cache->mem_proximity = node->node_id;
+    hmat_cache->size = node->size;
+    hmat_cache->total_levels = node->total;
+    hmat_cache->level = node->level;
+    hmat_cache->associativity = node->assoc;
+    hmat_cache->write_policy = node->policy;
+    hmat_cache->line_size = node->line;
+    hmat_cache->num_smbios_handles = 0;
+
+    hmat_cache_info[node->node_id][node->level] = hmat_cache;
+}
+
 static
 void set_numa_options(MachineState *ms, NumaOptions *object, Error **errp)
 {
@@ -337,6 +403,12 @@ void set_numa_options(MachineState *ms, NumaOptions *object, Error **errp)
             goto end;
         }
         break;
+    case NUMA_OPTIONS_TYPE_HMAT_CACHE:
+        parse_numa_hmat_cache(ms, &object->u.hmat_cache, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }
diff --git a/qapi/misc.json b/qapi/misc.json
index b18eb28459..0887a3791a 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2748,10 +2748,12 @@
 #
 # @hmat-lb: memory latency and bandwidth information (Since: 2.13)
 #
+# @hmat-cache: memory side cache information (Since: 2.13)
+#
 # Since: 2.1
 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node', 'dist', 'cpu', 'hmat-lb' ] }
+  'data': [ 'node', 'dist', 'cpu', 'hmat-lb', 'hmat-cache' ] }
 
 ##
 # @NumaOptions:
@@ -2767,7 +2769,8 @@
     'node': 'NumaNodeOptions',
     'dist': 'NumaDistOptions',
     'cpu': 'NumaCpuOptions',
-    'hmat-lb': 'NumaHmatLBOptions' }}
+    'hmat-lb': 'NumaHmatLBOptions',
+    'hmat-cache': 'NumaHmatCacheOptions' }}
 
 ##
 # @NumaNodeOptions:
@@ -2915,6 +2918,71 @@
    '*latency': 'uint16',
    '*bandwidth': 'uint16' }}
 
+##
+# @HmatCacheAssociativity:
+#
+# Cache associativity in the Memory Side Cache
+# Information Structure of HMAT
+#
+# @none: None
+#
+# @direct: Direct Mapped
+#
+# @complex: Complex Cache Indexing (implementation specific)
+#
+# Since: 2.13
+##
+{ 'enum': 'HmatCacheAssociativity',
+  'data': [ 'none', 'direct', 'complex' ] }
+
+##
+# @HmatCacheWritePolicy:
+#
+# Cache write policy in the Memory Side Cache
+# Information Structure of HMAT
+#
+# @none: None
+#
+# @write-back: Write Back (WB)
+#
+# @write-through: Write Through (WT)
+#
+# Since: 2.13
+##
+{ 'enum': 'HmatCacheWritePolicy',
+  'data': [ 'none', 'write-back', 'write-through' ] }
+
+##
+# @NumaHmatCacheOptions:
+#
+# Set the memory side cache information for a given memory domain.
+#
+# @node-id: the memory proximity domain to which the memory belongs.
+#
+# @size: the size of memory side cache in bytes.
+#
+# @total: the total cache levels for this memory proximity domain.
+#
+# @level: the cache level described in this structure.
+#
+# @assoc: the cache associativity, none/direct-mapped/complex(complex cache indexing).
+
+# @policy: the write policy, none/write-back/write-through.
+#
+# @line: the cache Line size in bytes.
+#
+# Since: 2.13
+##
+{ 'struct': 'NumaHmatCacheOptions',
+  'data': {
+   'node-id': 'uint32',
+   'size': 'size',
+   'total': 'uint8',
+   'level': 'uint8',
+   'assoc': 'HmatCacheAssociativity',
+   'policy': 'HmatCacheWritePolicy',
+   'line': 'uint16' }}
+
 ##
 # @HostMemPolicy:
 #
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 6/9] hmat acpi: Implement _HMA method to update HMAT at runtime
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (4 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 5/9] numa: Extend the command-line to provide memory side cache information Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues Tao Xu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

From: Liu Jingqi <jingqi.liu@intel.com>

OSPM evaluates HMAT only during system initialization.
Any changes to the HMAT state at runtime or information
regarding HMAT for hot plug are communicated using _HMA method.

_HMA is an optional object that enables the platform to provide
the OS with updated Heterogeneous Memory Attributes information
at runtime. _HMA provides OSPM with the latest HMAT in entirety
overriding existing HMAT.

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c       | 356 +++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/hmat.h       |  71 +++++++++
 hw/i386/acpi-build.c |   2 +
 hw/i386/pc.c         |   2 +
 hw/i386/pc_piix.c    |   3 +
 hw/i386/pc_q35.c     |   3 +
 include/hw/i386/pc.h |   2 +
 7 files changed, 439 insertions(+)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index 9d29ef7929..cf17c0ae4f 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -275,6 +275,267 @@ static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
     hmat_build_cache(hma);
 }
 
+static uint64_t
+hmat_hma_method_read(void *opaque, hwaddr addr, unsigned size)
+{
+    printf("BUG: we never read _HMA IO Port.\n");
+    return 0;
+}
+
+/* _HMA Method: read HMA data. */
+static void hmat_handle_hma_method(AcpiHmaState *state,
+                                   HmatHmamIn *in, hwaddr hmam_mem_addr)
+{
+    HmatHmaBuffer *hma_buf = &state->hma_buf;
+    HmatHmamOut *read_hma_out;
+    GArray *hma;
+    uint32_t read_len = 0, ret_status;
+    int size;
+
+    le32_to_cpus(&in->offset);
+
+    hma = hma_buf->hma;
+    if (in->offset > hma->len) {
+        ret_status = HMAM_RET_STATUS_INVALID;
+        goto exit;
+    }
+
+   /* It is the first time to read HMA. */
+    if (!in->offset) {
+        hma_buf->dirty = false;
+    } else if (hma_buf->dirty) { /* HMA has been changed during Reading HMA. */
+        ret_status = HMAM_RET_STATUS_HMA_CHANGED;
+        goto exit;
+    }
+
+    ret_status = HMAM_RET_STATUS_SUCCESS;
+    read_len = MIN(hma->len - in->offset,
+                   HMAM_MEMORY_SIZE - 2 * sizeof(uint32_t));
+exit:
+    size = sizeof(HmatHmamOut) + read_len;
+    read_hma_out = g_malloc(size);
+
+    read_hma_out->len = cpu_to_le32(size);
+    read_hma_out->ret_status = cpu_to_le32(ret_status);
+    memcpy(read_hma_out->data, hma->data + in->offset, read_len);
+
+    cpu_physical_memory_write(hmam_mem_addr, read_hma_out, size);
+
+    g_free(read_hma_out);
+}
+
+static void
+hmat_hma_method_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    AcpiHmaState *state = opaque;
+    hwaddr hmam_mem_addr = val;
+    HmatHmamIn *in;
+
+    in = g_new(HmatHmamIn, 1);
+    cpu_physical_memory_read(hmam_mem_addr, in, sizeof(*in));
+
+    hmat_handle_hma_method(state, in, hmam_mem_addr);
+}
+
+static const MemoryRegionOps hmat_hma_method_ops = {
+    .read = hmat_hma_method_read,
+    .write = hmat_hma_method_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void hmat_init_hma_buffer(HmatHmaBuffer *hma_buf)
+{
+    hma_buf->hma = g_array_new(false, true /* clear */, 1);
+}
+
+static uint8_t hmat_acpi_table_checksum(uint8_t *buffer, uint32_t length)
+{
+    uint8_t sum = 0;
+    uint8_t *end = buffer + length;
+
+    while (buffer < end) {
+        sum = (uint8_t) (sum + *(buffer++));
+    }
+    return (uint8_t)(0 - sum);
+}
+
+static void hmat_build_header(AcpiTableHeader *h,
+             const char *sig, int len, uint8_t rev,
+             const char *oem_id, const char *oem_table_id)
+{
+    memcpy(&h->signature, sig, 4);
+    h->length = cpu_to_le32(len);
+    h->revision = rev;
+
+    if (oem_id) {
+        strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
+    } else {
+        memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
+    }
+
+    if (oem_table_id) {
+        strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
+    } else {
+        memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
+        memcpy(h->oem_table_id + 4, sig, 4);
+    }
+
+    h->oem_revision = cpu_to_le32(1);
+    memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
+    h->asl_compiler_revision = cpu_to_le32(1);
+
+    /* Caculate the checksum of acpi table. */
+    h->checksum = 0;
+    h->checksum = hmat_acpi_table_checksum((uint8_t *)h, len);
+}
+
+static void hmat_build_hma_buffer(PCMachineState *pcms)
+{
+    HmatHmaBuffer *hma_buf = &(pcms->acpi_hma_state.hma_buf);
+
+    /* Free the old hma buffer before new allocation. */
+    g_array_free(hma_buf->hma, true);
+
+    hma_buf->hma = g_array_new(false, true /* clear */, 1);
+    acpi_data_push(hma_buf->hma, sizeof(AcpiHmat));
+
+    /* build HMAT in a given buffer. */
+    hmat_build_hma(hma_buf->hma, pcms);
+    hmat_build_header((void *)hma_buf->hma->data,
+                      "HMAT", hma_buf->hma->len, 1, NULL, NULL);
+    hma_buf->dirty = true;
+}
+
+static void hmat_build_common_aml(Aml *dev)
+{
+    Aml *method, *ifctx, *hmam_mem;
+    Aml *unsupport;
+    Aml *pckg, *pckg_index, *pckg_buf, *field;
+    Aml *hmam_out_buf, *hmam_out_buf_size;
+    uint8_t byte_list[1];
+
+    method = aml_method(HMA_COMMON_METHOD, 1, AML_SERIALIZED);
+    hmam_mem = aml_local(6);
+    hmam_out_buf = aml_local(7);
+
+    aml_append(method, aml_store(aml_name(HMAM_ACPI_MEM_ADDR), hmam_mem));
+
+    /* map _HMA memory and IO into ACPI namespace. */
+    aml_append(method, aml_operation_region(HMAM_IOPORT, AML_SYSTEM_IO,
+               aml_int(HMAM_ACPI_IO_BASE), HMAM_ACPI_IO_LEN));
+    aml_append(method, aml_operation_region(HMAM_MEMORY,
+               AML_SYSTEM_MEMORY, hmam_mem, HMAM_MEMORY_SIZE));
+
+    /*
+     * _HMAC notifier:
+     * HMAM_NOTIFY: write the address of DSM memory and notify QEMU to
+     *                    emulate the access.
+     *
+     * It is the IO port so that accessing them will cause VM-exit, the
+     * control will be transferred to QEMU.
+     */
+    field = aml_field(HMAM_IOPORT, AML_DWORD_ACC, AML_NOLOCK,
+                      AML_PRESERVE);
+    aml_append(field, aml_named_field(HMAM_NOTIFY,
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(method, field);
+
+    /*
+     * _HMAC input:
+     * HMAM_OFFSET: store the current offset of _HMA buffer.
+     *
+     * They are RAM mapping on host so that these accesses never cause VMExit.
+     */
+    field = aml_field(HMAM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
+                      AML_PRESERVE);
+    aml_append(field, aml_named_field(HMAM_OFFSET,
+               sizeof(typeof_field(HmatHmamIn, offset)) * BITS_PER_BYTE));
+    aml_append(method, field);
+
+    /*
+     * _HMAC output:
+     * HMAM_OUT_BUF_SIZE: the size of the buffer filled by QEMU.
+     * HMAM_OUT_BUF: the buffer QEMU uses to store the result.
+     *
+     * Since the page is reused by both input and out, the input data
+     * will be lost after storing new result into ODAT so we should fetch
+     * all the input data before writing the result.
+     */
+    field = aml_field(HMAM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
+                      AML_PRESERVE);
+    aml_append(field, aml_named_field(HMAM_OUT_BUF_SIZE,
+               sizeof(typeof_field(HmatHmamOut, len)) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field(HMAM_OUT_BUF,
+       (sizeof(HmatHmamOut) - sizeof(uint32_t)) * BITS_PER_BYTE));
+    aml_append(method, field);
+
+    /*
+     * do not support any method if HMA memory address has not been
+     * patched.
+     */
+    unsupport = aml_if(aml_equal(hmam_mem, aml_int(0x0)));
+    byte_list[0] = HMAM_RET_STATUS_UNSUPPORT;
+    aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
+    aml_append(method, unsupport);
+
+    /* The parameter (Arg0) of _HMAC is a package which contains a buffer. */
+    pckg = aml_arg(0);
+    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+                   aml_int(4 /* Package */)) /* It is a Package? */,
+                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element */,
+                   NULL));
+
+    pckg_index = aml_local(2);
+    pckg_buf = aml_local(3);
+    aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_index));
+    aml_append(ifctx, aml_store(aml_derefof(pckg_index), pckg_buf));
+    aml_append(ifctx, aml_store(pckg_buf, aml_name(HMAM_OFFSET)));
+    aml_append(method, ifctx);
+
+    /*
+     * tell QEMU about the real address of HMA memory, then QEMU
+     * gets the control and fills the result in _HMAC memory.
+     */
+    aml_append(method, aml_store(hmam_mem, aml_name(HMAM_NOTIFY)));
+
+    hmam_out_buf_size = aml_local(1);
+    /* RLEN is not included in the payload returned to guest. */
+    aml_append(method, aml_subtract(aml_name(HMAM_OUT_BUF_SIZE),
+                                aml_int(4), hmam_out_buf_size));
+    aml_append(method, aml_store(aml_shiftleft(hmam_out_buf_size, aml_int(3)),
+                                 hmam_out_buf_size));
+    aml_append(method, aml_create_field(aml_name(HMAM_OUT_BUF),
+                                aml_int(0), hmam_out_buf_size, "OBUF"));
+    aml_append(method, aml_concatenate(aml_buffer(0, NULL), aml_name("OBUF"),
+                                hmam_out_buf));
+    aml_append(method, aml_return(hmam_out_buf));
+    aml_append(dev, method);
+}
+
+void hmat_init_acpi_state(AcpiHmaState *state, MemoryRegion *io,
+                          FWCfgState *fw_cfg, Object *owner)
+{
+    memory_region_init_io(&state->io_mr, owner, &hmat_hma_method_ops, state,
+                          "hma-acpi-io", HMAM_ACPI_IO_LEN);
+    memory_region_add_subregion(io, HMAM_ACPI_IO_BASE, &state->io_mr);
+
+    state->hmam_mem = g_array_new(false, true /* clear */, 1);
+    fw_cfg_add_file(fw_cfg, HMAM_MEM_FILE, state->hmam_mem->data,
+                    state->hmam_mem->len);
+
+    hmat_init_hma_buffer(&state->hma_buf);
+}
+
+void hmat_update(PCMachineState *pcms)
+{
+    /* build HMAT in a given buffer. */
+    hmat_build_hma_buffer(pcms);
+}
+
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                      MachineState *machine)
 {
@@ -291,3 +552,98 @@ void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                  (void *)(table_data->data + hmat_start),
                  "HMAT", hmat_len, 1, NULL, NULL);
 }
+
+void hmat_build_aml(Aml *dev)
+{
+    Aml *method, *pkg, *buf, *buf_size, *offset, *call_result;
+    Aml *whilectx, *ifcond, *ifctx, *elsectx, *hma;
+
+    hmat_build_common_aml(dev);
+
+    buf = aml_local(0);
+    buf_size = aml_local(1);
+    hma = aml_local(2);
+
+    aml_append(dev, aml_name_decl(HMAM_RHMA_STATUS, aml_int(0)));
+
+    /* build helper function, RHMA. */
+    method = aml_method("RHMA", 1, AML_SERIALIZED);
+    aml_append(method, aml_name_decl("OFST", aml_int(0)));
+
+    /* prepare input package. */
+    pkg = aml_package(1);
+    aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
+    aml_append(pkg, aml_name("OFST"));
+
+    /* call Read HMA function. */
+    call_result = aml_call1(HMA_COMMON_METHOD, pkg);
+    aml_append(method, aml_store(call_result, buf));
+
+    /* handle _HMAC result. */
+    aml_append(method, aml_create_dword_field(buf,
+               aml_int(0) /* offset at byte 0 */, "STAU"));
+
+    aml_append(method, aml_store(aml_name("STAU"),
+                                 aml_name(HMAM_RHMA_STATUS)));
+
+    /* if something is wrong during _HMAC. */
+    ifcond = aml_equal(aml_int(HMAM_RET_STATUS_SUCCESS),
+                       aml_name("STAU"));
+    ifctx = aml_if(aml_lnot(ifcond));
+    aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
+    aml_append(method, ifctx);
+
+    aml_append(method, aml_store(aml_sizeof(buf), buf_size));
+    aml_append(method, aml_subtract(buf_size,
+                                    aml_int(4) /* the size of "STAU" */,
+                                    buf_size));
+
+    /* if we read the end of hma. */
+    ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
+    aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
+    aml_append(method, ifctx);
+
+    aml_append(method, aml_create_field(buf,
+                            aml_int(4 * BITS_PER_BYTE), /* offset at byte 4.*/
+                            aml_shiftleft(buf_size, aml_int(3)), "BUFF"));
+    aml_append(method, aml_return(aml_name("BUFF")));
+    aml_append(dev, method);
+
+    /* build _HMA. */
+    method = aml_method("_HMA", 0, AML_SERIALIZED);
+    offset = aml_local(3);
+
+    aml_append(method, aml_store(aml_buffer(0, NULL), hma));
+    aml_append(method, aml_store(aml_int(0), offset));
+
+    whilectx = aml_while(aml_int(1));
+    aml_append(whilectx, aml_store(aml_call1("RHMA", offset), buf));
+    aml_append(whilectx, aml_store(aml_sizeof(buf), buf_size));
+
+    /*
+     * if hma buffer was changed during RHMA, read from the beginning
+     * again.
+     */
+    ifctx = aml_if(aml_equal(aml_name(HMAM_RHMA_STATUS),
+                             aml_int(HMAM_RET_STATUS_HMA_CHANGED)));
+    aml_append(ifctx, aml_store(aml_buffer(0, NULL), hma));
+    aml_append(ifctx, aml_store(aml_int(0), offset));
+    aml_append(whilectx, ifctx);
+
+    elsectx = aml_else();
+
+    /* finish hma read if no data is read out. */
+    ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
+    aml_append(ifctx, aml_return(hma));
+    aml_append(elsectx, ifctx);
+
+    /* update the offset. */
+    aml_append(elsectx, aml_add(offset, buf_size, offset));
+    /* append the data we read out to the hma buffer. */
+    aml_append(elsectx, aml_concatenate(hma, buf, hma));
+    aml_append(whilectx, elsectx);
+    aml_append(method, whilectx);
+
+    aml_append(dev, method);
+}
+
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index f9fdcdcd33..dd6948f738 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -183,11 +183,82 @@ struct numa_hmat_cache_info {
     uint16_t    num_smbios_handles;
 };
 
+#define HMAM_MEMORY_SIZE    4096
+#define HMAM_MEM_FILE       "etc/acpi/hma-mem"
+
+/*
+ * 32 bits IO port starting from 0x0a19 in guest is reserved for
+ * HMA ACPI emulation.
+ */
+#define HMAM_ACPI_IO_BASE     0x0a19
+#define HMAM_ACPI_IO_LEN      4
+
+#define HMAM_ACPI_MEM_ADDR  "HMTA"
+#define HMAM_MEMORY         "HRAM"
+#define HMAM_IOPORT         "HPIO"
+
+#define HMAM_NOTIFY         "NTFI"
+#define HMAM_OUT_BUF_SIZE   "RLEN"
+#define HMAM_OUT_BUF        "ODAT"
+
+#define HMAM_RHMA_STATUS    "RSTA"
+#define HMA_COMMON_METHOD   "HMAC"
+#define HMAM_OFFSET         "OFFT"
+
+#define HMAM_RET_STATUS_SUCCESS        0 /* Success */
+#define HMAM_RET_STATUS_UNSUPPORT      1 /* Not Supported */
+#define HMAM_RET_STATUS_INVALID        2 /* Invalid Input Parameters */
+#define HMAM_RET_STATUS_HMA_CHANGED    0x100 /* HMA Changed */
+
+/*
+ * HmatHmaBuffer:
+ * @hma: HMA buffer with the updated HMAT. It is updated when
+ *   the memory device is plugged or unplugged.
+ * @dirty: It allows OSPM to detect changes and restart read if there is any.
+ */
+struct HmatHmaBuffer {
+    GArray *hma;
+    bool dirty;
+};
+typedef struct HmatHmaBuffer HmatHmaBuffer;
+
+struct AcpiHmaState {
+    /* detect if HMA support is enabled. */
+    bool is_enabled;
+
+    /* the data of the fw_cfg file HMAM_MEM_FILE. */
+    GArray *hmam_mem;
+
+    HmatHmaBuffer hma_buf;
+
+    /* the IO region used by OSPM to transfer control to QEMU. */
+    MemoryRegion io_mr;
+};
+typedef struct AcpiHmaState AcpiHmaState;
+
+struct HmatHmamIn {
+    /* the offset in the _HMA buffer */
+    uint32_t offset;
+} QEMU_PACKED;
+typedef struct HmatHmamIn HmatHmamIn;
+
+struct HmatHmamOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t ret_status;   /* return status code. */
+    uint8_t data[4088];
+} QEMU_PACKED;
+typedef struct HmatHmamOut HmatHmamOut;
+
 extern struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES];
 extern struct numa_hmat_cache_info
               *hmat_cache_info[MAX_NODES][MAX_HMAT_CACHE_LEVEL + 1];
 
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                      MachineState *machine);
+void hmat_build_aml(Aml *dsdt);
+void hmat_init_acpi_state(AcpiHmaState *state, MemoryRegion *io,
+                          FWCfgState *fw_cfg, Object *owner);
+void hmat_update(PCMachineState *pcms);
 
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a93d437175..569132f3ab 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1845,6 +1845,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         build_q35_pci0_int(dsdt);
     }
 
+    hmat_build_aml(dsdt);
+
     if (pcmc->legacy_cpu_hotplug) {
         build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
     } else {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4952feb476..9afed44139 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2401,6 +2401,8 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev,
         nvdimm_plug(&pcms->acpi_nvdimm_state);
     }
 
+    hmat_update(pcms);
+
     hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
     hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort);
 out:
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ed6984638e..38d7a758ef 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -301,6 +301,9 @@ static void pc_init1(MachineState *machine,
         nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
                                pcms->fw_cfg, OBJECT(pcms));
     }
+
+    hmat_init_acpi_state(&pcms->acpi_hma_state, system_io,
+                         pcms->fw_cfg, OBJECT(pcms));
 }
 
 /* Looking for a pc_compat_2_4() function? It doesn't exist.
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b7b7959934..e819c3b2f6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -333,6 +333,9 @@ static void pc_q35_init(MachineState *machine)
         nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
                                pcms->fw_cfg, OBJECT(pcms));
     }
+
+    hmat_init_acpi_state(&pcms->acpi_hma_state, system_io,
+                         pcms->fw_cfg, OBJECT(pcms));
 }
 
 #define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 84720bede9..800e9cac1d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -16,6 +16,7 @@
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/acpi_dev_interface.h"
+#include "hw/acpi/hmat.h"
 
 #define HPET_INTCAP "hpet-intcap"
 
@@ -46,6 +47,7 @@ struct PCMachineState {
     OnOffAuto smm;
 
     AcpiNVDIMMState acpi_nvdimm_state;
+    AcpiHmaState acpi_hma_state;
 
     bool acpi_build_enabled;
     bool smbus_enabled;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (5 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 6/9] hmat acpi: Implement _HMA method to update HMAT at runtime Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-21 17:11   ` Eric Blake
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 8/9] hmat acpi: move some function inside of the caller Tao Xu
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

Per Igor and Eric's comments, fix some small issues of V1 patch:
  - update the version number in qapi/misc.json
  - including the expansion of the acronym HMAT in qapi/misc.json
  - correct spell mistakes in qapi/misc.json and qemu-options.hx
  - fix the comment syle in hw/i386/acpi-build.c
  and hw/acpi/hmat.h
  - remove some unnecessary head files in hw/acpi/hmat.c
  - use hardcoded numbers from spec to generate
  Memory Subsystem Address Range Structure in hw/acpi/hmat.c
  - drop the struct AcpiHmat and AcpiHmatSpaRange
  in hw/acpi/hmat.h

Reviewed-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c       | 39 +++++++++++++++++----------------------
 hw/acpi/hmat.h       | 26 ++++----------------------
 hw/i386/acpi-build.c |  6 ++++--
 qapi/misc.json       | 44 +++++++++++++++++++++++---------------------
 qemu-options.hx      |  2 +-
 5 files changed, 49 insertions(+), 68 deletions(-)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index cf17c0ae4f..e8ba9250e9 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -22,17 +22,12 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
 
-#include "unistd.h"
-#include "fcntl.h"
 #include "qemu/osdep.h"
 #include "sysemu/numa.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/acpi-build.h"
-#include "hw/acpi/acpi.h"
 #include "hw/acpi/hmat.h"
-#include "hw/acpi/aml-build.h"
 #include "hw/nvram/fw_cfg.h"
-#include "hw/acpi/bios-linker-loader.h"
 
 struct numa_hmat_lb_info *hmat_lb_info[HMAT_LB_LEVELS][HMAT_LB_TYPES] = {0};
 struct numa_hmat_cache_info
@@ -42,7 +37,7 @@ static uint32_t initiator_pxm[MAX_NODES], target_pxm[MAX_NODES];
 static uint32_t num_initiator, num_target;
 
 /* Build Memory Subsystem Address Range Structure */
-static void hmat_build_spa_info(GArray *table_data,
+static void build_hmat_spa(GArray *table_data,
                                 uint64_t base, uint64_t length, int node)
 {
     uint16_t flags = 0;
@@ -54,27 +49,27 @@ static void hmat_build_spa_info(GArray *table_data,
         flags |= HMAT_SPA_MEM_VALID;
     }
 
+    /* Memory Subsystem Address Range Structure */
     /* Type */
-    build_append_int_noprefix(table_data, ACPI_HMAT_SPA, sizeof(uint16_t));
-    /* Reserved0 */
-    build_append_int_noprefix(table_data, 0, sizeof(uint16_t));
+    build_append_int_noprefix(table_data, 0, 2);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 2);
     /* Length */
-    build_append_int_noprefix(table_data, sizeof(AcpiHmatSpaRange),
-                              sizeof(uint32_t));
+    build_append_int_noprefix(table_data, 40, 4);
     /* Flags */
-    build_append_int_noprefix(table_data, flags, sizeof(uint16_t));
-    /* Reserved1 */
-    build_append_int_noprefix(table_data, 0, sizeof(uint16_t));
+    build_append_int_noprefix(table_data, flags, 2);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 2);
     /* Process Proximity Domain */
-    build_append_int_noprefix(table_data, node, sizeof(uint32_t));
+    build_append_int_noprefix(table_data, node, 4);
     /* Memory Proximity Domain */
-    build_append_int_noprefix(table_data, node, sizeof(uint32_t));
-    /* Reserved2 */
-    build_append_int_noprefix(table_data, 0, sizeof(uint32_t));
+    build_append_int_noprefix(table_data, node, 4);
+    /* Reserved */
+    build_append_int_noprefix(table_data, 0, 4);
     /* System Physical Address Range Base */
-    build_append_int_noprefix(table_data, base, sizeof(uint64_t));
+    build_append_int_noprefix(table_data, base, 8);
     /* System Physical Address Range Length */
-    build_append_int_noprefix(table_data, length, sizeof(uint64_t));
+    build_append_int_noprefix(table_data, length, 8);
 }
 
 static int pc_dimm_device_list(Object *obj, void *opaque)
@@ -401,7 +396,7 @@ static void hmat_build_hma_buffer(PCMachineState *pcms)
     g_array_free(hma_buf->hma, true);
 
     hma_buf->hma = g_array_new(false, true /* clear */, 1);
-    acpi_data_push(hma_buf->hma, sizeof(AcpiHmat));
+    acpi_data_push(hma_buf->hma, 40);
 
     /* build HMAT in a given buffer. */
     hmat_build_hma(hma_buf->hma, pcms);
@@ -543,7 +538,7 @@ void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
     uint64_t hmat_start, hmat_len;
 
     hmat_start = table_data->len;
-    acpi_data_push(table_data, sizeof(AcpiHmat));
+    acpi_data_push(table_data, 40);
 
     hmat_build_hma(table_data, pcms);
     hmat_len = table_data->len - hmat_start;
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index dd6948f738..2c080a51b8 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -78,27 +78,6 @@ enum {
 #define HMAT_LB_LEVELS    (HMAT_LB_MEM_CACHE_3RD_LEVEL + 1)
 #define HMAT_LB_TYPES     (HMAT_LB_DATA_WRITE_BANDWIDTH + 1)
 
-/*
- * HMAT (Heterogeneous Memory Attributes Table)
- */
-struct AcpiHmat {
-    ACPI_TABLE_HEADER_DEF
-    uint32_t    reserved;
-} QEMU_PACKED;
-typedef struct AcpiHmat AcpiHmat;
-
-struct AcpiHmatSpaRange {
-    ACPI_HMAT_SUB_HEADER_DEF
-    uint16_t    flags;
-    uint16_t    reserved1;
-    uint32_t    proc_proximity;
-    uint32_t    mem_proximity;
-    uint32_t    reserved2;
-    uint64_t    spa_base;
-    uint64_t    spa_length;
-} QEMU_PACKED;
-typedef struct AcpiHmatSpaRange AcpiHmatSpaRange;
-
 struct AcpiHmatLBInfo {
     ACPI_HMAT_SUB_HEADER_DEF
     uint8_t     flags;
@@ -166,7 +145,10 @@ struct numa_hmat_cache_info {
     uint32_t    mem_proximity;
     /* Size of memory side cache in bytes. */
     uint64_t    size;
-    /* Total cache levels for this memory proximity domain. */
+    /*
+     * Total cache levels for this memory
+     * pr#include "hw/acpi/aml-build.h"oximity domain.
+     */
     uint8_t     total_levels;
     /* Cache level described in this structure. */
     uint8_t     level;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 569132f3ab..729e67e829 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -120,7 +120,8 @@ typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
-/* The memory contains at least one hole
+/*
+ * The memory contains at least one hole
  * from 640k-1M and possibly another one from 3.5G-4G.
  * So far, the number of memory ranges is up to 2
  * more than the number of numa nodes.
@@ -2267,7 +2268,8 @@ void build_mem_ranges(PCMachineState *pcms)
     uint64_t mem_len, mem_base, next_base;
     int i;
 
-    /* the memory map is a bit tricky, it contains at least one hole
+    /*
+     * the memory map is a bit tricky, it contains at least one hole
      * from 640k-1M and possibly another one from 3.5G-4G.
      */
     mem_ranges_number = 0;
diff --git a/qapi/misc.json b/qapi/misc.json
index 0887a3791a..dc06190168 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2746,9 +2746,9 @@
 #
 # @cpu: property based CPU(s) to node mapping (Since: 2.10)
 #
-# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
+# @hmat-lb: memory latency and bandwidth information (Since: 3.10)
 #
-# @hmat-cache: memory side cache information (Since: 2.13)
+# @hmat-cache: memory side cache information (Since: 3.10)
 #
 # Since: 2.1
 ##
@@ -2837,43 +2837,45 @@
 # @HmatLBMemoryHierarchy:
 #
 # The memory hierarchy in the System Locality Latency
-# and Bandwidth Information Structure of HMAT
+# and Bandwidth Information Structure of HMAT (Heterogeneous
+# Memory Attribute Table)
 #
 # @memory: the structure represents the memory performance
 #
 # @last-level: last level memory of memory side cached memory
 #
-# @1st-level: first level memory of memory side cached memory
+# @first-level: first level memory of memory side cached memory
 #
-# @2nd-level: second level memory of memory side cached memory
+# @second-level: second level memory of memory side cached memory
 #
-# @3rd-level: third level memory of memory side cached memory
+# @third-level: third level memory of memory side cached memory
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'enum': 'HmatLBMemoryHierarchy',
-  'data': [ 'memory', 'last-level', '1st-level',
-            '2nd-level', '3rd-level' ] }
+  'data': [ 'memory', 'last-level', 'first-level',
+            'second-level', 'third-level' ] }
 
 ##
 # @HmatLBDataType:
 #
 # Data type in the System Locality Latency
-# and Bandwidth Information Structure of HMAT
+# and Bandwidth Information Structure of HMAT (Heterogeneous
+# Memory Attribute Table)
 #
-# @access-latency: access latency
+# @access-latency: access latency (nanoseconds)
 #
-# @read-latency: read latency
+# @read-latency: read latency (nanoseconds)
 #
-# @write-latency: write latency
+# @write-latency: write latency (nanoseconds)
 #
-# @access-bandwidth: access bandwitch
+# @access-bandwidth: access bandwidth (MB/s)
 #
-# @read-bandwidth: read bandwidth
+# @read-bandwidth: read bandwidth (MB/s)
 #
-# @write-bandwidth: write bandwidth
+# @write-bandwidth: write bandwidth (MB/s)
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'enum': 'HmatLBDataType',
   'data': [ 'access-latency', 'read-latency', 'write-latency',
@@ -2905,7 +2907,7 @@
 # @bandwidth: the value of bandwidth based on Base Unit between
 #             @initiator and @target proximity domain.
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'struct': 'NumaHmatLBOptions',
   'data': {
@@ -2930,7 +2932,7 @@
 #
 # @complex: Complex Cache Indexing (implementation specific)
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'enum': 'HmatCacheAssociativity',
   'data': [ 'none', 'direct', 'complex' ] }
@@ -2947,7 +2949,7 @@
 #
 # @write-through: Write Through (WT)
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'enum': 'HmatCacheWritePolicy',
   'data': [ 'none', 'write-back', 'write-through' ] }
@@ -2971,7 +2973,7 @@
 #
 # @line: the cache Line size in bytes.
 #
-# Since: 2.13
+# Since: 3.10
 ##
 { 'struct': 'NumaHmatCacheOptions',
   'data': {
diff --git a/qemu-options.hx b/qemu-options.hx
index 88f078c846..99363b7144 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -246,7 +246,7 @@ For example:
 -numa node,mem=1G,cpus=2,nodeid=1 \
 -numa node,mem=1G,nodeid=2 \
 -numa hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,base-lat=10,base-bw=20,latency=10,bandwidth=10 \
--numa hmat-lb,initiator=1,target=2,hierarchy=1st-level,data-type=access-latency,base-bw=10,bandwidth=20
+-numa hmat-lb,initiator=1,target=2,hierarchy=first-level,data-type=access-latency,base-bw=10,bandwidth=20
 @end example
 
 When the processors in NUMA node 0 access memory in NUMA node 1,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 8/9] hmat acpi: move some function inside of the caller
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (6 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 9/9] acpi: rewrite the _FIT method to use it in _HMA method Tao Xu
  2019-01-13 19:07 ` [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) no-reply
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

Per Igor comments, these function is used only once,
move its body inside of the caller

Reviewed-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c | 93 ++++++++++++++++++++------------------------------
 1 file changed, 37 insertions(+), 56 deletions(-)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index e8ba9250e9..4523e98ef1 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -84,22 +84,41 @@ static int pc_dimm_device_list(Object *obj, void *opaque)
     return 0;
 }
 
+static void classify_proximity_domains(void)
+{
+    int node;
+
+    for (node = 0; node < nb_numa_nodes; node++) {
+        if (numa_info[node].is_initiator) {
+            initiator_pxm[num_initiator++] = node;
+        }
+        if (numa_info[node].is_target) {
+            target_pxm[num_target++] = node;
+        }
+    }
+}
+
+static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
+{
 /*
  * The Proximity Domain of System Physical Address ranges defined
  * in the HMAT, NFIT and SRAT tables shall match each other.
  */
-static void hmat_build_spa(GArray *table_data, PCMachineState *pcms)
-{
+
     GSList *device_list = NULL;
+    AcpiHmatLBInfo *hmat_lb;
+    AcpiHmatCacheInfo *hmat_cache;
+    struct numa_hmat_lb_info *numa_hmat_lb;
+    struct numa_hmat_cache_info *numa_hmat_cache;
     uint64_t mem_base, mem_len;
-    int i;
+    int i, j, hrchy, type, level;
 
     if (pcms->numa_nodes && !mem_ranges_number) {
         build_mem_ranges(pcms);
     }
 
     for (i = 0; i < mem_ranges_number; i++) {
-        hmat_build_spa_info(table_data, mem_ranges[i].base,
+        build_hmat_spa(hma, mem_ranges[i].base,
                             mem_ranges[i].length, mem_ranges[i].node);
     }
 
@@ -113,30 +132,10 @@ static void hmat_build_spa(GArray *table_data, PCMachineState *pcms)
         mem_len = object_property_get_uint(OBJECT(dimm), PC_DIMM_SIZE_PROP,
                                            NULL);
         i = object_property_get_uint(OBJECT(dimm), PC_DIMM_NODE_PROP, NULL);
-        hmat_build_spa_info(table_data, mem_base, mem_len, i);
-    }
-}
-
-static void classify_proximity_domains(void)
-{
-    int node;
-
-    for (node = 0; node < nb_numa_nodes; node++) {
-        if (numa_info[node].is_initiator) {
-            initiator_pxm[num_initiator++] = node;
-        }
-        if (numa_info[node].is_target) {
-            target_pxm[num_target++] = node;
-        }
+        build_hmat_spa(hma, mem_base, mem_len, i);
     }
-}
-
-static void hmat_build_lb(GArray *table_data)
-{
-    AcpiHmatLBInfo *hmat_lb;
-    struct numa_hmat_lb_info *numa_hmat_lb;
-    int i, j, hrchy, type;
 
+    /* Build HMAT System Locality Latency and Bandwidth Information. */
     if (!num_initiator && !num_target) {
         classify_proximity_domains();
     }
@@ -154,8 +153,8 @@ static void hmat_build_lb(GArray *table_data)
                 uint32_t size;
                 uint8_t m, n;
 
-                start = table_data->len;
-                hmat_lb = acpi_data_push(table_data, sizeof(*hmat_lb));
+                start = hma->len;
+                hmat_lb = acpi_data_push(hma, sizeof(*hmat_lb));
 
                 hmat_lb->type          = cpu_to_le16(ACPI_HMAT_LB_INFO);
                 hmat_lb->flags         = numa_hmat_lb->hierarchy;
@@ -174,19 +173,19 @@ static void hmat_build_lb(GArray *table_data)
 
                 /* the initiator proximity domain list */
                 for (i = 0; i < num_initiator; i++) {
-                    list_entry = acpi_data_push(table_data, sizeof(uint32_t));
+                    list_entry = acpi_data_push(hma, sizeof(uint32_t));
                     *list_entry = cpu_to_le32(initiator_pxm[i]);
                 }
 
                 /* the target proximity domain list */
                 for (i = 0; i < num_target; i++) {
-                    list_entry = acpi_data_push(table_data, sizeof(uint32_t));
+                    list_entry = acpi_data_push(hma, sizeof(uint32_t));
                     *list_entry = cpu_to_le32(target_pxm[i]);
                 }
 
                 /* latency or bandwidth entries */
                 size = sizeof(uint16_t) * num_initiator * num_target;
-                entry_start = acpi_data_push(table_data, size);
+                entry_start = acpi_data_push(hma, size);
 
                 for (i = 0; i < num_initiator; i++) {
                     m = initiator_pxm[i];
@@ -200,26 +199,20 @@ static void hmat_build_lb(GArray *table_data)
                         }
                     }
                 }
-                hmat_lb = (AcpiHmatLBInfo *)(table_data->data + start);
-                hmat_lb->length = cpu_to_le16(table_data->len - start);
+                hmat_lb = (AcpiHmatLBInfo *)(hma->data + start);
+                hmat_lb->length = cpu_to_le16(hma->len - start);
             }
         }
     }
-}
-
-static void hmat_build_cache(GArray *table_data)
-{
-    AcpiHmatCacheInfo *hmat_cache;
-    struct numa_hmat_cache_info *numa_hmat_cache;
-    int i, level;
 
+    /* Build HMAT Memory Side Cache Information. */
     for (i = 0; i < nb_numa_nodes; i++) {
         for (level = 0; level <= MAX_HMAT_CACHE_LEVEL; level++) {
             numa_hmat_cache = hmat_cache_info[i][level];
             if (numa_hmat_cache) {
-                uint64_t start = table_data->len;
+                uint64_t start = hma->len;
 
-                hmat_cache = acpi_data_push(table_data, sizeof(*hmat_cache));
+                hmat_cache = acpi_data_push(hma, sizeof(*hmat_cache));
                 hmat_cache->length = cpu_to_le32(sizeof(*hmat_cache));
                 hmat_cache->type = cpu_to_le16(ACPI_HMAT_CACHE_INFO);
                 hmat_cache->mem_proximity =
@@ -242,10 +235,10 @@ static void hmat_build_cache(GArray *table_data)
                     int size;
 
                     size = hmat_cache->num_smbios_handles * sizeof(uint16_t);
-                    smbios_handles = acpi_data_push(table_data, size);
+                    smbios_handles = acpi_data_push(hma, size);
 
                     hmat_cache = (AcpiHmatCacheInfo *)
-                                 (table_data->data + start);
+                                 (hma->data + start);
                     hmat_cache->length += size;
 
                     /* TBD: set smbios handles */
@@ -258,18 +251,6 @@ static void hmat_build_cache(GArray *table_data)
     }
 }
 
-static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
-{
-    /* Build HMAT Memory Subsystem Address Range. */
-    hmat_build_spa(hma, pcms);
-
-    /* Build HMAT System Locality Latency and Bandwidth Information. */
-    hmat_build_lb(hma);
-
-    /* Build HMAT Memory Side Cache Information. */
-    hmat_build_cache(hma);
-}
-
 static uint64_t
 hmat_hma_method_read(void *opaque, hwaddr addr, unsigned size)
 {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 9/9] acpi: rewrite the _FIT method to use it in _HMA method
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (7 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 8/9] hmat acpi: move some function inside of the caller Tao Xu
@ 2019-01-11 15:34 ` Tao Xu
  2019-01-13 19:07 ` [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) no-reply
  9 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-11 15:34 UTC (permalink / raw
  To: mst, imammedo
  Cc: marcel.apfelbaum, pbonzini, rth, ehabkost, jingqi.liu, danmei.wei,
	tao3.xu, qemu-devel

Per Igor's comment, rewrite NFIT code to build _HMA method.

We rewirte the function nvdimm_build_common_dsm(Aml *dev) and
nvdimm_build_fit(Aml *dev) in hw/acpi/nvdimm.c so that we can use
method_number as input to decide to generate _FIT or _HMA method.

Reviewed-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 hw/acpi/hmat.c          | 209 +--------------------
 hw/acpi/hmat.h          |   2 -
 hw/acpi/nvdimm.c        | 389 ++++++++++++++++++++++++++--------------
 hw/i386/acpi-build.c    |   2 +-
 include/hw/mem/nvdimm.h |  11 ++
 5 files changed, 270 insertions(+), 343 deletions(-)

diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index 4523e98ef1..37ece80101 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -100,10 +100,10 @@ static void classify_proximity_domains(void)
 
 static void hmat_build_hma(GArray *hma, PCMachineState *pcms)
 {
-/*
- * The Proximity Domain of System Physical Address ranges defined
- * in the HMAT, NFIT and SRAT tables shall match each other.
- */
+    /*
+     * The Proximity Domain of System Physical Address ranges defined
+     * in the HMAT, NFIT and SRAT tables shall match each other.
+     */
 
     GSList *device_list = NULL;
     AcpiHmatLBInfo *hmat_lb;
@@ -386,112 +386,6 @@ static void hmat_build_hma_buffer(PCMachineState *pcms)
     hma_buf->dirty = true;
 }
 
-static void hmat_build_common_aml(Aml *dev)
-{
-    Aml *method, *ifctx, *hmam_mem;
-    Aml *unsupport;
-    Aml *pckg, *pckg_index, *pckg_buf, *field;
-    Aml *hmam_out_buf, *hmam_out_buf_size;
-    uint8_t byte_list[1];
-
-    method = aml_method(HMA_COMMON_METHOD, 1, AML_SERIALIZED);
-    hmam_mem = aml_local(6);
-    hmam_out_buf = aml_local(7);
-
-    aml_append(method, aml_store(aml_name(HMAM_ACPI_MEM_ADDR), hmam_mem));
-
-    /* map _HMA memory and IO into ACPI namespace. */
-    aml_append(method, aml_operation_region(HMAM_IOPORT, AML_SYSTEM_IO,
-               aml_int(HMAM_ACPI_IO_BASE), HMAM_ACPI_IO_LEN));
-    aml_append(method, aml_operation_region(HMAM_MEMORY,
-               AML_SYSTEM_MEMORY, hmam_mem, HMAM_MEMORY_SIZE));
-
-    /*
-     * _HMAC notifier:
-     * HMAM_NOTIFY: write the address of DSM memory and notify QEMU to
-     *                    emulate the access.
-     *
-     * It is the IO port so that accessing them will cause VM-exit, the
-     * control will be transferred to QEMU.
-     */
-    field = aml_field(HMAM_IOPORT, AML_DWORD_ACC, AML_NOLOCK,
-                      AML_PRESERVE);
-    aml_append(field, aml_named_field(HMAM_NOTIFY,
-               sizeof(uint32_t) * BITS_PER_BYTE));
-    aml_append(method, field);
-
-    /*
-     * _HMAC input:
-     * HMAM_OFFSET: store the current offset of _HMA buffer.
-     *
-     * They are RAM mapping on host so that these accesses never cause VMExit.
-     */
-    field = aml_field(HMAM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
-                      AML_PRESERVE);
-    aml_append(field, aml_named_field(HMAM_OFFSET,
-               sizeof(typeof_field(HmatHmamIn, offset)) * BITS_PER_BYTE));
-    aml_append(method, field);
-
-    /*
-     * _HMAC output:
-     * HMAM_OUT_BUF_SIZE: the size of the buffer filled by QEMU.
-     * HMAM_OUT_BUF: the buffer QEMU uses to store the result.
-     *
-     * Since the page is reused by both input and out, the input data
-     * will be lost after storing new result into ODAT so we should fetch
-     * all the input data before writing the result.
-     */
-    field = aml_field(HMAM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
-                      AML_PRESERVE);
-    aml_append(field, aml_named_field(HMAM_OUT_BUF_SIZE,
-               sizeof(typeof_field(HmatHmamOut, len)) * BITS_PER_BYTE));
-    aml_append(field, aml_named_field(HMAM_OUT_BUF,
-       (sizeof(HmatHmamOut) - sizeof(uint32_t)) * BITS_PER_BYTE));
-    aml_append(method, field);
-
-    /*
-     * do not support any method if HMA memory address has not been
-     * patched.
-     */
-    unsupport = aml_if(aml_equal(hmam_mem, aml_int(0x0)));
-    byte_list[0] = HMAM_RET_STATUS_UNSUPPORT;
-    aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
-    aml_append(method, unsupport);
-
-    /* The parameter (Arg0) of _HMAC is a package which contains a buffer. */
-    pckg = aml_arg(0);
-    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
-                   aml_int(4 /* Package */)) /* It is a Package? */,
-                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element */,
-                   NULL));
-
-    pckg_index = aml_local(2);
-    pckg_buf = aml_local(3);
-    aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_index));
-    aml_append(ifctx, aml_store(aml_derefof(pckg_index), pckg_buf));
-    aml_append(ifctx, aml_store(pckg_buf, aml_name(HMAM_OFFSET)));
-    aml_append(method, ifctx);
-
-    /*
-     * tell QEMU about the real address of HMA memory, then QEMU
-     * gets the control and fills the result in _HMAC memory.
-     */
-    aml_append(method, aml_store(hmam_mem, aml_name(HMAM_NOTIFY)));
-
-    hmam_out_buf_size = aml_local(1);
-    /* RLEN is not included in the payload returned to guest. */
-    aml_append(method, aml_subtract(aml_name(HMAM_OUT_BUF_SIZE),
-                                aml_int(4), hmam_out_buf_size));
-    aml_append(method, aml_store(aml_shiftleft(hmam_out_buf_size, aml_int(3)),
-                                 hmam_out_buf_size));
-    aml_append(method, aml_create_field(aml_name(HMAM_OUT_BUF),
-                                aml_int(0), hmam_out_buf_size, "OBUF"));
-    aml_append(method, aml_concatenate(aml_buffer(0, NULL), aml_name("OBUF"),
-                                hmam_out_buf));
-    aml_append(method, aml_return(hmam_out_buf));
-    aml_append(dev, method);
-}
-
 void hmat_init_acpi_state(AcpiHmaState *state, MemoryRegion *io,
                           FWCfgState *fw_cfg, Object *owner)
 {
@@ -528,98 +422,3 @@ void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                  (void *)(table_data->data + hmat_start),
                  "HMAT", hmat_len, 1, NULL, NULL);
 }
-
-void hmat_build_aml(Aml *dev)
-{
-    Aml *method, *pkg, *buf, *buf_size, *offset, *call_result;
-    Aml *whilectx, *ifcond, *ifctx, *elsectx, *hma;
-
-    hmat_build_common_aml(dev);
-
-    buf = aml_local(0);
-    buf_size = aml_local(1);
-    hma = aml_local(2);
-
-    aml_append(dev, aml_name_decl(HMAM_RHMA_STATUS, aml_int(0)));
-
-    /* build helper function, RHMA. */
-    method = aml_method("RHMA", 1, AML_SERIALIZED);
-    aml_append(method, aml_name_decl("OFST", aml_int(0)));
-
-    /* prepare input package. */
-    pkg = aml_package(1);
-    aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
-    aml_append(pkg, aml_name("OFST"));
-
-    /* call Read HMA function. */
-    call_result = aml_call1(HMA_COMMON_METHOD, pkg);
-    aml_append(method, aml_store(call_result, buf));
-
-    /* handle _HMAC result. */
-    aml_append(method, aml_create_dword_field(buf,
-               aml_int(0) /* offset at byte 0 */, "STAU"));
-
-    aml_append(method, aml_store(aml_name("STAU"),
-                                 aml_name(HMAM_RHMA_STATUS)));
-
-    /* if something is wrong during _HMAC. */
-    ifcond = aml_equal(aml_int(HMAM_RET_STATUS_SUCCESS),
-                       aml_name("STAU"));
-    ifctx = aml_if(aml_lnot(ifcond));
-    aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
-    aml_append(method, ifctx);
-
-    aml_append(method, aml_store(aml_sizeof(buf), buf_size));
-    aml_append(method, aml_subtract(buf_size,
-                                    aml_int(4) /* the size of "STAU" */,
-                                    buf_size));
-
-    /* if we read the end of hma. */
-    ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
-    aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
-    aml_append(method, ifctx);
-
-    aml_append(method, aml_create_field(buf,
-                            aml_int(4 * BITS_PER_BYTE), /* offset at byte 4.*/
-                            aml_shiftleft(buf_size, aml_int(3)), "BUFF"));
-    aml_append(method, aml_return(aml_name("BUFF")));
-    aml_append(dev, method);
-
-    /* build _HMA. */
-    method = aml_method("_HMA", 0, AML_SERIALIZED);
-    offset = aml_local(3);
-
-    aml_append(method, aml_store(aml_buffer(0, NULL), hma));
-    aml_append(method, aml_store(aml_int(0), offset));
-
-    whilectx = aml_while(aml_int(1));
-    aml_append(whilectx, aml_store(aml_call1("RHMA", offset), buf));
-    aml_append(whilectx, aml_store(aml_sizeof(buf), buf_size));
-
-    /*
-     * if hma buffer was changed during RHMA, read from the beginning
-     * again.
-     */
-    ifctx = aml_if(aml_equal(aml_name(HMAM_RHMA_STATUS),
-                             aml_int(HMAM_RET_STATUS_HMA_CHANGED)));
-    aml_append(ifctx, aml_store(aml_buffer(0, NULL), hma));
-    aml_append(ifctx, aml_store(aml_int(0), offset));
-    aml_append(whilectx, ifctx);
-
-    elsectx = aml_else();
-
-    /* finish hma read if no data is read out. */
-    ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
-    aml_append(ifctx, aml_return(hma));
-    aml_append(elsectx, ifctx);
-
-    /* update the offset. */
-    aml_append(elsectx, aml_add(offset, buf_size, offset));
-    /* append the data we read out to the hma buffer. */
-    aml_append(elsectx, aml_concatenate(hma, buf, hma));
-    aml_append(whilectx, elsectx);
-    aml_append(method, whilectx);
-
-    aml_append(dev, method);
-}
-
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index 2c080a51b8..dd57678816 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -31,7 +31,6 @@
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/acpi/aml-build.h"
 
-#define ACPI_HMAT_SPA               0
 #define ACPI_HMAT_LB_INFO           1
 #define ACPI_HMAT_CACHE_INFO        2
 
@@ -238,7 +237,6 @@ extern struct numa_hmat_cache_info
 
 void hmat_build_acpi(GArray *table_data, BIOSLinker *linker,
                      MachineState *machine);
-void hmat_build_aml(Aml *dsdt);
 void hmat_init_acpi_state(AcpiHmaState *state, MemoryRegion *io,
                           FWCfgState *fw_cfg, Object *owner);
 void hmat_update(PCMachineState *pcms);
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e53b2cb681..795236bb1b 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -32,6 +32,7 @@
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/mem/nvdimm.h"
+#include "hw/acpi/hmat.h"
 
 static int nvdimm_device_list(Object *obj, void *opaque)
 {
@@ -959,26 +960,49 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 
 #define NVDIMM_QEMU_RSVD_UUID   "648B9CF2-CDA1-4312-8AD9-49C4AF32BD62"
 
-static void nvdimm_build_common_dsm(Aml *dev)
+static void nvdimm_build_common_dsm(Aml *dev, uint16_t method_number)
 {
-    Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *elsectx2;
+    Aml *method = NULL, *ifctx = NULL, *function = NULL;
+    Aml *handle = NULL, *uuid = NULL, *dsm_mem, *elsectx2;
     Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
-    Aml *pckg, *pckg_index, *pckg_buf, *field, *dsm_out_buf, *dsm_out_buf_size;
+    Aml *pckg = NULL, *pckg_index, *pckg_buf, *field;
+    Aml *dsm_out_buf, *dsm_out_buf_size;
     uint8_t byte_list[1];
+    uint16_t acpi_io_base = 0;
+    const char *acpi_mem_addr = NULL, *ioport = NULL, *memory = NULL;
+    const char *aml_offset = NULL;
+
+    switch (method_number) {
+    case 0: /* build common dsm in _FIT method */
+        acpi_mem_addr = NVDIMM_ACPI_MEM_ADDR;
+        ioport = NVDIMM_DSM_IOPORT;
+        acpi_io_base = NVDIMM_ACPI_IO_BASE;
+        memory = NVDIMM_DSM_MEMORY;
+        aml_offset = NVDIMM_DSM_ARG3;
+        method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
+        uuid = aml_arg(0);
+        function = aml_arg(2);
+        handle = aml_arg(4);
+        break;
+    case 1: /* build common dsm in _HMA method */
+        acpi_mem_addr = HMAM_ACPI_MEM_ADDR;
+        ioport = HMAM_IOPORT;
+        acpi_io_base = HMAM_ACPI_IO_BASE;
+        memory = HMAM_MEMORY;
+        aml_offset = HMAM_OFFSET;
+        method = aml_method(HMA_COMMON_METHOD, 1, AML_SERIALIZED);
+        break;
+    }
 
-    method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
-    uuid = aml_arg(0);
-    function = aml_arg(2);
-    handle = aml_arg(4);
     dsm_mem = aml_local(6);
     dsm_out_buf = aml_local(7);
 
-    aml_append(method, aml_store(aml_name(NVDIMM_ACPI_MEM_ADDR), dsm_mem));
+    aml_append(method, aml_store(aml_name("%s", acpi_mem_addr), dsm_mem));
 
     /* map DSM memory and IO into ACPI namespace. */
-    aml_append(method, aml_operation_region(NVDIMM_DSM_IOPORT, AML_SYSTEM_IO,
-               aml_int(NVDIMM_ACPI_IO_BASE), NVDIMM_ACPI_IO_LEN));
-    aml_append(method, aml_operation_region(NVDIMM_DSM_MEMORY,
+    aml_append(method, aml_operation_region(ioport, AML_SYSTEM_IO,
+               aml_int(acpi_io_base), NVDIMM_ACPI_IO_LEN));
+    aml_append(method, aml_operation_region(memory,
                AML_SYSTEM_MEMORY, dsm_mem, sizeof(NvdimmDsmIn)));
 
     /*
@@ -989,122 +1013,191 @@ static void nvdimm_build_common_dsm(Aml *dev)
      * It is the IO port so that accessing them will cause VM-exit, the
      * control will be transferred to QEMU.
      */
-    field = aml_field(NVDIMM_DSM_IOPORT, AML_DWORD_ACC, AML_NOLOCK,
+    field = aml_field(ioport, AML_DWORD_ACC, AML_NOLOCK,
                       AML_PRESERVE);
     aml_append(field, aml_named_field(NVDIMM_DSM_NOTIFY,
                sizeof(uint32_t) * BITS_PER_BYTE));
     aml_append(method, field);
 
-    /*
-     * DSM input:
-     * NVDIMM_DSM_HANDLE: store device's handle, it's zero if the _DSM call
-     *                    happens on NVDIMM Root Device.
-     * NVDIMM_DSM_REVISION: store the Arg1 of _DSM call.
-     * NVDIMM_DSM_FUNCTION: store the Arg2 of _DSM call.
-     * NVDIMM_DSM_ARG3: store the Arg3 of _DSM call which is a Package
-     *                  containing function-specific arguments.
-     *
-     * They are RAM mapping on host so that these accesses never cause
-     * VM-EXIT.
-     */
-    field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
-                      AML_PRESERVE);
-    aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
-               sizeof(typeof_field(NvdimmDsmIn, handle)) * BITS_PER_BYTE));
-    aml_append(field, aml_named_field(NVDIMM_DSM_REVISION,
-               sizeof(typeof_field(NvdimmDsmIn, revision)) * BITS_PER_BYTE));
-    aml_append(field, aml_named_field(NVDIMM_DSM_FUNCTION,
-               sizeof(typeof_field(NvdimmDsmIn, function)) * BITS_PER_BYTE));
-    aml_append(field, aml_named_field(NVDIMM_DSM_ARG3,
-         (sizeof(NvdimmDsmIn) - offsetof(NvdimmDsmIn, arg3)) * BITS_PER_BYTE));
-    aml_append(method, field);
+    field = aml_field(memory, AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
+    switch (method_number) {
+    case 0: /* build common dsm in _FIT method */
+        /*
+         * DSM input:
+         * NVDIMM_DSM_HANDLE: store device's handle, it's zero if
+         *                    the _DSM call happens on NVDIMM Root
+         *                    Device.
+         * NVDIMM_DSM_REVISION: store the Arg1 of _DSM call.
+         * NVDIMM_DSM_FUNCTION: store the Arg2 of _DSM call.
+         * NVDIMM_DSM_ARG3: store the Arg3 of _DSM call which is a
+         *                  Package containing function-specific
+         *                  arguments.
+         *
+         * They are RAM mapping on host so that these accesses
+         * never cause VM-EXIT.
+         */
+        aml_append(field, aml_named_field(NVDIMM_DSM_HANDLE,
+                    sizeof(typeof_field(NvdimmDsmIn, handle)) *
+                    BITS_PER_BYTE));
+        aml_append(field, aml_named_field(NVDIMM_DSM_REVISION,
+                    sizeof(typeof_field(NvdimmDsmIn, revision)) *
+                    BITS_PER_BYTE));
+        aml_append(field, aml_named_field(NVDIMM_DSM_FUNCTION,
+                    sizeof(typeof_field(NvdimmDsmIn, function)) *
+                    BITS_PER_BYTE));
+        aml_append(field, aml_named_field(NVDIMM_DSM_ARG3,
+                    (sizeof(NvdimmDsmIn) - offsetof(NvdimmDsmIn, arg3)) *
+                    BITS_PER_BYTE));
+        aml_append(method, field);
 
-    /*
-     * DSM output:
-     * NVDIMM_DSM_OUT_BUF_SIZE: the size of the buffer filled by QEMU.
-     * NVDIMM_DSM_OUT_BUF: the buffer QEMU uses to store the result.
-     *
-     * Since the page is reused by both input and out, the input data
-     * will be lost after storing new result into ODAT so we should fetch
-     * all the input data before writing the result.
-     */
-    field = aml_field(NVDIMM_DSM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
-                      AML_PRESERVE);
-    aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF_SIZE,
-               sizeof(typeof_field(NvdimmDsmOut, len)) * BITS_PER_BYTE));
-    aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF,
-       (sizeof(NvdimmDsmOut) - offsetof(NvdimmDsmOut, data)) * BITS_PER_BYTE));
-    aml_append(method, field);
+        /*
+         * DSM output:
+         * NVDIMM_DSM_OUT_BUF_SIZE: the size of the buffer
+         *                          filled by QEMU.
+         * NVDIMM_DSM_OUT_BUF: the buffer QEMU uses to
+         *                     store the result.
+         *
+         * Since the page is reused by both input and out,
+         * the input data will be lost after storing new
+         * result into ODAT so we should fetch all the input
+         * data before writing the result.
+         */
+        field = aml_field(memory, AML_DWORD_ACC, AML_NOLOCK,
+                        AML_PRESERVE);
+        aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF_SIZE,
+                sizeof(typeof_field(NvdimmDsmOut, len)) *
+                BITS_PER_BYTE));
+        aml_append(field, aml_named_field(NVDIMM_DSM_OUT_BUF,
+                (sizeof(NvdimmDsmOut) - offsetof(NvdimmDsmOut, data)) *
+                BITS_PER_BYTE));
+        aml_append(method, field);
 
-    /*
-     * do not support any method if DSM memory address has not been
-     * patched.
-     */
-    unpatched = aml_equal(dsm_mem, aml_int(0x0));
+        /*
+         * do not support any method if DSM memory address has not been
+         * patched.
+         */
+        unpatched = aml_equal(dsm_mem, aml_int(0x0));
+
+        expected_uuid = aml_local(0);
+
+        ifctx = aml_if(aml_equal(handle, aml_int(0x0)));
+        aml_append(ifctx, aml_store(
+                aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA")
+                /* UUID for NVDIMM Root Device */, expected_uuid));
+        aml_append(method, ifctx);
+        elsectx = aml_else();
+        ifctx = aml_if(aml_equal(handle,
+                                aml_int(NVDIMM_QEMU_RSVD_HANDLE_ROOT)));
+        aml_append(ifctx, aml_store(aml_touuid(NVDIMM_QEMU_RSVD_UUID
+                /* UUID for QEMU internal use */), expected_uuid));
+        aml_append(elsectx, ifctx);
+        elsectx2 = aml_else();
+        aml_append(elsectx2, aml_store(
+                aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
+                /* UUID for NVDIMM Devices */, expected_uuid));
+        aml_append(elsectx, elsectx2);
+        aml_append(method, elsectx);
+
+        uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
+
+        unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
 
-    expected_uuid = aml_local(0);
+        /*
+         * function 0 is called to inquire what functions are supported by
+         * OSPM
+         */
+        ifctx = aml_if(aml_equal(function, aml_int(0)));
+        byte_list[0] = 0 /* No function Supported */;
+        aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
+        aml_append(unsupport, ifctx);
 
-    ifctx = aml_if(aml_equal(handle, aml_int(0x0)));
-    aml_append(ifctx, aml_store(
-               aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA")
-               /* UUID for NVDIMM Root Device */, expected_uuid));
-    aml_append(method, ifctx);
-    elsectx = aml_else();
-    ifctx = aml_if(aml_equal(handle, aml_int(NVDIMM_QEMU_RSVD_HANDLE_ROOT)));
-    aml_append(ifctx, aml_store(aml_touuid(NVDIMM_QEMU_RSVD_UUID
-               /* UUID for QEMU internal use */), expected_uuid));
-    aml_append(elsectx, ifctx);
-    elsectx2 = aml_else();
-    aml_append(elsectx2, aml_store(
-               aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
-               /* UUID for NVDIMM Devices */, expected_uuid));
-    aml_append(elsectx, elsectx2);
-    aml_append(method, elsectx);
+        /* No function is supported yet. */
+        byte_list[0] = NVDIMM_DSM_RET_STATUS_UNSUPPORT;
+        aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
+        aml_append(method, unsupport);
+
+        /*
+         * The HDLE indicates the DSM function is issued from which device,
+         * it reserves 0 for root device and is the handle for
+         * NVDIMM devices. See the comments in nvdimm_slot_to_handle().
+         */
+        aml_append(method, aml_store(handle,
+                aml_name(NVDIMM_DSM_HANDLE)));
+        aml_append(method, aml_store(aml_arg(1),
+                aml_name(NVDIMM_DSM_REVISION)));
+        aml_append(method, aml_store(aml_arg(2),
+                aml_name(NVDIMM_DSM_FUNCTION)));
 
-    uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
+        /*
+         * The fourth parameter (Arg3) of _DSM is a package which contains
+         * a buffer, the layout of the buffer is specified by UUID (Arg0),
+         * Revision ID (Arg1) and Function Index (Arg2) which are documented
+         * in the DSM Spec.
+         */
+        pckg = aml_arg(3);
+        ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+                    aml_int(4 /* Package */)) /* It is a Package? */,
+                    aml_equal(aml_sizeof(pckg),
+                                aml_int(1)) /* 1 element? */,
+                                NULL));
 
-    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
+        break;
+    case 1: /* build common dsm in _HMA method */
+        /*
+         * _HMAC input:
+         * HMAM_OFFSET: store the current offset of _HMA buffer.
+         *
+         * They are RAM mapping on host so that
+         * these accesses never cause VMExit.
+         */
+        aml_append(field, aml_named_field(HMAM_OFFSET,
+                sizeof(typeof_field(HmatHmamIn, offset)) * BITS_PER_BYTE));
+        aml_append(method, field);
 
-    /*
-     * function 0 is called to inquire what functions are supported by
-     * OSPM
-     */
-    ifctx = aml_if(aml_equal(function, aml_int(0)));
-    byte_list[0] = 0 /* No function Supported */;
-    aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
-    aml_append(unsupport, ifctx);
+        /*
+         * _HMAC output:
+         * HMAM_OUT_BUF_SIZE: the size of the buffer filled by QEMU.
+         * HMAM_OUT_BUF: the buffer QEMU uses to store the result.
+         *
+         * Since the page is reused by both input and out, the input data
+         * will be lost after storing new result into ODAT so we should
+         * fetch all the input data before writing the result.
+         */
+        field = aml_field(HMAM_MEMORY, AML_DWORD_ACC, AML_NOLOCK,
+                        AML_PRESERVE);
+        aml_append(field, aml_named_field(HMAM_OUT_BUF_SIZE,
+                sizeof(typeof_field(HmatHmamOut, len)) * BITS_PER_BYTE));
+        aml_append(field, aml_named_field(HMAM_OUT_BUF,
+                (sizeof(HmatHmamOut) - sizeof(uint32_t)) * BITS_PER_BYTE));
+        aml_append(method, field);
 
-    /* No function is supported yet. */
-    byte_list[0] = NVDIMM_DSM_RET_STATUS_UNSUPPORT;
-    aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
-    aml_append(method, unsupport);
+        /*
+         * do not support any method if HMA memory address has not been
+         * patched.
+         */
+        unsupport = aml_if(aml_equal(dsm_mem, aml_int(0x0)));
+        byte_list[0] = HMAM_RET_STATUS_UNSUPPORT;
+        aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
+        aml_append(method, unsupport);
 
-    /*
-     * The HDLE indicates the DSM function is issued from which device,
-     * it reserves 0 for root device and is the handle for NVDIMM devices.
-     * See the comments in nvdimm_slot_to_handle().
-     */
-    aml_append(method, aml_store(handle, aml_name(NVDIMM_DSM_HANDLE)));
-    aml_append(method, aml_store(aml_arg(1), aml_name(NVDIMM_DSM_REVISION)));
-    aml_append(method, aml_store(aml_arg(2), aml_name(NVDIMM_DSM_FUNCTION)));
+        /*
+         * The parameter (Arg0) of _HMAC is
+         * a package which contains a buffer.
+         */
+        pckg = aml_arg(0);
+        ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+                    aml_int(4 /* Package */)) /* It is a Package? */,
+                    aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element */,
+                    NULL));
 
-    /*
-     * The fourth parameter (Arg3) of _DSM is a package which contains
-     * a buffer, the layout of the buffer is specified by UUID (Arg0),
-     * Revision ID (Arg1) and Function Index (Arg2) which are documented
-     * in the DSM Spec.
-     */
-    pckg = aml_arg(3);
-    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
-                   aml_int(4 /* Package */)) /* It is a Package? */,
-                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
-                   NULL));
+        break;
+    }
 
     pckg_index = aml_local(2);
     pckg_buf = aml_local(3);
     aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_index));
     aml_append(ifctx, aml_store(aml_derefof(pckg_index), pckg_buf));
-    aml_append(ifctx, aml_store(pckg_buf, aml_name(NVDIMM_DSM_ARG3)));
+    aml_append(ifctx, aml_store(pckg_buf, aml_name("%s", aml_offset)));
     aml_append(method, ifctx);
 
     /*
@@ -1138,19 +1231,37 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle)
     aml_append(dev, method);
 }
 
-static void nvdimm_build_fit(Aml *dev)
+void nvdimm_build_fit(Aml *dev, uint16_t method_number)
 {
-    Aml *method, *pkg, *buf, *buf_size, *offset, *call_result;
-    Aml *whilectx, *ifcond, *ifctx, *elsectx, *fit;
+    Aml *method, *pkg, *buf, *buf_size, *offset, *call_result = NULL;
+    Aml *whilectx, *ifcond, *ifctx, *elsectx, *buf_name;
+    const char *help_function = NULL, *method_name = NULL;
+    int ret_status_success, ret_status_changed;
+
+    switch (method_number) {
+    case 0: /* _FIT method */
+        method_name = "_FIT";
+        help_function = "RFIT";
+        ret_status_success = NVDIMM_DSM_RET_STATUS_SUCCESS;
+        ret_status_changed = NVDIMM_DSM_RET_STATUS_FIT_CHANGED;
+        break;
+    case 1: /* _HMA method */
+        method_name = "_HMA";
+        nvdimm_build_common_dsm(dev, METHOD_NAME_HMA);
+        help_function = "RHMA";
+        ret_status_success = HMAM_RET_STATUS_SUCCESS;
+        ret_status_changed = HMAM_RET_STATUS_HMA_CHANGED;
+        break;
+    }
 
     buf = aml_local(0);
     buf_size = aml_local(1);
-    fit = aml_local(2);
+    buf_name = aml_local(2);
 
     aml_append(dev, aml_name_decl(NVDIMM_DSM_RFIT_STATUS, aml_int(0)));
 
-    /* build helper function, RFIT. */
-    method = aml_method("RFIT", 1, AML_SERIALIZED);
+    /* build helper function. */
+    method = aml_method(help_function, 1, AML_SERIALIZED);
     aml_append(method, aml_name_decl("OFST", aml_int(0)));
 
     /* prepare input package. */
@@ -1158,12 +1269,20 @@ static void nvdimm_build_fit(Aml *dev)
     aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
     aml_append(pkg, aml_name("OFST"));
 
-    /* call Read_FIT function. */
-    call_result = aml_call5(NVDIMM_COMMON_DSM,
-                            aml_touuid(NVDIMM_QEMU_RSVD_UUID),
-                            aml_int(1) /* Revision 1 */,
-                            aml_int(0x1) /* Read FIT */,
-                            pkg, aml_int(NVDIMM_QEMU_RSVD_HANDLE_ROOT));
+    /* call Read function. */
+    switch (method_number) {
+    case 0: /* build common dsm in _FIT method */
+        call_result = aml_call5(NVDIMM_COMMON_DSM,
+                                aml_touuid(NVDIMM_QEMU_RSVD_UUID),
+                                aml_int(1) /* Revision 1 */,
+                                aml_int(0x1) /* Read FIT */,
+                                pkg, aml_int(NVDIMM_QEMU_RSVD_HANDLE_ROOT));
+        break;
+    case 1: /* build common dsm in _FIT method */
+        call_result = aml_call1(HMA_COMMON_METHOD, pkg);
+        break;
+    }
+
     aml_append(method, aml_store(call_result, buf));
 
     /* handle _DSM result. */
@@ -1174,7 +1293,7 @@ static void nvdimm_build_fit(Aml *dev)
                                  aml_name(NVDIMM_DSM_RFIT_STATUS)));
 
      /* if something is wrong during _DSM. */
-    ifcond = aml_equal(aml_int(NVDIMM_DSM_RET_STATUS_SUCCESS),
+    ifcond = aml_equal(aml_int(ret_status_success),
                        aml_name("STAU"));
     ifctx = aml_if(aml_lnot(ifcond));
     aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
@@ -1185,7 +1304,7 @@ static void nvdimm_build_fit(Aml *dev)
                                     aml_int(4) /* the size of "STAU" */,
                                     buf_size));
 
-    /* if we read the end of fit. */
+    /* if we read the end of fit or hma. */
     ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
     aml_append(ifctx, aml_return(aml_buffer(0, NULL)));
     aml_append(method, ifctx);
@@ -1196,38 +1315,38 @@ static void nvdimm_build_fit(Aml *dev)
     aml_append(method, aml_return(aml_name("BUFF")));
     aml_append(dev, method);
 
-    /* build _FIT. */
-    method = aml_method("_FIT", 0, AML_SERIALIZED);
+    /* build _FIT or _HMA. */
+    method = aml_method(method_name, 0, AML_SERIALIZED);
     offset = aml_local(3);
 
-    aml_append(method, aml_store(aml_buffer(0, NULL), fit));
+    aml_append(method, aml_store(aml_buffer(0, NULL), buf_name));
     aml_append(method, aml_store(aml_int(0), offset));
 
     whilectx = aml_while(aml_int(1));
-    aml_append(whilectx, aml_store(aml_call1("RFIT", offset), buf));
+    aml_append(whilectx, aml_store(aml_call1(help_function, offset), buf));
     aml_append(whilectx, aml_store(aml_sizeof(buf), buf_size));
 
     /*
-     * if fit buffer was changed during RFIT, read from the beginning
-     * again.
+     * if buffer was changed during RFIT or RHMA,
+     * read from the beginning again.
      */
     ifctx = aml_if(aml_equal(aml_name(NVDIMM_DSM_RFIT_STATUS),
-                             aml_int(NVDIMM_DSM_RET_STATUS_FIT_CHANGED)));
-    aml_append(ifctx, aml_store(aml_buffer(0, NULL), fit));
+                             aml_int(ret_status_changed)));
+    aml_append(ifctx, aml_store(aml_buffer(0, NULL), buf_name));
     aml_append(ifctx, aml_store(aml_int(0), offset));
     aml_append(whilectx, ifctx);
 
     elsectx = aml_else();
 
-    /* finish fit read if no data is read out. */
+    /* finish fit or hma read if no data is read out. */
     ifctx = aml_if(aml_equal(buf_size, aml_int(0)));
-    aml_append(ifctx, aml_return(fit));
+    aml_append(ifctx, aml_return(buf_name));
     aml_append(elsectx, ifctx);
 
     /* update the offset. */
     aml_append(elsectx, aml_add(offset, buf_size, offset));
-    /* append the data we read out to the fit buffer. */
-    aml_append(elsectx, aml_concatenate(fit, buf, fit));
+    /* append the data we read out to the fit or hma buffer. */
+    aml_append(elsectx, aml_concatenate(buf_name, buf, buf_name));
     aml_append(whilectx, elsectx);
     aml_append(method, whilectx);
 
@@ -1288,11 +1407,11 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
      */
     aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012")));
 
-    nvdimm_build_common_dsm(dev);
+    nvdimm_build_common_dsm(dev, METHOD_NAME_FIT);
 
     /* 0 is reserved for root device. */
     nvdimm_build_device_dsm(dev, 0);
-    nvdimm_build_fit(dev);
+    nvdimm_build_fit(dev, METHOD_NAME_FIT);
 
     nvdimm_build_nvdimm_devices(dev, ram_slots);
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 729e67e829..3e014b1ead 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1846,7 +1846,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         build_q35_pci0_int(dsdt);
     }
 
-    hmat_build_aml(dsdt);
+    nvdimm_build_fit(dsdt, METHOD_NAME_HMA);
 
     if (pcmc->legacy_cpu_hotplug) {
         build_legacy_cpu_hotplug_aml(dsdt, machine, pm->cpu_hp_io_base);
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index c5c9b3c7f8..3bc38735e4 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -25,6 +25,7 @@
 
 #include "hw/mem/pc-dimm.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/aml-build.h"
 
 #define NVDIMM_DEBUG 0
 #define nvdimm_debug(fmt, ...)                                \
@@ -110,6 +111,15 @@ typedef struct NVDIMMClass NVDIMMClass;
 #define NVDIMM_ACPI_IO_BASE     0x0a18
 #define NVDIMM_ACPI_IO_LEN      4
 
+/*
+ * The ACPI Device Configuration method name used in nvdimm_build_fit,
+ * use number to representative the name:
+ * 0 means "_FIT"
+ * 1 means "_HMA"
+ */
+#define METHOD_NAME_FIT     0
+#define METHOD_NAME_HMA     1
+
 /*
  * NvdimmFitBuffer:
  * @fit: FIT structures for present NVDIMMs. It is updated when
@@ -150,4 +160,5 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
                        uint32_t ram_slots);
 void nvdimm_plug(AcpiNVDIMMState *state);
 void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev);
+void nvdimm_build_fit(Aml *dev, uint16_t method_number);
 #endif
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT)
  2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
                   ` (8 preceding siblings ...)
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 9/9] acpi: rewrite the _FIT method to use it in _HMA method Tao Xu
@ 2019-01-13 19:07 ` no-reply
  9 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2019-01-13 19:07 UTC (permalink / raw
  To: tao3.xu
  Cc: fam, mst, imammedo, ehabkost, jingqi.liu, qemu-devel, pbonzini,
	danmei.wei, rth

Patchew URL: https://patchew.org/QEMU/20190111153451.14304-1-tao3.xu@intel.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14
=== TEST SCRIPT END ===

  LINK    aarch64-softmmu/qemu-system-aarch64w.exe
numa.o:numa.c:(.rdata$.refptr.hmat_cache_info[.refptr.hmat_cache_info]+0x0): undefined reference to `hmat_cache_info'
numa.o:numa.c:(.rdata$.refptr.hmat_lb_info[.refptr.hmat_lb_info]+0x0): undefined reference to `hmat_lb_info'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:196: qemu-system-aarch64w.exe] Error 1
make: *** [Makefile:428: subdir-aarch64-softmmu] Error 2
Traceback (most recent call last):


The full log is available at
http://patchew.org/logs/20190111153451.14304-1-tao3.xu@intel.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information Tao Xu
@ 2019-01-14 19:38   ` Eric Blake
  2019-01-21  6:03     ` Tao Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2019-01-14 19:38 UTC (permalink / raw
  To: Tao Xu, mst, imammedo
  Cc: ehabkost, jingqi.liu, qemu-devel, pbonzini, danmei.wei, rth

[-- Attachment #1: Type: text/plain, Size: 3858 bytes --]

On 1/11/19 9:34 AM, Tao Xu wrote:
> From: Liu Jingqi <jingqi.liu@intel.com>
> 
> Add -numa hmat-lb option to provide System Locality Latency and
> Bandwidth Information. These memory attributes help to build
> System Locality Latency and Bandwidth Information Structure(s)
> in ACPI Heterogeneous Memory Attribute Table (HMAT).
> 
> Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---
>  numa.c          | 124 ++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/misc.json  |  92 ++++++++++++++++++++++++++++++++++-
>  qemu-options.hx |  28 ++++++++++-
>  3 files changed, 241 insertions(+), 3 deletions(-)

> +++ b/qapi/misc.json
> @@ -2746,10 +2746,12 @@
>  #
>  # @cpu: property based CPU(s) to node mapping (Since: 2.10)
>  #
> +# @hmat-lb: memory latency and bandwidth information (Since: 2.13)

s/2.13/4.0/ (probably in multiple spots in your series)


> +##
> +# @HmatLBMemoryHierarchy:
> +#
> +# The memory hierarchy in the System Locality Latency
> +# and Bandwidth Information Structure of HMAT
> +#
> +# @memory: the structure represents the memory performance
> +#
> +# @last-level: last level memory of memory side cached memory
> +#
> +# @1st-level: first level memory of memory side cached memory
> +#
> +# @2nd-level: second level memory of memory side cached memory
> +#
> +# @3rd-level: third level memory of memory side cached memory

Let's spell these first-level, second-level, third-level (rather than
adding even more spots where we have enums with leading digits)

> +#
> +# Since: 2.13
> +##
> +{ 'enum': 'HmatLBMemoryHierarchy',
> +  'data': [ 'memory', 'last-level', '1st-level',
> +            '2nd-level', '3rd-level' ] }
> +
> +##
> +# @HmatLBDataType:
> +#
> +# Data type in the System Locality Latency
> +# and Bandwidth Information Structure of HMAT
> +#
> +# @access-latency: access latency
> +#
> +# @read-latency: read latency
> +#
> +# @write-latency: write latency
> +#
> +# @access-bandwidth: access bandwitch
> +#

s/bandwitch/bandwidth/

> +# @read-bandwidth: read bandwidth
> +#
> +# @write-bandwidth: write bandwidth

All 6 of these should probably list their units.

> +#
> +# Since: 2.13
> +##
> +{ 'enum': 'HmatLBDataType',
> +  'data': [ 'access-latency', 'read-latency', 'write-latency',
> +            'access-bandwidth', 'read-bandwidth', 'write-bandwidth' ] }
> +
> +##
> +# @NumaHmatLBOptions:
> +#
> +# Set the system locality latency and bandwidth information
> +# between Initiator and Target proximity Domains.
> +#
> +# @initiator: the Initiator Proximity Domain.
> +#
> +# @target: the Target Proximity Domain.
> +#
> +# @hierarchy: the Memory Hierarchy. Indicates the performance
> +#             of memory or side cache.
> +#
> +# @data-type: presents the type of data, access/read/write
> +#             latency or hit latency.
> +#
> +# @base-lat: the base unit for latency in nanoseconds.
> +#
> +# @base-bw: the base unit for bandwidth in megabytes per second(MB/s).
> +#
> +# @latency: the value of latency based on Base Unit from @initiator
> +#           to @target proximity domain.
> +#
> +# @bandwidth: the value of bandwidth based on Base Unit between
> +#             @initiator and @target proximity domain.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'NumaHmatLBOptions',
> +  'data': {
> +   'initiator': 'uint16',
> +   'target': 'uint16',
> +   'hierarchy': 'HmatLBMemoryHierarchy',
> +   'data-type': 'HmatLBDataType',
> +   '*base-lat': 'uint64',
> +   '*base-bw': 'uint64',
> +   '*latency': 'uint16',
> +   '*bandwidth': 'uint16' }}
> +
>  ##
>  # @HostMemPolicy:
>  #


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information
  2019-01-14 19:38   ` Eric Blake
@ 2019-01-21  6:03     ` Tao Xu
  2019-01-21 17:09       ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Tao Xu @ 2019-01-21  6:03 UTC (permalink / raw
  To: Eric Blake, mst, imammedo
  Cc: ehabkost, jingqi.liu, qemu-devel, pbonzini, danmei.wei, rth


On 1/15/2019 3:38 AM, Eric Blake wrote:
> On 1/11/19 9:34 AM, Tao Xu wrote:
>> From: Liu Jingqi <jingqi.liu@intel.com>
>>
>> Add -numa hmat-lb option to provide System Locality Latency and
>> Bandwidth Information. These memory attributes help to build
>> System Locality Latency and Bandwidth Information Structure(s)
>> in ACPI Heterogeneous Memory Attribute Table (HMAT).
>>
>> Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
>> Signed-off-by: Tao Xu <tao3.xu@intel.com>
>> ---
>>   numa.c          | 124 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   qapi/misc.json  |  92 ++++++++++++++++++++++++++++++++++-
>>   qemu-options.hx |  28 ++++++++++-
>>   3 files changed, 241 insertions(+), 3 deletions(-)
>> +++ b/qapi/misc.json
>> @@ -2746,10 +2746,12 @@
>>   #
>>   # @cpu: property based CPU(s) to node mapping (Since: 2.10)
>>   #
>> +# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
> s/2.13/4.0/ (probably in multiple spots in your series) 
Hi Eric,

Thank you for your comments. The spell mistakes in patches 1/9 to 6/9
have been corrected in patch 7/9. Because patches 1/9 to 6/9 are jingqi's
initial V1 patchesand7/9 to 9/9 are the changes compared withV1.

About s/2.13/4.0/,do you mean ACPI HMAT will not be merged before QEMU
4.0?

In addition, do you have any other comments about these patches?
Thank you very much!

Tao
>> +##
>> +# @HmatLBMemoryHierarchy:
>> +#
>> +# The memory hierarchy in the System Locality Latency
>> +# and Bandwidth Information Structure of HMAT
>> +#
>> +# @memory: the structure represents the memory performance
>> +#
>> +# @last-level: last level memory of memory side cached memory
>> +#
>> +# @1st-level: first level memory of memory side cached memory
>> +#
>> +# @2nd-level: second level memory of memory side cached memory
>> +#
>> +# @3rd-level: third level memory of memory side cached memory
> Let's spell these first-level, second-level, third-level (rather than
> adding even more spots where we have enums with leading digits)
>
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum': 'HmatLBMemoryHierarchy',
>> +  'data': [ 'memory', 'last-level', '1st-level',
>> +            '2nd-level', '3rd-level' ] }
>> +
>> +##
>> +# @HmatLBDataType:
>> +#
>> +# Data type in the System Locality Latency
>> +# and Bandwidth Information Structure of HMAT
>> +#
>> +# @access-latency: access latency
>> +#
>> +# @read-latency: read latency
>> +#
>> +# @write-latency: write latency
>> +#
>> +# @access-bandwidth: access bandwitch
>> +#
> s/bandwitch/bandwidth/
>
>> +# @read-bandwidth: read bandwidth
>> +#
>> +# @write-bandwidth: write bandwidth
> All 6 of these should probably list their units.
>
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum': 'HmatLBDataType',
>> +  'data': [ 'access-latency', 'read-latency', 'write-latency',
>> +            'access-bandwidth', 'read-bandwidth', 'write-bandwidth' ] }
>> +
>> +##
>> +# @NumaHmatLBOptions:
>> +#
>> +# Set the system locality latency and bandwidth information
>> +# between Initiator and Target proximity Domains.
>> +#
>> +# @initiator: the Initiator Proximity Domain.
>> +#
>> +# @target: the Target Proximity Domain.
>> +#
>> +# @hierarchy: the Memory Hierarchy. Indicates the performance
>> +#             of memory or side cache.
>> +#
>> +# @data-type: presents the type of data, access/read/write
>> +#             latency or hit latency.
>> +#
>> +# @base-lat: the base unit for latency in nanoseconds.
>> +#
>> +# @base-bw: the base unit for bandwidth in megabytes per second(MB/s).
>> +#
>> +# @latency: the value of latency based on Base Unit from @initiator
>> +#           to @target proximity domain.
>> +#
>> +# @bandwidth: the value of bandwidth based on Base Unit between
>> +#             @initiator and @target proximity domain.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct': 'NumaHmatLBOptions',
>> +  'data': {
>> +   'initiator': 'uint16',
>> +   'target': 'uint16',
>> +   'hierarchy': 'HmatLBMemoryHierarchy',
>> +   'data-type': 'HmatLBDataType',
>> +   '*base-lat': 'uint64',
>> +   '*base-bw': 'uint64',
>> +   '*latency': 'uint16',
>> +   '*bandwidth': 'uint16' }}
>> +
>>   ##
>>   # @HostMemPolicy:
>>   #
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information
  2019-01-21  6:03     ` Tao Xu
@ 2019-01-21 17:09       ` Eric Blake
  2019-01-28  9:42         ` Tao Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2019-01-21 17:09 UTC (permalink / raw
  To: Tao Xu, mst, imammedo
  Cc: ehabkost, jingqi.liu, qemu-devel, pbonzini, danmei.wei, rth

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

On 1/21/19 12:03 AM, Tao Xu wrote:

>>>   #
>>> +# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
>> s/2.13/4.0/ (probably in multiple spots in your series) 
> Hi Eric,
> 
> Thank you for your comments. The spell mistakes in patches 1/9 to 6/9
> have been corrected in patch 7/9. Because patches 1/9 to 6/9 are jingqi's
> initial V1 patchesand7/9 to 9/9 are the changes compared withV1.

Still, it's better to rebase the series to avoid the mistakes in the
first place, instead of having churn with a mistake early in the series
corrected only later in the series.  Even if the mistake is corrected by
a different author than the bulk of the patch, as long as there are
Signed-off-by lines for both the original author and the person making
the spelling correction, then proper attribution has been made.

> 
> About s/2.13/4.0/,do you mean ACPI HMAT will not be merged before QEMU
> 4.0?

Correct - 4.0 is the next release planned.  There was no 2.13 release;
after 2.12 was the 3.0 release; the current release is 3.1, and the next
release is 4.0; for more details, see
https://www.qemu.org/2018/08/15/qemu-3-0-0/ for a description of the
current version numbering scheme.  And even making it in time for 4.0
means polishing this by mid-March: https://wiki.qemu.org/Planning/4.0

> 
> In addition, do you have any other comments about these patches?

I was commenting on the high-level user interface issues, but will leave
the bulk of the technical review to those more familiar with the code
being added.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues
  2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues Tao Xu
@ 2019-01-21 17:11   ` Eric Blake
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2019-01-21 17:11 UTC (permalink / raw
  To: Tao Xu, mst, imammedo
  Cc: ehabkost, jingqi.liu, qemu-devel, pbonzini, danmei.wei, rth

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On 1/11/19 9:34 AM, Tao Xu wrote:
> Per Igor and Eric's comments, fix some small issues of V1 patch:
>   - update the version number in qapi/misc.json
>   - including the expansion of the acronym HMAT in qapi/misc.json
>   - correct spell mistakes in qapi/misc.json and qemu-options.hx
>   - fix the comment syle in hw/i386/acpi-build.c
>   and hw/acpi/hmat.h
>   - remove some unnecessary head files in hw/acpi/hmat.c
>   - use hardcoded numbers from spec to generate
>   Memory Subsystem Address Range Structure in hw/acpi/hmat.c
>   - drop the struct AcpiHmat and AcpiHmatSpaRange
>   in hw/acpi/hmat.h

These fixes should be rebased into the patches that introduced the
problem, rather than being a followup patch (at least, as long as the
original patches have not been merged into mainline yet).

> 
> Reviewed-by: Liu Jingqi <jingqi.liu@intel.com>
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---

> +++ b/qapi/misc.json
> @@ -2746,9 +2746,9 @@
>  #
>  # @cpu: property based CPU(s) to node mapping (Since: 2.10)
>  #
> -# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
> +# @hmat-lb: memory latency and bandwidth information (Since: 3.10)

There is no 3.10 release; the next release will be 4.0.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information
  2019-01-21 17:09       ` Eric Blake
@ 2019-01-28  9:42         ` Tao Xu
  0 siblings, 0 replies; 16+ messages in thread
From: Tao Xu @ 2019-01-28  9:42 UTC (permalink / raw
  To: Eric Blake, mst, imammedo
  Cc: ehabkost, jingqi.liu, qemu-devel, pbonzini, danmei.wei, rth

On 1/22/2019 1:09 AM, Eric Blake wrote:
> On 1/21/19 12:03 AM, Tao Xu wrote:
> 
>>>>    #
>>>> +# @hmat-lb: memory latency and bandwidth information (Since: 2.13)
>>> s/2.13/4.0/ (probably in multiple spots in your series)
>> Hi Eric,
>>
>> Thank you for your comments. The spell mistakes in patches 1/9 to 6/9
>> have been corrected in patch 7/9. Because patches 1/9 to 6/9 are jingqi's
>> initial V1 patchesand7/9 to 9/9 are the changes compared withV1.
> 
> Still, it's better to rebase the series to avoid the mistakes in the
> first place, instead of having churn with a mistake early in the series
> corrected only later in the series.  Even if the mistake is corrected by
> a different author than the bulk of the patch, as long as there are
> Signed-off-by lines for both the original author and the person making
> the spelling correction, then proper attribution has been made.
> 
>>
>> About s/2.13/4.0/,do you mean ACPI HMAT will not be merged before QEMU
>> 4.0?
> 
> Correct - 4.0 is the next release planned.  There was no 2.13 release;
> after 2.12 was the 3.0 release; the current release is 3.1, and the next
> release is 4.0; for more details, see
> https://www.qemu.org/2018/08/15/qemu-3-0-0/ for a description of the
> current version numbering scheme.  And even making it in time for 4.0
> means polishing this by mid-March: https://wiki.qemu.org/Planning/4.0
> 
>>
>> In addition, do you have any other comments about these patches?
> 
> I was commenting on the high-level user interface issues, but will leave
> the bulk of the technical review to those more familiar with the code
> being added.
> 
Hi Eric, I am sorry for asking you so frequently and don't mean to rush 
you. Could you tell me when I can know this comments on the high-level 
user interface issues? So that I can improve my patch in the next 
version. Thank you and looking forward to your reply!

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-01-28  9:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-11 15:34 [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 1/9] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 2/9] hmat acpi: Build System Locality Latency and Bandwidth Information " Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 3/9] hmat acpi: Build Memory Side Cache " Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 4/9] Extend the command-line to provide memory latency and bandwidth information Tao Xu
2019-01-14 19:38   ` Eric Blake
2019-01-21  6:03     ` Tao Xu
2019-01-21 17:09       ` Eric Blake
2019-01-28  9:42         ` Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 5/9] numa: Extend the command-line to provide memory side cache information Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 6/9] hmat acpi: Implement _HMA method to update HMAT at runtime Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 7/9] hmat acpi: fix some coding style and small issues Tao Xu
2019-01-21 17:11   ` Eric Blake
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 8/9] hmat acpi: move some function inside of the caller Tao Xu
2019-01-11 15:34 ` [Qemu-devel] [PATCH v2 9/9] acpi: rewrite the _FIT method to use it in _HMA method Tao Xu
2019-01-13 19:07 ` [Qemu-devel] [PATCH v2 0/9] Build ACPI Heterogeneous Memory Attribute Table (HMAT) no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.