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

[Qemu-devel] [PATCH v6 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 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 CPUs
  ARM: Add numa-node-id for /memory node
  ACPI: Add GICC Affinity Structure
  ACPI: move acpi_build_srat_memory to common place
  ACPI: Virt: Generate SRAT table

 hw/acpi/aml-build.c         | 12 +++++++++++
 hw/arm/boot.c               | 43 +++++++++++++++++++++++++++++++------
 hw/arm/virt-acpi-build.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/virt.c               |  8 +++++++
 hw/i386/acpi-build.c        | 22 +------------------
 include/hw/acpi/acpi-defs.h | 15 ++++++++++++-
 include/hw/acpi/aml-build.h | 10 +++++++++
 7 files changed, 134 insertions(+), 28 deletions(-)

-- 
2.0.4

[Qemu-devel] [PATCH v6 1/5] ARM: Virt: Set numa-node-id for CPUs

[Shannon Zhao]

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

Add a numa-node-id property to specify NUMA information for CPUs.

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

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 v6 2/5] ARM: Add numa-node-id for /memory node

[Shannon Zhao]

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

When specifying NUMA for ARM machine, generate /memory node according to
NUMA topology.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/boot.c | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 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) {
-- 
2.0.4

[Qemu-devel] [PATCH v6 3/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 v6 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.

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         | 12 ++++++++++++
 hw/i386/acpi-build.c        | 20 --------------------
 include/hw/acpi/aml-build.h | 10 ++++++++++
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ab89ca6..d167003 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1563,3 +1563,15 @@ 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 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);
+    memset(numamem->proximity, 0, 4);
+    numamem->proximity[0] = 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 9ae4c0d..cd93825 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2427,26 +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);
-    memset(numamem->proximity, 0, 4);
-    numamem->proximity[0] = 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)
 {
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 2c994b3..d8f9fca 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 acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
+                            uint64_t len, int node, MemoryAffinityFlags flags);
+
 #endif
-- 
2.0.4

[Qemu-devel] [PATCH v6 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>
---
 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..e0e90d4 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));
+        acpi_build_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 v6 2/5] ARM: Add numa-node-id for /memory node

[Andrew Jones]

On Sat, Apr 23, 2016 at 05:06:33PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> When specifying NUMA for ARM machine, generate /memory node according to
> NUMA topology.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/boot.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)

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

> 
> 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) {
> -- 
> 2.0.4
> 
>

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

[Andrew Jones]

On Sat, Apr 23, 2016 at 05:06:35PM +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.
> 
> 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         | 12 ++++++++++++
>  hw/i386/acpi-build.c        | 20 --------------------
>  include/hw/acpi/aml-build.h | 10 ++++++++++
>  3 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ab89ca6..d167003 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1563,3 +1563,15 @@ 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 acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags)

It looks like functions like these in hw/acpi/aml-build.c usually start
with 'build_' not 'acpi_'

> +{
> +    numamem->type = ACPI_SRAT_MEMORY;
> +    numamem->length = sizeof(*numamem);
> +    memset(numamem->proximity, 0, 4);

This memset thing is still weird...

> +    numamem->proximity[0] = 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 9ae4c0d..cd93825 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2427,26 +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);
> -    memset(numamem->proximity, 0, 4);
> -    numamem->proximity[0] = 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)
>  {
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 2c994b3..d8f9fca 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 acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags);
> +
>  #endif
> -- 
> 2.0.4
> 
> 
>

Otherwise looks good to me.

drew 

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

[Andrew Jones]

On Sat, Apr 23, 2016 at 05:06:36PM +0800, Shannon Zhao wrote:
> 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>
> ---
>  hw/arm/virt-acpi-build.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)

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

> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f51fe39..e0e90d4 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));
> +        acpi_build_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 v6 0/5] ARM: Add NUMA support for machine virt

[Andrew Jones]

On Sat, Apr 23, 2016 at 05:06:31PM +0800, Shannon Zhao wrote:
> 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 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 CPUs
>   ARM: Add numa-node-id for /memory node

I think we need to squash these first two patch together. Otherwise if
you're bisecting qemu using a guest boot for a test case, then you could
end up booting with numa-node-id set in the cpus, without their
corresponding memory node.

drew

>   ACPI: Add GICC Affinity Structure
>   ACPI: move acpi_build_srat_memory to common place
>   ACPI: Virt: Generate SRAT table
> 
>  hw/acpi/aml-build.c         | 12 +++++++++++
>  hw/arm/boot.c               | 43 +++++++++++++++++++++++++++++++------
>  hw/arm/virt-acpi-build.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/virt.c               |  8 +++++++
>  hw/i386/acpi-build.c        | 22 +------------------
>  include/hw/acpi/acpi-defs.h | 15 ++++++++++++-
>  include/hw/acpi/aml-build.h | 10 +++++++++
>  7 files changed, 134 insertions(+), 28 deletions(-)
> 
> -- 
> 2.0.4
> 
> 
> 

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

[Shannon Zhao]

On 2016/4/25 1:00, Andrew Jones wrote:
> On Sat, Apr 23, 2016 at 05:06:35PM +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.
>> > 
>> > 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         | 12 ++++++++++++
>> >  hw/i386/acpi-build.c        | 20 --------------------
>> >  include/hw/acpi/aml-build.h | 10 ++++++++++
>> >  3 files changed, 22 insertions(+), 20 deletions(-)
>> > 
>> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> > index ab89ca6..d167003 100644
>> > --- a/hw/acpi/aml-build.c
>> > +++ b/hw/acpi/aml-build.c
>> > @@ -1563,3 +1563,15 @@ 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 acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>> > +                            uint64_t len, int node, MemoryAffinityFlags flags)
> It looks like functions like these in hw/acpi/aml-build.c usually start
> with 'build_' not 'acpi_'
> 
Ok, I will rename it to build_acpi_srat_memory.

>> > +{
>> > +    numamem->type = ACPI_SRAT_MEMORY;
>> > +    numamem->length = sizeof(*numamem);
>> > +    memset(numamem->proximity, 0, 4);
> This memset thing is still weird...
> 
I will add a patch before this one to fix the definition of proximity
and use uint32_t as you said before.

Thanks,
-- 
Shannon