[Qemu-devel] [PATCH v7 0/5] ARM: Add NUMA support for machine virt

[Qemu-devel] [PATCH v7 0/5] ARM: Add NUMA support for machine virt

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add NUMA support for machine virt. Tested successfully running a guest
Linux kernel with the following patch applied:

- [PATCH v16 0/6] arm64, numa: Add numa support for arm64 platforms
https://lkml.org/lkml/2016/4/8/571
- [PATCH v5 00/14] ACPI NUMA support for ARM64
https://lkml.org/lkml/2016/4/19/852

Example qemu command line:
qemu-system-aarch64 \
    -enable-kvm -smp 4\
    -kernel Image \
    -m 512 -machine virt,kernel_irqchip=on \
    -initrd guestfs.cpio.gz \
    -cpu host -nographic \
    -numa node,mem=256M,cpus=0-1,nodeid=0 \
    -numa node,mem=256M,cpus=2-3,nodeid=1 \
    -append "console=ttyAMA0 root=/dev/ram"

Changes since v6:
* squash first two patches of previous series together
* fix the definition of proximity in AcpiSratMemoryAffinity
* rename acpi_build_srat_memory to build_acpi_srat_memory

Changes since v5:
* don't generate /distance-map node since it's optional
* improve the /memory node name
* move acpi_build_srat_memory to common place then reuse it to generate
  SRAT table

Changes since v4:
* rebased on new kernel driver and device bindings, especially the
  compatible string "numa-distance-map-v1" of /distance-map node
* set the numa-node-id for first /memory node

Changes since v3:
* based on new kernel driver and device bindings
* add ACPI part

Changes since v2:
* update to use NUMA node property arm,associativity.

Changes since v1:
Take into account Peter's comments:
* rename virt_memory_init to arm_generate_memory_dtb
* move arm_generate_memory_dtb to boot.c and make it a common func
* use a struct numa_map to generate numa dtb

Shannon Zhao (5):
  ARM: Virt: Set numa-node-id for cpu and memory nodes
  ACPI: Add GICC Affinity Structure
  ACPI: Fix the definition of proximity in AcpiSratMemoryAffinity
  ACPI: move acpi_build_srat_memory to common place
  ACPI: Virt: Generate SRAT table

 hw/acpi/aml-build.c         | 11 ++++++++++
 hw/arm/boot.c               | 43 +++++++++++++++++++++++++++++++------
 hw/arm/virt-acpi-build.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/virt.c               |  8 +++++++
 hw/i386/acpi-build.c        | 32 ++++++----------------------
 include/hw/acpi/acpi-defs.h | 17 +++++++++++++--
 include/hw/acpi/aml-build.h | 10 +++++++++
 7 files changed, 139 insertions(+), 34 deletions(-)

-- 
2.0.4

[Qemu-devel] [PATCH v7 1/5] ARM: Virt: Set numa-node-id for cpu and memory nodes

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Generate memory nodes according to NUMA topology. Set numa-node-id
property for cpu and memory nodes.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/boot.c | 43 +++++++++++++++++++++++++++++++++++++------
 hw/arm/virt.c |  8 ++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5975fbf..cbc65a7 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -14,6 +14,7 @@
 #include "hw/arm/linux-boot-if.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/numa.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -405,6 +406,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
     void *fdt = NULL;
     int size, rc;
     uint32_t acells, scells;
+    char *nodename;
+    unsigned int i;
+    hwaddr mem_base, mem_len;
 
     if (binfo->dtb_filename) {
         char *filename;
@@ -456,12 +460,39 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         goto fail;
     }
 
-    rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
-                                      acells, binfo->loader_start,
-                                      scells, binfo->ram_size);
-    if (rc < 0) {
-        fprintf(stderr, "couldn't set /memory/reg\n");
-        goto fail;
+    if (nb_numa_nodes > 0) {
+        /*
+         * Turn the /memory node created before into a NOP node, then create
+         * /memory@addr nodes for all numa nodes respectively.
+         */
+        qemu_fdt_nop_node(fdt, "/memory");
+        mem_base = binfo->loader_start;
+        for (i = 0; i < nb_numa_nodes; i++) {
+            mem_len = numa_info[i].node_mem;
+            nodename = g_strdup_printf("/memory@%" PRIx64, mem_base);
+            qemu_fdt_add_subnode(fdt, nodename);
+            qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
+            rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",
+                                              acells, mem_base,
+                                              scells, mem_len);
+            if (rc < 0) {
+                fprintf(stderr, "couldn't set %s/reg for node %d\n", nodename,
+                        i);
+                goto fail;
+            }
+
+            qemu_fdt_setprop_cell(fdt, nodename, "numa-node-id", i);
+            mem_base += mem_len;
+            g_free(nodename);
+        }
+    } else {
+        rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
+                                          acells, binfo->loader_start,
+                                          scells, binfo->ram_size);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /memory/reg\n");
+            goto fail;
+        }
     }
 
     if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 56d35c7..fe6b11d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -38,6 +38,7 @@
 #include "net/net.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/device_tree.h"
+#include "sysemu/numa.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "hw/boards.h"
@@ -329,6 +330,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
 {
     int cpu;
     int addr_cells = 1;
+    unsigned int i;
 
     /*
      * From Documentation/devicetree/bindings/arm/cpus.txt
@@ -378,6 +380,12 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
                                   armcpu->mp_affinity);
         }
 
+        for (i = 0; i < nb_numa_nodes; i++) {
+            if (test_bit(cpu, numa_info[i].node_cpu)) {
+                qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i);
+            }
+        }
+
         g_free(nodename);
     }
 }
-- 
2.0.4

[Qemu-devel] [PATCH v7 2/5] ACPI: Add GICC Affinity Structure

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 hw/i386/acpi-build.c        |  2 +-
 include/hw/acpi/acpi-defs.h | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6477003..9ae4c0d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2474,7 +2474,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
         int apic_id = apic_ids->cpus[i].arch_id;
 
         core = acpi_data_push(table_data, sizeof *core);
-        core->type = ACPI_SRAT_PROCESSOR;
+        core->type = ACPI_SRAT_PROCESSOR_APIC;
         core->length = sizeof(*core);
         core->local_apic_id = apic_id;
         curnode = pcms->node_cpu[apic_id];
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c7a03d4..bcf5c3f 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -455,8 +455,10 @@ struct AcpiSystemResourceAffinityTable
 } QEMU_PACKED;
 typedef struct AcpiSystemResourceAffinityTable AcpiSystemResourceAffinityTable;
 
-#define ACPI_SRAT_PROCESSOR          0
+#define ACPI_SRAT_PROCESSOR_APIC     0
 #define ACPI_SRAT_MEMORY             1
+#define ACPI_SRAT_PROCESSOR_x2APIC   2
+#define ACPI_SRAT_PROCESSOR_GICC     3
 
 struct AcpiSratProcessorAffinity
 {
@@ -483,6 +485,17 @@ struct AcpiSratMemoryAffinity
 } QEMU_PACKED;
 typedef struct AcpiSratMemoryAffinity AcpiSratMemoryAffinity;
 
+struct AcpiSratProcessorGiccAffinity
+{
+    ACPI_SUB_HEADER_DEF
+    uint32_t    proximity;
+    uint32_t    acpi_processor_uid;
+    uint32_t    flags;
+    uint32_t    clock_domain;
+} QEMU_PACKED;
+
+typedef struct AcpiSratProcessorGiccAffinity AcpiSratProcessorGiccAffinity;
+
 /* PCI fw r3.0 MCFG table. */
 /* Subtable */
 struct AcpiMcfgAllocation {
-- 
2.0.4

[Qemu-devel] [PATCH v7 3/5] ACPI: Fix the definition of proximity in AcpiSratMemoryAffinity

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

ACPI spec says that Proximity Domain is an "Integer that represents
the proximity domain to which the processor belongs". So define it as a
uint32_t.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/i386/acpi-build.c        | 3 +--
 include/hw/acpi/acpi-defs.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9ae4c0d..3c031aa 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2440,8 +2440,7 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
 {
     numamem->type = ACPI_SRAT_MEMORY;
     numamem->length = sizeof(*numamem);
-    memset(numamem->proximity, 0, 4);
-    numamem->proximity[0] = node;
+    numamem->proximity = cpu_to_le32(node);
     numamem->flags = cpu_to_le32(flags);
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index bcf5c3f..850a962 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -475,7 +475,7 @@ typedef struct AcpiSratProcessorAffinity AcpiSratProcessorAffinity;
 struct AcpiSratMemoryAffinity
 {
     ACPI_SUB_HEADER_DEF
-    uint8_t     proximity[4];
+    uint32_t    proximity;
     uint16_t    reserved1;
     uint64_t    base_addr;
     uint64_t    range_length;
-- 
2.0.4

[Qemu-devel] [PATCH v7 4/5] ACPI: move acpi_build_srat_memory to common place

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Move acpi_build_srat_memory to common place so that it could be reused
by ARM. Rename it to build_acpi_srat_memory.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/acpi/aml-build.c         | 11 +++++++++++
 hw/i386/acpi-build.c        | 29 +++++------------------------
 include/hw/acpi/aml-build.h | 10 ++++++++++
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ab89ca6..f945524 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1563,3 +1563,14 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
     build_header(linker, table_data,
                  (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
 }
+
+void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
+                            uint64_t len, int node, MemoryAffinityFlags flags)
+{
+    numamem->type = ACPI_SRAT_MEMORY;
+    numamem->length = sizeof(*numamem);
+    numamem->proximity = cpu_to_le32(node);
+    numamem->flags = cpu_to_le32(flags);
+    numamem->base_addr = cpu_to_le64(base);
+    numamem->range_length = cpu_to_le64(len);
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3c031aa..f939191 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2427,25 +2427,6 @@ build_tpm2(GArray *table_data, GArray *linker)
                  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
-typedef enum {
-    MEM_AFFINITY_NOFLAGS      = 0,
-    MEM_AFFINITY_ENABLED      = (1 << 0),
-    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
-    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
-} MemoryAffinityFlags;
-
-static void
-acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
-                       uint64_t len, int node, MemoryAffinityFlags flags)
-{
-    numamem->type = ACPI_SRAT_MEMORY;
-    numamem->length = sizeof(*numamem);
-    numamem->proximity = cpu_to_le32(node);
-    numamem->flags = cpu_to_le32(flags);
-    numamem->base_addr = cpu_to_le64(base);
-    numamem->range_length = cpu_to_le64(len);
-}
-
 static void
 build_srat(GArray *table_data, GArray *linker, MachineState *machine)
 {
@@ -2491,7 +2472,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
     numa_start = table_data->len;
 
     numamem = acpi_data_push(table_data, sizeof *numamem);
-    acpi_build_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);
+    build_acpi_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);
     next_base = 1024 * 1024;
     for (i = 1; i < pcms->numa_nodes + 1; ++i) {
         mem_base = next_base;
@@ -2507,7 +2488,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
             mem_len -= next_base - pcms->below_4g_mem_size;
             if (mem_len > 0) {
                 numamem = acpi_data_push(table_data, sizeof *numamem);
-                acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
+                build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
                                        MEM_AFFINITY_ENABLED);
             }
             mem_base = 1ULL << 32;
@@ -2515,13 +2496,13 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
             next_base += (1ULL << 32) - pcms->below_4g_mem_size;
         }
         numamem = acpi_data_push(table_data, sizeof *numamem);
-        acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
+        build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
                                MEM_AFFINITY_ENABLED);
     }
     slots = (table_data->len - numa_start) / sizeof *numamem;
     for (; slots < pcms->numa_nodes + 2; slots++) {
         numamem = acpi_data_push(table_data, sizeof *numamem);
-        acpi_build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
+        build_acpi_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
     }
 
     /*
@@ -2531,7 +2512,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
      */
     if (hotplugabble_address_space_size) {
         numamem = acpi_data_push(table_data, sizeof *numamem);
-        acpi_build_srat_memory(numamem, pcms->hotplug_memory.base,
+        build_acpi_srat_memory(numamem, pcms->hotplug_memory.base,
                                hotplugabble_address_space_size, 0,
                                MEM_AFFINITY_HOTPLUGGABLE |
                                MEM_AFFINITY_ENABLED);
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 2c994b3..a9e8a04 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -198,6 +198,13 @@ typedef enum {
     AML_PULL_NONE = 3,
 } AmlPinConfig;
 
+typedef enum {
+    MEM_AFFINITY_NOFLAGS      = 0,
+    MEM_AFFINITY_ENABLED      = (1 << 0),
+    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
+    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
+} MemoryAffinityFlags;
+
 typedef
 struct AcpiBuildTables {
     GArray *table_data;
@@ -372,4 +379,7 @@ int
 build_append_named_dword(GArray *array, const char *name_format, ...)
 GCC_FMT_ATTR(2, 3);
 
+void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
+                            uint64_t len, int node, MemoryAffinityFlags flags);
+
 #endif
-- 
2.0.4

[Qemu-devel] [PATCH v7 5/5] ACPI: Virt: Generate SRAT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

To support NUMA, it needs to generate SRAT ACPI table.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f51fe39..59b3c8f 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -43,6 +43,7 @@
 #include "hw/acpi/aml-build.h"
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pci.h"
+#include "sysemu/numa.h"
 
 #define ARM_SPI_BASE 32
 #define ACPI_POWER_BUTTON_DEVICE "PWRB"
@@ -414,6 +415,52 @@ build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 }
 
 static void
+build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+{
+    AcpiSystemResourceAffinityTable *srat;
+    AcpiSratProcessorGiccAffinity *core;
+    AcpiSratMemoryAffinity *numamem;
+    int i, j, srat_start;
+    uint64_t mem_base;
+    uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
+
+    for (i = 0; i < guest_info->smp_cpus; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            if (test_bit(i, numa_info[j].node_cpu)) {
+                cpu_node[i] = j;
+                break;
+            }
+        }
+    }
+
+    srat_start = table_data->len;
+    srat = acpi_data_push(table_data, sizeof(*srat));
+    srat->reserved1 = cpu_to_le32(1);
+
+    for (i = 0; i < guest_info->smp_cpus; ++i) {
+        core = acpi_data_push(table_data, sizeof(*core));
+        core->type = ACPI_SRAT_PROCESSOR_GICC;
+        core->length = sizeof(*core);
+        core->proximity = cpu_to_le32(cpu_node[i]);
+        core->acpi_processor_uid = cpu_to_le32(i);
+        core->flags = cpu_to_le32(1);
+    }
+    g_free(cpu_node);
+
+    mem_base = guest_info->memmap[VIRT_MEM].base;
+    for (i = 0; i < nb_numa_nodes; ++i) {
+        numamem = acpi_data_push(table_data, sizeof(*numamem));
+        build_acpi_srat_memory(numamem, mem_base, numa_info[i].node_mem, i,
+                               MEM_AFFINITY_ENABLED);
+        mem_base += numa_info[i].node_mem;
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + srat_start), "SRAT",
+                 table_data->len - srat_start, 3, NULL, NULL);
+}
+
+static void
 build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 {
     AcpiTableMcfg *mcfg;
@@ -638,6 +685,11 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
     acpi_add_table(table_offsets, tables_blob);
     build_spcr(tables_blob, tables->linker, guest_info);
 
+    if (nb_numa_nodes > 0) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_srat(tables_blob, tables->linker, guest_info);
+    }
+
     /* RSDT is pointed to by RSDP */
     rsdt = tables_blob->len;
     build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
-- 
2.0.4

Re: [Qemu-devel] [PATCH v7 3/5] ACPI: Fix the definition of proximity in AcpiSratMemoryAffinity

[Andrew Jones]

On Mon, Apr 25, 2016 at 04:05:45PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> ACPI spec says that Proximity Domain is an "Integer that represents
> the proximity domain to which the processor belongs". So define it as a
> uint32_t.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/i386/acpi-build.c        | 3 +--
>  include/hw/acpi/acpi-defs.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9ae4c0d..3c031aa 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2440,8 +2440,7 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>  {
>      numamem->type = ACPI_SRAT_MEMORY;
>      numamem->length = sizeof(*numamem);
> -    memset(numamem->proximity, 0, 4);
> -    numamem->proximity[0] = node;
> +    numamem->proximity = cpu_to_le32(node);
>      numamem->flags = cpu_to_le32(flags);
>      numamem->base_addr = cpu_to_le64(base);
>      numamem->range_length = cpu_to_le64(len);
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index bcf5c3f..850a962 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -475,7 +475,7 @@ typedef struct AcpiSratProcessorAffinity AcpiSratProcessorAffinity;
>  struct AcpiSratMemoryAffinity
>  {
>      ACPI_SUB_HEADER_DEF
> -    uint8_t     proximity[4];
> +    uint32_t    proximity;
>      uint16_t    reserved1;
>      uint64_t    base_addr;
>      uint64_t    range_length;
> -- 
> 2.0.4
> 
> 
> 

Re: [Qemu-devel] [PATCH v7 4/5] ACPI: move acpi_build_srat_memory to common place

[Andrew Jones]

On Mon, Apr 25, 2016 at 04:05:46PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move acpi_build_srat_memory to common place so that it could be reused
> by ARM. Rename it to build_acpi_srat_memory.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/acpi/aml-build.c         | 11 +++++++++++
>  hw/i386/acpi-build.c        | 29 +++++------------------------
>  include/hw/acpi/aml-build.h | 10 ++++++++++
>  3 files changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ab89ca6..f945524 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1563,3 +1563,14 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
>      build_header(linker, table_data,
>                   (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>  }
> +
> +void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags)
> +{
> +    numamem->type = ACPI_SRAT_MEMORY;
> +    numamem->length = sizeof(*numamem);
> +    numamem->proximity = cpu_to_le32(node);
> +    numamem->flags = cpu_to_le32(flags);
> +    numamem->base_addr = cpu_to_le64(base);
> +    numamem->range_length = cpu_to_le64(len);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 3c031aa..f939191 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2427,25 +2427,6 @@ build_tpm2(GArray *table_data, GArray *linker)
>                   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>  }
>  
> -typedef enum {
> -    MEM_AFFINITY_NOFLAGS      = 0,
> -    MEM_AFFINITY_ENABLED      = (1 << 0),
> -    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> -    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> -} MemoryAffinityFlags;
> -
> -static void
> -acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> -                       uint64_t len, int node, MemoryAffinityFlags flags)
> -{
> -    numamem->type = ACPI_SRAT_MEMORY;
> -    numamem->length = sizeof(*numamem);
> -    numamem->proximity = cpu_to_le32(node);
> -    numamem->flags = cpu_to_le32(flags);
> -    numamem->base_addr = cpu_to_le64(base);
> -    numamem->range_length = cpu_to_le64(len);
> -}
> -
>  static void
>  build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>  {
> @@ -2491,7 +2472,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>      numa_start = table_data->len;
>  
>      numamem = acpi_data_push(table_data, sizeof *numamem);
> -    acpi_build_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);
> +    build_acpi_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);
>      next_base = 1024 * 1024;
>      for (i = 1; i < pcms->numa_nodes + 1; ++i) {
>          mem_base = next_base;
> @@ -2507,7 +2488,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>              mem_len -= next_base - pcms->below_4g_mem_size;
>              if (mem_len > 0) {
>                  numamem = acpi_data_push(table_data, sizeof *numamem);
> -                acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
> +                build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
>                                         MEM_AFFINITY_ENABLED);
>              }
>              mem_base = 1ULL << 32;
> @@ -2515,13 +2496,13 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>              next_base += (1ULL << 32) - pcms->below_4g_mem_size;
>          }
>          numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
> +        build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
>                                 MEM_AFFINITY_ENABLED);
>      }
>      slots = (table_data->len - numa_start) / sizeof *numamem;
>      for (; slots < pcms->numa_nodes + 2; slots++) {
>          numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
> +        build_acpi_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>      }
>  
>      /*
> @@ -2531,7 +2512,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>       */
>      if (hotplugabble_address_space_size) {
>          numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, pcms->hotplug_memory.base,
> +        build_acpi_srat_memory(numamem, pcms->hotplug_memory.base,
>                                 hotplugabble_address_space_size, 0,
>                                 MEM_AFFINITY_HOTPLUGGABLE |
>                                 MEM_AFFINITY_ENABLED);
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 2c994b3..a9e8a04 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -198,6 +198,13 @@ typedef enum {
>      AML_PULL_NONE = 3,
>  } AmlPinConfig;
>  
> +typedef enum {
> +    MEM_AFFINITY_NOFLAGS      = 0,
> +    MEM_AFFINITY_ENABLED      = (1 << 0),
> +    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> +    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> +} MemoryAffinityFlags;
> +
>  typedef
>  struct AcpiBuildTables {
>      GArray *table_data;
> @@ -372,4 +379,7 @@ int
>  build_append_named_dword(GArray *array, const char *name_format, ...)
>  GCC_FMT_ATTR(2, 3);
>  
> +void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags);
> +
>  #endif
> -- 
> 2.0.4
> 
> 
>

I probably would have just called it 'build_srat_memory', but anyway

Reviewed-by: Andrew Jones <drjones@redhat.com>
 

Re: [Qemu-devel] [PATCH v7 4/5] ACPI: move acpi_build_srat_memory to common place

[Marcel Apfelbaum]

On 04/25/2016 11:05 AM, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Move acpi_build_srat_memory to common place so that it could be reused
> by ARM. Rename it to build_acpi_srat_memory.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>   hw/acpi/aml-build.c         | 11 +++++++++++
>   hw/i386/acpi-build.c        | 29 +++++------------------------
>   include/hw/acpi/aml-build.h | 10 ++++++++++
>   3 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ab89ca6..f945524 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1563,3 +1563,14 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
>       build_header(linker, table_data,
>                    (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>   }
> +
> +void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags)
> +{
> +    numamem->type = ACPI_SRAT_MEMORY;
> +    numamem->length = sizeof(*numamem);
> +    numamem->proximity = cpu_to_le32(node);
> +    numamem->flags = cpu_to_le32(flags);
> +    numamem->base_addr = cpu_to_le64(base);
> +    numamem->range_length = cpu_to_le64(len);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 3c031aa..f939191 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2427,25 +2427,6 @@ build_tpm2(GArray *table_data, GArray *linker)
>                    (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>   }
>
> -typedef enum {
> -    MEM_AFFINITY_NOFLAGS      = 0,
> -    MEM_AFFINITY_ENABLED      = (1 << 0),
> -    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> -    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> -} MemoryAffinityFlags;
> -
> -static void
> -acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> -                       uint64_t len, int node, MemoryAffinityFlags flags)
> -{
> -    numamem->type = ACPI_SRAT_MEMORY;
> -    numamem->length = sizeof(*numamem);
> -    numamem->proximity = cpu_to_le32(node);
> -    numamem->flags = cpu_to_le32(flags);
> -    numamem->base_addr = cpu_to_le64(base);
> -    numamem->range_length = cpu_to_le64(len);
> -}
> -
>   static void
>   build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>   {
> @@ -2491,7 +2472,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>       numa_start = table_data->len;
>
>       numamem = acpi_data_push(table_data, sizeof *numamem);
> -    acpi_build_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);
> +    build_acpi_srat_memory(numamem, 0, 640*1024, 0, MEM_AFFINITY_ENABLED);

Please run checkpatch before submitting. It will help getting it in sooner :)
In this case the ERROR was here before your code, but you have chance to fix it.

Thanks,
Marcel

>       next_base = 1024 * 1024;
>       for (i = 1; i < pcms->numa_nodes + 1; ++i) {
>           mem_base = next_base;
> @@ -2507,7 +2488,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>               mem_len -= next_base - pcms->below_4g_mem_size;
>               if (mem_len > 0) {
>                   numamem = acpi_data_push(table_data, sizeof *numamem);
> -                acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
> +                build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
>                                          MEM_AFFINITY_ENABLED);
>               }
>               mem_base = 1ULL << 32;
> @@ -2515,13 +2496,13 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>               next_base += (1ULL << 32) - pcms->below_4g_mem_size;
>           }
>           numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, mem_base, mem_len, i - 1,
> +        build_acpi_srat_memory(numamem, mem_base, mem_len, i - 1,
>                                  MEM_AFFINITY_ENABLED);
>       }
>       slots = (table_data->len - numa_start) / sizeof *numamem;
>       for (; slots < pcms->numa_nodes + 2; slots++) {
>           numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
> +        build_acpi_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>       }
>
>       /*
> @@ -2531,7 +2512,7 @@ build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>        */
>       if (hotplugabble_address_space_size) {
>           numamem = acpi_data_push(table_data, sizeof *numamem);
> -        acpi_build_srat_memory(numamem, pcms->hotplug_memory.base,
> +        build_acpi_srat_memory(numamem, pcms->hotplug_memory.base,
>                                  hotplugabble_address_space_size, 0,
>                                  MEM_AFFINITY_HOTPLUGGABLE |
>                                  MEM_AFFINITY_ENABLED);
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 2c994b3..a9e8a04 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -198,6 +198,13 @@ typedef enum {
>       AML_PULL_NONE = 3,
>   } AmlPinConfig;
>
> +typedef enum {
> +    MEM_AFFINITY_NOFLAGS      = 0,
> +    MEM_AFFINITY_ENABLED      = (1 << 0),
> +    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> +    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> +} MemoryAffinityFlags;
> +
>   typedef
>   struct AcpiBuildTables {
>       GArray *table_data;
> @@ -372,4 +379,7 @@ int
>   build_append_named_dword(GArray *array, const char *name_format, ...)
>   GCC_FMT_ATTR(2, 3);
>
> +void build_acpi_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags);
> +
>   #endif
>