Allow setting NUMA distance for different NUMA nodes

Current, QEMU does not provide a clear command to set vNUMA distance for guest although we already have -numa command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA info via numactl -H, we will see:

node distance:
node 0 1 2 3

  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node as distance 20 when there is no SLIT table since QEMU doesn't build it. It looks like a little strange when you have seen the distance in an actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node 0 1 2 3

  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table. I found QEMU has provide -acpitable command that allows users to add a ACPI table into guest, but it requires users building ACPI table by themselves first. Using -acpitable to add a SLIT table may be not so straightforward or flexible, imagine that when the vNUMA configuration is changes and we need to generate another SLIT table manually. It may not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides additional option dist for command -numa to allow user set vNUMA distance by QEMU command.

With this patch, when a user wants to create a guest that contains several vNUMA nodes and also wants to set distance among those nodes, the QEMU command would like:

```

(quoted diff) +0 -12

-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...

```

Signed-off-by He Chen <[email protected]>

fix

---
 hw/acpi/aml-build.c         | 26 +++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    |  2 ++
 hw/i386/acpi-build.c        |  2 ++
 include/hw/acpi/aml-build.h |  1 +
 include/sysemu/numa.h       |  1 +
 include/sysemu/sysemu.h     |  4 ++++
 numa.c                      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json            | 30 ++++++++++++++++++++++++++---
 qemu-options.hx             | 12 +++++++++++-
 9 files changed, 121 insertions(+), 4 deletions(-)

hw/acpi/aml-build.c +26 -0

index c6f2032..410b30e 100644
@@ -24,6 +24,7 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
+#include "sysemu/numa.h"
 
 static GArray *build_alloc_array(void)
 {

@@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }
+
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+void build_slit(GArray *table_data, BIOSLinker *linker)
+{
+    int slit_start, i, j;
+    slit_start = table_data->len;
+
+    acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+    build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+        }
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + slit_start),
+                 "SLIT",
+                 table_data->len - slit_start, 1, NULL, NULL);
+}
+

hw/arm/virt-acpi-build.c +0 -0

index 0835e59..d9e6828 100644

hw/arm/virt-acpi-build.c +0 -0

     }
 
     if (its_class_name() && !vmc->no_its) {

hw/i386/acpi-build.c +2 -0

index 2073108..12730ea 100644
@@ -2678,6 +2678,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (pcms->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/aml-build.h +1 -0

index 00c21f1..329a0d0 100644
@@ -389,4 +389,5 @@ GCC_FMT_ATTR(2, 3);
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_slit(GArray *table_data, BIOSLinker *linker);
 #endif

include/sysemu/numa.h +1 -0

index 8f09dcf..2f7a941 100644
@@ -21,6 +21,7 @@ typedef struct node_info {
     struct HostMemoryBackend *node_memdev;
     bool present;
     QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+    uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];

include/sysemu/sysemu.h +0 -0

index 576c7ce..a4e328d 100644

include/sysemu/sysemu.h +0 -0

 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

numa.c +3 -0

index e01cb54..425a320 100644
@@ -212,6 +212,28 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 
+static void numa_distance_parse(NumaDistOptions *dist, QemuOpts *opts, Error **errp)
+{
+    uint64_t src = dist->src;

numa.c +34 -0

+        error_setg(errp,
+                "NUMA distance (%" PRIu8 ") out of range (%d) ~ (%d)",
+                dist->val, MIN_NUMA_DISTANCE, MAX_NUMA_DISTANCE);
+        return;
+    }
+
+    numa_info[src].distance[dst] = val;
+}
+
 static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;

@@ -235,6 +257,12 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         }
         nb_numa_nodes++;
         break;
+    case NUMA_OPTIONS_TYPE_DIST:
+        numa_distance_parse(&object->u.dist, opts, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }

@@ -294,6 +322,24 @@ static void validate_numa_cpus(void)
     g_free(seen_cpus);
 }
 
+static void default_numa_distance(void)
+{
+    int src, dst;
+
+    for (src = 0; src < nb_numa_nodes; src++) {
+        for (dst = 0; dst < nb_numa_nodes; dst++) {
+            if (src == dst && numa_info[src].distance[dst] != MIN_NUMA_DISTANCE) {
+                numa_info[src].distance[dst] = MIN_NUMA_DISTANCE;
+            } else if (numa_info[src].distance[dst] <= MIN_NUMA_DISTANCE) {
+                if (numa_info[dst].distance[src] > MIN_NUMA_DISTANCE)
+                    numa_info[src].distance[dst] = numa_info[dst].distance[src];
+                else
+                    numa_info[src].distance[dst] = DEF_NUMA_DISTANCE;
+            }
+        }
+    }
+}
+
 void parse_numa_opts(MachineClass *mc)
 {
     int i;

@@ -390,6 +436,7 @@ void parse_numa_opts(MachineClass *mc)
         }
 
         validate_numa_cpus();
+        default_numa_distance();
     } else {
         numa_set_mem_node_id(0, ram_size, 0);
     }

qapi-schema.json +8 -3

index 32b4a4b..21ad94a 100644
@@ -5644,15 +5644,19 @@
 ##
 # @NumaOptionsType:
 #
+# @node: NUMA nodes configuration
+#
+# @dist: NUMA distance configuration
+#
 # Since: 2.1
 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node' ] }
+  'data': [ 'node', 'dist' ] }
 
 ##
 # @NumaOptions:
 #
-# A discriminated record of NUMA options. (for OptsVisitor)
+# A discriminated record of NUMA options.
 #
 # Since: 2.1
 ##

@@ -5660,7 +5664,8 @@
   'base': { 'type': 'NumaOptionsType' },
   'discriminator': 'type',
   'data': {
-    'node': 'NumaNodeOptions' }}
+    'node': 'NumaNodeOptions',
+    'dist': 'NumaDistOptions' }}
 
 ##
 # @NumaNodeOptions:

qapi-schema.json +16 -0

+#
+# @src: source NUMA node.
+#
+# @dst: destination NUMA node.
+#
+# @val: NUMA distance from source node to destination node.
+#
+# Since: 2.10
+##
+{ 'struct': 'NumaDistOptions',
+  'data': {
+   'src': 'uint64',
+   'dst': 'uint64',
+   'val': 'uint8' }}
+
+##
 # @HostMemPolicy:
 #
 # Host memory policy types

qemu-options.hx +0 -0

index 8dd8ee3..43c3950 100644
@@ -139,12 +139,15 @@ ETEXI
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
     "-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"

qemu-options.hx +1 -0

+
 Note that the -@option{numa} option doesn't allocate any of the
 specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},

Signature · 1 line

2.7.4

You should have CC'ed me, considering this version is addressing my review comments, but whatever, I usually skim the list so I found it...

Quoted 58 lines · He Chen

Current, QEMU does not provide a clear command to set vNUMA distance for guest although we already have -numa command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA info via numactl -H, we will see:

node distance:
node 0 1 2 3

  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node as distance 20 when there is no SLIT table since QEMU doesn't build it. It looks like a little strange when you have seen the distance in an actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node 0 1 2 3

  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table. I found QEMU has provide -acpitable command that allows users to add a ACPI table into guest, but it requires users building ACPI table by themselves first. Using -acpitable to add a SLIT table may be not so straightforward or flexible, imagine that when the vNUMA configuration is changes and we need to generate another SLIT table manually. It may not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides additional option dist for command -numa to allow user set vNUMA distance by QEMU command.

With this patch, when a user wants to create a guest that contains several vNUMA nodes and also wants to set distance among those nodes, the QEMU command would like:

```

(quoted diff) +0 -12

-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...

```

Quoted 3 lines

Signed-off-by He Chen <[email protected]>

fix

stray 'fix' above the ---

Quoted 59 lines

---
 hw/acpi/aml-build.c         | 26 +++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    |  2 ++
 hw/i386/acpi-build.c        |  2 ++
 include/hw/acpi/aml-build.h |  1 +
 include/sysemu/numa.h       |  1 +
 include/sysemu/sysemu.h     |  4 ++++
 numa.c                      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json            | 30 ++++++++++++++++++++++++++---
 qemu-options.hx             | 12 +++++++++++-
 9 files changed, 121 insertions(+), 4 deletions(-)

hw/acpi/aml-build.c +26 -0

index c6f2032..410b30e 100644
@@ -24,6 +24,7 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
+#include "sysemu/numa.h"
 
 static GArray *build_alloc_array(void)
 {

@@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }
+
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+void build_slit(GArray *table_data, BIOSLinker *linker)
+{
+    int slit_start, i, j;
+    slit_start = table_data->len;
+
+    acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+    build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+        }
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + slit_start),
+                 "SLIT",
+                 table_data->len - slit_start, 1, NULL, NULL);
+}
+

hw/arm/virt-acpi-build.c +0 -0

index 0835e59..d9e6828 100644
@@ -781,6 +781,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     if (nb_numa_nodes > 0) {

hw/arm/virt-acpi-build.c void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) · line 785

Quoted 4 lines

hw/arm/virt-acpi-build.c +2 -0

         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, vms);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);

I didn't expect you'd actually add the support to the ARM machine model, but only provide the API. Anyway, thanks for this, assuming you've tested it and it works.

Quoted 1 line

hw/arm/virt-acpi-build.c +0 -0

     }

Quoted 43 lines

hw/arm/virt-acpi-build.c +0 -0

     if (its_class_name() && !vmc->no_its) {

hw/i386/acpi-build.c +2 -0

index 2073108..12730ea 100644
@@ -2678,6 +2678,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (pcms->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/aml-build.h +1 -0

index 00c21f1..329a0d0 100644
@@ -389,4 +389,5 @@ GCC_FMT_ATTR(2, 3);
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_slit(GArray *table_data, BIOSLinker *linker);
 #endif

include/sysemu/numa.h +1 -0

index 8f09dcf..2f7a941 100644
@@ -21,6 +21,7 @@ typedef struct node_info {
     struct HostMemoryBackend *node_memdev;
     bool present;
     QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+    uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];

include/sysemu/sysemu.h +0 -0

index 576c7ce..a4e328d 100644
@@ -169,6 +169,10 @@ extern int mem_prealloc;
 

include/sysemu/sysemu.h extern int mem_prealloc; · line 174

Quoted 5 lines

include/sysemu/sysemu.h +3 -0

 #define MAX_NODES 128
 #define NUMA_NODE_UNASSIGNED MAX_NODES
+#define MIN_NUMA_DISTANCE 10
+#define DEF_NUMA_DISTANCE 20
+#define MAX_NUMA_DISTANCE 254

You ignored my comment about starting with 'NUMA_DISTANCE' without even replying to that comment stating why you prefer it this way. That's a bit annoying.

include/sysemu/sysemu.h extern int mem_prealloc; · line 175

Quoted 1 line

include/sysemu/sysemu.h +1 -0

+#define NUMA_DISTANCE_UNREACHABLE 255

Here you do start with NUMA_DISTANCE, so these names have an inconsistent format.

Quoted 11 lines

include/sysemu/sysemu.h +0 -0

 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

numa.c +0 -0

index e01cb54..425a320 100644
@@ -212,6 +212,28 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 

numa.c static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) · line 227

Quoted 13 lines

numa.c +13 -0

+static void numa_distance_parse(NumaDistOptions *dist, QemuOpts *opts, Error **errp)
+{
+    uint64_t src = dist->src;
+    uint64_t dst = dist->dst;
+    uint8_t val = dist->val;
+
+    if (src >= MAX_NODES || dst >= MAX_NODES) {
+        error_setg(errp, "Max number of NUMA nodes reached: %"
+                   PRIu64 "", src > dst ? src : dst);
+        return;
+    }
+
+    if (val < MIN_NUMA_DISTANCE || val > MAX_NUMA_DISTANCE) {

val > MAX_NUMA_DISTANCE is not invalid, as NUMA_DISTANCE_UNREACHABLE is 255. Isn't it possible that that could be what the user wants?

Quoted 95 lines

numa.c +34 -0

+        error_setg(errp,
+                "NUMA distance (%" PRIu8 ") out of range (%d) ~ (%d)",
+                dist->val, MIN_NUMA_DISTANCE, MAX_NUMA_DISTANCE);
+        return;
+    }
+
+    numa_info[src].distance[dst] = val;
+}
+
 static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;

@@ -235,6 +257,12 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         }
         nb_numa_nodes++;
         break;
+    case NUMA_OPTIONS_TYPE_DIST:
+        numa_distance_parse(&object->u.dist, opts, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }

@@ -294,6 +322,24 @@ static void validate_numa_cpus(void)
     g_free(seen_cpus);
 }
 
+static void default_numa_distance(void)
+{
+    int src, dst;
+
+    for (src = 0; src < nb_numa_nodes; src++) {
+        for (dst = 0; dst < nb_numa_nodes; dst++) {
+            if (src == dst && numa_info[src].distance[dst] != MIN_NUMA_DISTANCE) {
+                numa_info[src].distance[dst] = MIN_NUMA_DISTANCE;
+            } else if (numa_info[src].distance[dst] <= MIN_NUMA_DISTANCE) {
+                if (numa_info[dst].distance[src] > MIN_NUMA_DISTANCE)
+                    numa_info[src].distance[dst] = numa_info[dst].distance[src];
+                else
+                    numa_info[src].distance[dst] = DEF_NUMA_DISTANCE;
+            }
+        }
+    }
+}
+
 void parse_numa_opts(MachineClass *mc)
 {
     int i;

@@ -390,6 +436,7 @@ void parse_numa_opts(MachineClass *mc)
         }
 
         validate_numa_cpus();
+        default_numa_distance();
     } else {
         numa_set_mem_node_id(0, ram_size, 0);
     }

qapi-schema.json +8 -3

index 32b4a4b..21ad94a 100644
@@ -5644,15 +5644,19 @@
 ##
 # @NumaOptionsType:
 #
+# @node: NUMA nodes configuration
+#
+# @dist: NUMA distance configuration
+#
 # Since: 2.1
 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node' ] }
+  'data': [ 'node', 'dist' ] }
 
 ##
 # @NumaOptions:
 #
-# A discriminated record of NUMA options. (for OptsVisitor)
+# A discriminated record of NUMA options.
 #
 # Since: 2.1
 ##

@@ -5660,7 +5664,8 @@
   'base': { 'type': 'NumaOptionsType' },
   'discriminator': 'type',
   'data': {
-    'node': 'NumaNodeOptions' }}
+    'node': 'NumaNodeOptions',
+    'dist': 'NumaDistOptions' }}
 
 ##
 # @NumaNodeOptions:

qapi-schema.json line 5699

Quoted 6 lines

qapi-schema.json +3 -0

@@ -5689,6 +5694,25 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaDistOptions:
+#
+# Set distance between 2 NUMA nodes. (for OptsVisitor)

Set the distance

Quoted 27 lines

qapi-schema.json +16 -0

+#
+# @src: source NUMA node.
+#
+# @dst: destination NUMA node.
+#
+# @val: NUMA distance from source node to destination node.
+#
+# Since: 2.10
+##
+{ 'struct': 'NumaDistOptions',
+  'data': {
+   'src': 'uint64',
+   'dst': 'uint64',
+   'val': 'uint8' }}
+
+##
 # @HostMemPolicy:
 #
 # Host memory policy types

qemu-options.hx +0 -0

index 8dd8ee3..43c3950 100644
@@ -139,12 +139,15 @@ ETEXI
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
     "-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"

qemu-options.hx ETEXI · line 150

Quoted 10 lines

qemu-options.hx +4 -1

-    "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
+    "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
+    "-numa dist,src=source,dst=destination,val=distance\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}
 @findex -numa
 Define a NUMA node and assign RAM and VCPUs to it.
+Set NUMA distance from source node to destination node.

Set the NUMA distance from a source node to a destination node.

qemu-options.hx split equally between them. · line 176

Quoted 11 lines

qemu-options.hx +4 -0

 @var{firstcpu} and @var{lastcpu} are CPU indexes. Each
 @samp{cpus} option represent a contiguous range of CPU indexes

@@ -167,6 +170,13 @@ split equally between them.
 @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
 if one node uses @samp{memdev}, all of them have to use it.
 
+@var{source} and @var{destination} are NUMA node IDs.
+@var{distance} is the NUMA distance from @var{source} to @var{destination}.
+The distance from node A to node B may be different from the distance from
+node B to node A since the distance can to be asymmetry.

/since/as/
/asymmetry/asymmetrical/

qemu-options.hx split equally between them. · line 177

Quoted 1 line

qemu-options.hx +1 -0

+If the distance is not set, the default distance for a local NUMA node is 10,

no need for ,

qemu-options.hx split equally between them. · line 178

Quoted 1 line

qemu-options.hx +1 -0

+and 20 for a remote node.

and the default distance for a remote node is 20.

Also need sentence like: When the distance for only one direction is given, then the opposite direction defaults to the same distance.

And maybe should also point out how to set an unreachable node with 255.

Quoted 8 lines

qemu-options.hx +1 -0

+
 Note that the -@option{numa} option doesn't allocate any of the
 specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},

Signature · 1 line

2.7.4

drew

Quoted 3 lines

And maybe should also point out how to set an unreachable node with 255.

Thanks for your careful review. Regarding setting unreachable node, which way is good? Set exact 255 as distance or distance that is larger than 255 is regarded as unreachable?

Quoted 5 lines

Quoted 1 line

-numa dist,src=1,dst=0,val=21 \

And maybe should also point out how to set an unreachable node with 255.

Thanks for your careful review. Regarding setting unreachable node, which way is good? Set exact 255 as distance or distance that is larger than 255 is regarded as unreachable?

Current, QEMU does not provide a clear command to set vNUMA distance for guest although we already have -numa command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA info via numactl -H, we will see:

node distance:
node 0 1 2 3

  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node as distance 20 when there is no SLIT table since QEMU doesn't build it. It looks like a little strange when you have seen the distance in an actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node 0 1 2 3

  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table. I found QEMU has provide -acpitable command that allows users to add a ACPI table into guest, but it requires users building ACPI table by themselves first. Using -acpitable to add a SLIT table may be not so straightforward or flexible, imagine that when the vNUMA configuration is changes and we need to generate another SLIT table manually. It may not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides additional option dist for command -numa to allow user set vNUMA distance by QEMU command.

With this patch, when a user wants to create a guest that contains several vNUMA nodes and also wants to set distance among those nodes, the QEMU command would like:

```

(quoted diff) +0 -12

-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...

```

Signed-off-by He Chen <[email protected]>

fix

---
 hw/acpi/aml-build.c         | 26 +++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    |  2 ++
 hw/i386/acpi-build.c        |  2 ++
 include/hw/acpi/aml-build.h |  1 +
 include/sysemu/numa.h       |  1 +
 include/sysemu/sysemu.h     |  4 ++++
 numa.c                      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json            | 30 ++++++++++++++++++++++++++---
 qemu-options.hx             | 12 +++++++++++-
 9 files changed, 121 insertions(+), 4 deletions(-)

hw/acpi/aml-build.c +26 -0

index c6f2032..410b30e 100644
@@ -24,6 +24,7 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
+#include "sysemu/numa.h"
 
 static GArray *build_alloc_array(void)
 {

@@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }
+
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+void build_slit(GArray *table_data, BIOSLinker *linker)
+{
+    int slit_start, i, j;
+    slit_start = table_data->len;
+
+    acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+    build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+        }
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + slit_start),
+                 "SLIT",
+                 table_data->len - slit_start, 1, NULL, NULL);
+}
+

hw/arm/virt-acpi-build.c +2 -0

index 0835e59..d9e6828 100644
@@ -781,6 +781,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     if (nb_numa_nodes > 0) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, vms);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
 
     if (its_class_name() && !vmc->no_its) {

hw/i386/acpi-build.c +2 -0

index 2073108..12730ea 100644
@@ -2678,6 +2678,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (pcms->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/aml-build.h +1 -0

index 00c21f1..329a0d0 100644
@@ -389,4 +389,5 @@ GCC_FMT_ATTR(2, 3);
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_slit(GArray *table_data, BIOSLinker *linker);
 #endif

include/sysemu/numa.h +1 -0

index 8f09dcf..2f7a941 100644
@@ -21,6 +21,7 @@ typedef struct node_info {
     struct HostMemoryBackend *node_memdev;
     bool present;
     QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+    uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];

include/sysemu/sysemu.h +4 -0

index 576c7ce..a4e328d 100644
@@ -169,6 +169,10 @@ extern int mem_prealloc;
 
 #define MAX_NODES 128
 #define NUMA_NODE_UNASSIGNED MAX_NODES
+#define MIN_NUMA_DISTANCE 10
+#define DEF_NUMA_DISTANCE 20
+#define MAX_NUMA_DISTANCE 254
+#define NUMA_DISTANCE_UNREACHABLE 255
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

numa.c +0 -0

index e01cb54..425a320 100644

numa.c +21 -0

+        error_setg(errp, "Max number of NUMA nodes reached: %"
+                   PRIu64 "", src > dst ? src : dst);
+        return;
+    }
+
+    if (val < MIN_NUMA_DISTANCE || val > MAX_NUMA_DISTANCE) {
+        error_setg(errp,
+                "NUMA distance (%" PRIu8 ") out of range (%d) ~ (%d)",
+                dist->val, MIN_NUMA_DISTANCE, MAX_NUMA_DISTANCE);
+        return;
+    }
+
+    numa_info[src].distance[dst] = val;
+}
+
 static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;

@@ -235,6 +257,12 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         }
         nb_numa_nodes++;
         break;
+    case NUMA_OPTIONS_TYPE_DIST:
+        numa_distance_parse(&object->u.dist, opts, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }

numa.c +15 -0

+    for (src = 0; src < nb_numa_nodes; src++) {
+        for (dst = 0; dst < nb_numa_nodes; dst++) {
+            if (src == dst && numa_info[src].distance[dst] != MIN_NUMA_DISTANCE) {
+                numa_info[src].distance[dst] = MIN_NUMA_DISTANCE;
+            } else if (numa_info[src].distance[dst] <= MIN_NUMA_DISTANCE) {
+                if (numa_info[dst].distance[src] > MIN_NUMA_DISTANCE)
+                    numa_info[src].distance[dst] = numa_info[dst].distance[src];
+                else
+                    numa_info[src].distance[dst] = DEF_NUMA_DISTANCE;
+            }
+        }
+    }
+}
+
 void parse_numa_opts(MachineClass *mc)
 {
     int i;

@@ -390,6 +436,7 @@ void parse_numa_opts(MachineClass *mc)
         }
 
         validate_numa_cpus();
+        default_numa_distance();
     } else {
         numa_set_mem_node_id(0, ram_size, 0);
     }

qapi-schema.json +4 -0

index 32b4a4b..21ad94a 100644
@@ -5644,15 +5644,19 @@
 ##
 # @NumaOptionsType:
 #
+# @node: NUMA nodes configuration
+#
+# @dist: NUMA distance configuration
+#
 # Since: 2.1

qapi-schema.json +8 -1

 #
 # Since: 2.1
 ##

@@ -5660,7 +5664,8 @@
   'base': { 'type': 'NumaOptionsType' },
   'discriminator': 'type',
   'data': {
-    'node': 'NumaNodeOptions' }}
+    'node': 'NumaNodeOptions',
+    'dist': 'NumaDistOptions' }}
 
 ##
 # @NumaNodeOptions:

@@ -5689,6 +5694,25 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaDistOptions:
+#
+# Set distance between 2 NUMA nodes. (for OptsVisitor)
+#
+# @src: source NUMA node.
+#

qapi-schema.json +3 -0

+   'val': 'uint8' }}
+
+##
 # @HostMemPolicy:
 #
 # Host memory policy types

qemu-options.hx +11 -1

index 8dd8ee3..43c3950 100644
@@ -139,12 +139,15 @@ ETEXI
 
 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", QEMU_ARCH_ALL)
+    "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
+    "-numa dist,src=source,dst=destination,val=distance\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}
 @findex -numa
 Define a NUMA node and assign RAM and VCPUs to it.
+Set NUMA distance from source node to destination node.
 
 @var{firstcpu} and @var{lastcpu} are CPU indexes. Each
 @samp{cpus} option represent a contiguous range of CPU indexes

@@ -167,6 +170,13 @@ split equally between them.
 @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
 if one node uses @samp{memdev}, all of them have to use it.
 
+@var{source} and @var{destination} are NUMA node IDs.
+@var{distance} is the NUMA distance from @var{source} to @var{destination}.
+The distance from node A to node B may be different from the distance from
+node B to node A since the distance can to be asymmetry.
+If the distance is not set, the default distance for a local NUMA node is 10,
+and 20 for a remote node.
+
 Note that the -@option{numa} option doesn't allocate any of the
 specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},

Quoted 182 lines · On Wed, 22 Mar 2017 17:32:46 +0800 He Chen <[email protected]> wrote:

Current, QEMU does not provide a clear command to set vNUMA distance for guest although we already have -numa command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA info via numactl -H, we will see:

node distance:
node 0 1 2 3

  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node as distance 20 when there is no SLIT table since QEMU doesn't build it. It looks like a little strange when you have seen the distance in an actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node 0 1 2 3

  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table. I found QEMU has provide -acpitable command that allows users to add a ACPI table into guest, but it requires users building ACPI table by themselves first. Using -acpitable to add a SLIT table may be not so straightforward or flexible, imagine that when the vNUMA configuration is changes and we need to generate another SLIT table manually. It may not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides additional option dist for command -numa to allow user set vNUMA distance by QEMU command.

With this patch, when a user wants to create a guest that contains several vNUMA nodes and also wants to set distance among those nodes, the QEMU command would like:

```

(quoted diff) +0 -12

-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...

```

Signed-off-by He Chen <[email protected]>

fix

---
 hw/acpi/aml-build.c         | 26 +++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    |  2 ++
 hw/i386/acpi-build.c        |  2 ++
 include/hw/acpi/aml-build.h |  1 +
 include/sysemu/numa.h       |  1 +
 include/sysemu/sysemu.h     |  4 ++++
 numa.c                      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json            | 30 ++++++++++++++++++++++++++---
 qemu-options.hx             | 12 +++++++++++-
 9 files changed, 121 insertions(+), 4 deletions(-)

hw/acpi/aml-build.c +26 -0

index c6f2032..410b30e 100644
@@ -24,6 +24,7 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
+#include "sysemu/numa.h"
 
 static GArray *build_alloc_array(void)
 {

@@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }
+
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+void build_slit(GArray *table_data, BIOSLinker *linker)
+{
+    int slit_start, i, j;
+    slit_start = table_data->len;
+
+    acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+    build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+        }
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + slit_start),
+                 "SLIT",
+                 table_data->len - slit_start, 1, NULL, NULL);
+}
+

hw/arm/virt-acpi-build.c +2 -0

index 0835e59..d9e6828 100644
@@ -781,6 +781,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     if (nb_numa_nodes > 0) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, vms);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
 
     if (its_class_name() && !vmc->no_its) {

hw/i386/acpi-build.c +2 -0

index 2073108..12730ea 100644
@@ -2678,6 +2678,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (pcms->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/aml-build.h +1 -0

index 00c21f1..329a0d0 100644
@@ -389,4 +389,5 @@ GCC_FMT_ATTR(2, 3);
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_slit(GArray *table_data, BIOSLinker *linker);
 #endif

include/sysemu/numa.h +1 -0

index 8f09dcf..2f7a941 100644
@@ -21,6 +21,7 @@ typedef struct node_info {
     struct HostMemoryBackend *node_memdev;
     bool present;
     QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+    uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];

include/sysemu/sysemu.h +4 -0

index 576c7ce..a4e328d 100644
@@ -169,6 +169,10 @@ extern int mem_prealloc;
 
 #define MAX_NODES 128
 #define NUMA_NODE_UNASSIGNED MAX_NODES
+#define MIN_NUMA_DISTANCE 10
+#define DEF_NUMA_DISTANCE 20
+#define MAX_NUMA_DISTANCE 254
+#define NUMA_DISTANCE_UNREACHABLE 255
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

numa.c +0 -0

index e01cb54..425a320 100644

numa.c static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) · line 221

Quoted 10 lines

numa.c +7 -0

@@ -212,6 +212,28 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 
+static void numa_distance_parse(NumaDistOptions *dist, QemuOpts *opts, Error **errp)
+{
+    uint64_t src = dist->src;
+    uint64_t dst = dist->dst;
+    uint8_t val = dist->val;
+
+    if (src >= MAX_NODES || dst >= MAX_NODES) {

this allows to specify any src/dst instead of present nodes only, you should probably check that src/dst are present here,
see: numa_info[x].present

if node is not present complain that it should be declared with '-numa node' option first and exit.

Quoted 35 lines

numa.c +21 -0

+        error_setg(errp, "Max number of NUMA nodes reached: %"
+                   PRIu64 "", src > dst ? src : dst);
+        return;
+    }
+
+    if (val < MIN_NUMA_DISTANCE || val > MAX_NUMA_DISTANCE) {
+        error_setg(errp,
+                "NUMA distance (%" PRIu8 ") out of range (%d) ~ (%d)",
+                dist->val, MIN_NUMA_DISTANCE, MAX_NUMA_DISTANCE);
+        return;
+    }
+
+    numa_info[src].distance[dst] = val;
+}
+
 static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;

@@ -235,6 +257,12 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         }
         nb_numa_nodes++;
         break;
+    case NUMA_OPTIONS_TYPE_DIST:
+        numa_distance_parse(&object->u.dist, opts, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }

@@ -294,6 +322,24 @@ static void validate_numa_cpus(void)
     g_free(seen_cpus);
 }
 

numa.c static void validate_numa_cpus(void) · line 328

Quoted 4 lines

numa.c +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

Quoted 38 lines

numa.c +15 -0

+    for (src = 0; src < nb_numa_nodes; src++) {
+        for (dst = 0; dst < nb_numa_nodes; dst++) {
+            if (src == dst && numa_info[src].distance[dst] != MIN_NUMA_DISTANCE) {
+                numa_info[src].distance[dst] = MIN_NUMA_DISTANCE;
+            } else if (numa_info[src].distance[dst] <= MIN_NUMA_DISTANCE) {
+                if (numa_info[dst].distance[src] > MIN_NUMA_DISTANCE)
+                    numa_info[src].distance[dst] = numa_info[dst].distance[src];
+                else
+                    numa_info[src].distance[dst] = DEF_NUMA_DISTANCE;
+            }
+        }
+    }
+}
+
 void parse_numa_opts(MachineClass *mc)
 {
     int i;

@@ -390,6 +436,7 @@ void parse_numa_opts(MachineClass *mc)
         }
 
         validate_numa_cpus();
+        default_numa_distance();
     } else {
         numa_set_mem_node_id(0, ram_size, 0);
     }

qapi-schema.json +4 -0

index 32b4a4b..21ad94a 100644
@@ -5644,15 +5644,19 @@
 ##
 # @NumaOptionsType:
 #
+# @node: NUMA nodes configuration
+#
+# @dist: NUMA distance configuration
+#
 # Since: 2.1

qapi-schema.json line 5659

Quoted 10 lines

qapi-schema.json +2 -2

 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node' ] }
+  'data': [ 'node', 'dist' ] }
 
 ##
 # @NumaOptions:
 #
-# A discriminated record of NUMA options. (for OptsVisitor)
+# A discriminated record of NUMA options.

unrelated change?

Quoted 23 lines

qapi-schema.json +8 -1

 #
 # Since: 2.1
 ##

@@ -5660,7 +5664,8 @@
   'base': { 'type': 'NumaOptionsType' },
   'discriminator': 'type',
   'data': {
-    'node': 'NumaNodeOptions' }}
+    'node': 'NumaNodeOptions',
+    'dist': 'NumaDistOptions' }}
 
 ##
 # @NumaNodeOptions:

@@ -5689,6 +5694,25 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaDistOptions:
+#
+# Set distance between 2 NUMA nodes. (for OptsVisitor)
+#
+# @src: source NUMA node.
+#

qapi-schema.json line 5712

Quoted 10 lines

qapi-schema.json +10 -0

+# @dst: destination NUMA node.
+#
+# @val: NUMA distance from source node to destination node.
+#
+# Since: 2.10
+##
+{ 'struct': 'NumaDistOptions',
+  'data': {
+   'src': 'uint64',
+   'dst': 'uint64',

nodeid is uint16, so probably these should be also uint16

Quoted 41 lines

qapi-schema.json +3 -0

+   'val': 'uint8' }}
+
+##
 # @HostMemPolicy:
 #
 # Host memory policy types

qemu-options.hx +11 -1

index 8dd8ee3..43c3950 100644
@@ -139,12 +139,15 @@ ETEXI
 
 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", QEMU_ARCH_ALL)
+    "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
+    "-numa dist,src=source,dst=destination,val=distance\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}
 @findex -numa
 Define a NUMA node and assign RAM and VCPUs to it.
+Set NUMA distance from source node to destination node.
 
 @var{firstcpu} and @var{lastcpu} are CPU indexes. Each
 @samp{cpus} option represent a contiguous range of CPU indexes

@@ -167,6 +170,13 @@ split equally between them.
 @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
 if one node uses @samp{memdev}, all of them have to use it.
 
+@var{source} and @var{destination} are NUMA node IDs.
+@var{distance} is the NUMA distance from @var{source} to @var{destination}.
+The distance from node A to node B may be different from the distance from
+node B to node A since the distance can to be asymmetry.
+If the distance is not set, the default distance for a local NUMA node is 10,
+and 20 for a remote node.
+
 Note that the -@option{numa} option doesn't allocate any of the
 specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},

Quoted 199 lines · Igor Mammedov

Quoted 182 lines · On Wed, 22 Mar 2017 17:32:46 +0800 He Chen <[email protected]> wrote:

Current, QEMU does not provide a clear command to set vNUMA distance for guest although we already have -numa command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA info via numactl -H, we will see:

node distance:
node 0 1 2 3

  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node as distance 20 when there is no SLIT table since QEMU doesn't build it. It looks like a little strange when you have seen the distance in an actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node 0 1 2 3

  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table. I found QEMU has provide -acpitable command that allows users to add a ACPI table into guest, but it requires users building ACPI table by themselves first. Using -acpitable to add a SLIT table may be not so straightforward or flexible, imagine that when the vNUMA configuration is changes and we need to generate another SLIT table manually. It may not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides additional option dist for command -numa to allow user set vNUMA distance by QEMU command.

With this patch, when a user wants to create a guest that contains several vNUMA nodes and also wants to set distance among those nodes, the QEMU command would like:

```

(quoted diff) +0 -12

-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...

```

Signed-off-by He Chen <[email protected]>

fix

---
 hw/acpi/aml-build.c         | 26 +++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    |  2 ++
 hw/i386/acpi-build.c        |  2 ++
 include/hw/acpi/aml-build.h |  1 +
 include/sysemu/numa.h       |  1 +
 include/sysemu/sysemu.h     |  4 ++++
 numa.c                      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json            | 30 ++++++++++++++++++++++++++---
 qemu-options.hx             | 12 +++++++++++-
 9 files changed, 121 insertions(+), 4 deletions(-)

hw/acpi/aml-build.c +26 -0

index c6f2032..410b30e 100644
@@ -24,6 +24,7 @@
 #include "hw/acpi/aml-build.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
+#include "sysemu/numa.h"
 
 static GArray *build_alloc_array(void)
 {

@@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }
+
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+void build_slit(GArray *table_data, BIOSLinker *linker)
+{
+    int slit_start, i, j;
+    slit_start = table_data->len;
+
+    acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+    build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        for (j = 0; j < nb_numa_nodes; j++) {
+            build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+        }
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + slit_start),
+                 "SLIT",
+                 table_data->len - slit_start, 1, NULL, NULL);
+}
+

hw/arm/virt-acpi-build.c +2 -0

index 0835e59..d9e6828 100644
@@ -781,6 +781,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     if (nb_numa_nodes > 0) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, vms);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
 
     if (its_class_name() && !vmc->no_its) {

hw/i386/acpi-build.c +2 -0

index 2073108..12730ea 100644
@@ -2678,6 +2678,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (pcms->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
+        acpi_add_table(table_offsets, tables_blob);
+        build_slit(tables_blob, tables->linker);
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/aml-build.h +1 -0

index 00c21f1..329a0d0 100644
@@ -389,4 +389,5 @@ GCC_FMT_ATTR(2, 3);
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
+void build_slit(GArray *table_data, BIOSLinker *linker);
 #endif

include/sysemu/numa.h +1 -0

index 8f09dcf..2f7a941 100644
@@ -21,6 +21,7 @@ typedef struct node_info {
     struct HostMemoryBackend *node_memdev;
     bool present;
     QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+    uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];

include/sysemu/sysemu.h +4 -0

index 576c7ce..a4e328d 100644
@@ -169,6 +169,10 @@ extern int mem_prealloc;
 
 #define MAX_NODES 128
 #define NUMA_NODE_UNASSIGNED MAX_NODES
+#define MIN_NUMA_DISTANCE 10
+#define DEF_NUMA_DISTANCE 20
+#define MAX_NUMA_DISTANCE 254
+#define NUMA_DISTANCE_UNREACHABLE 255
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

numa.c +0 -0

index e01cb54..425a320 100644

numa.c static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp) · line 221

Quoted 10 lines

numa.c +7 -0

@@ -212,6 +212,28 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 
+static void numa_distance_parse(NumaDistOptions *dist, QemuOpts *opts, Error **errp)
+{
+    uint64_t src = dist->src;
+    uint64_t dst = dist->dst;
+    uint8_t val = dist->val;
+
+    if (src >= MAX_NODES || dst >= MAX_NODES) {  

this allows to specify any src/dst instead of present nodes only, you should probably check that src/dst are present here,
see: numa_info[x].present

Quoted 3 lines

if node is not present complain that it should be declared with '-numa node' option first and exit.

Thanks for pointing out this.

Quoted 35 lines

Quoted 35 lines

numa.c +21 -0

+        error_setg(errp, "Max number of NUMA nodes reached: %"
+                   PRIu64 "", src > dst ? src : dst);
+        return;
+    }
+
+    if (val < MIN_NUMA_DISTANCE || val > MAX_NUMA_DISTANCE) {
+        error_setg(errp,
+                "NUMA distance (%" PRIu8 ") out of range (%d) ~ (%d)",
+                dist->val, MIN_NUMA_DISTANCE, MAX_NUMA_DISTANCE);
+        return;
+    }
+
+    numa_info[src].distance[dst] = val;
+}
+
 static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;

@@ -235,6 +257,12 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         }
         nb_numa_nodes++;
         break;
+    case NUMA_OPTIONS_TYPE_DIST:
+        numa_distance_parse(&object->u.dist, opts, &err);
+        if (err) {
+            goto end;
+        }
+        break;
     default:
         abort();
     }

@@ -294,6 +322,24 @@ static void validate_numa_cpus(void)
     g_free(seen_cpus);
 }
 

Quoted 7 lines

numa.c static void validate_numa_cpus(void) · line 328

Quoted 4 lines

numa.c +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+  

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

I am afraid that I may have a bad function name here, fill_numa_distance might be a better name?

Also, since the distance can be asymmetric, IMHO, providing a partial filled table looks fine. If we set many NUMA nodes e.g. 8 nodes for guest, the whole filled table would require many command lines which might be inconvenient and less flexible.

Please correct me if I am wrong, thanks.

Quoted 4 lines

Quoted 4 lines

numa.c +4 -0

+    for (src = 0; src < nb_numa_nodes; src++) {
+        for (dst = 0; dst < nb_numa_nodes; dst++) {
+            if (src == dst && numa_info[src].distance[dst] != MIN_NUMA_DISTANCE) {
+                numa_info[src].distance[dst] = MIN_NUMA_DISTANCE;

Quoted 124 lines

Quoted 34 lines

numa.c +11 -0

+            } else if (numa_info[src].distance[dst] <= MIN_NUMA_DISTANCE) {
+                if (numa_info[dst].distance[src] > MIN_NUMA_DISTANCE)
+                    numa_info[src].distance[dst] = numa_info[dst].distance[src];
+                else
+                    numa_info[src].distance[dst] = DEF_NUMA_DISTANCE;
+            }
+        }
+    }
+}
+
 void parse_numa_opts(MachineClass *mc)
 {
     int i;

@@ -390,6 +436,7 @@ void parse_numa_opts(MachineClass *mc)
         }
 
         validate_numa_cpus();
+        default_numa_distance();
     } else {
         numa_set_mem_node_id(0, ram_size, 0);
     }

qapi-schema.json +4 -0

index 32b4a4b..21ad94a 100644
@@ -5644,15 +5644,19 @@
 ##
 # @NumaOptionsType:
 #
+# @node: NUMA nodes configuration
+#
+# @dist: NUMA distance configuration
+#
 # Since: 2.1

qapi-schema.json line 5659

Quoted 10 lines

qapi-schema.json +2 -2

 ##
 { 'enum': 'NumaOptionsType',
-  'data': [ 'node' ] }
+  'data': [ 'node', 'dist' ] }
 
 ##
 # @NumaOptions:
 #
-# A discriminated record of NUMA options. (for OptsVisitor)
+# A discriminated record of NUMA options.  

unrelated change?

Quoted 23 lines

qapi-schema.json +8 -1

 #
 # Since: 2.1
 ##

@@ -5660,7 +5664,8 @@
   'base': { 'type': 'NumaOptionsType' },
   'discriminator': 'type',
   'data': {
-    'node': 'NumaNodeOptions' }}
+    'node': 'NumaNodeOptions',
+    'dist': 'NumaDistOptions' }}
 
 ##
 # @NumaNodeOptions:

@@ -5689,6 +5694,25 @@
    '*memdev': 'str' }}
 
 ##
+# @NumaDistOptions:
+#
+# Set distance between 2 NUMA nodes. (for OptsVisitor)
+#
+# @src: source NUMA node.
+#

qapi-schema.json line 5712

Quoted 10 lines

qapi-schema.json +10 -0

+# @dst: destination NUMA node.
+#
+# @val: NUMA distance from source node to destination node.
+#
+# Since: 2.10
+##
+{ 'struct': 'NumaDistOptions',
+  'data': {
+   'src': 'uint64',
+   'dst': 'uint64',  

nodeid is uint16, so probably these should be also uint16

Quoted 41 lines

qapi-schema.json +3 -0

+   'val': 'uint8' }}
+
+##
 # @HostMemPolicy:
 #
 # Host memory policy types

qemu-options.hx +11 -1

index 8dd8ee3..43c3950 100644
@@ -139,12 +139,15 @@ ETEXI
 
 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", QEMU_ARCH_ALL)
+    "-numa node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node]\n"
+    "-numa dist,src=source,dst=destination,val=distance\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}
 @findex -numa
 Define a NUMA node and assign RAM and VCPUs to it.
+Set NUMA distance from source node to destination node.
 
 @var{firstcpu} and @var{lastcpu} are CPU indexes. Each
 @samp{cpus} option represent a contiguous range of CPU indexes

@@ -167,6 +170,13 @@ split equally between them.
 @samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
 if one node uses @samp{memdev}, all of them have to use it.
 
+@var{source} and @var{destination} are NUMA node IDs.
+@var{distance} is the NUMA distance from @var{source} to @var{destination}.
+The distance from node A to node B may be different from the distance from
+node B to node A since the distance can to be asymmetry.
+If the distance is not set, the default distance for a local NUMA node is 10,
+and 20 for a remote node.
+
 Note that the -@option{numa} option doesn't allocate any of the
 specified resources, it just assigns existing resources to NUMA
 nodes. This means that one still has to use the @option{-m},  

Quoted 10 lines · On Wed, 29 Mar 2017 16:50:00 +0800 He Chen <[email protected]> wrote:

Quoted 7 lines

numa.c static void validate_numa_cpus(void) · line 328

Quoted 4 lines

numa.c +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+  

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

I am afraid that I may have a bad function name here, fill_numa_distance might be a better name?

Quoted 4 lines

Also, since the distance can be asymmetric, IMHO, providing a partial filled table looks fine. If we set many NUMA nodes e.g. 8 nodes for guest, the whole filled table would require many command lines which might be inconvenient and less flexible.

asymmetric doesn't imply sparse, so one has to specify full matrix it might be inconvenient /long/ but is very flexible.

Quoted 12 lines · Igor Mammedov

Quoted 10 lines · On Wed, 29 Mar 2017 16:50:00 +0800 He Chen <[email protected]> wrote:

Quoted 7 lines

numa.c static void validate_numa_cpus(void) · line 328

Quoted 4 lines

numa.c +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+  

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

I am afraid that I may have a bad function name here, fill_numa_distance might be a better name?

Quoted 6 lines

Quoted 4 lines

Also, since the distance can be asymmetric, IMHO, providing a partial filled table looks fine. If we set many NUMA nodes e.g. 8 nodes for guest, the whole filled table would require many command lines which might be inconvenient and less flexible.

asymmetric doesn't imply sparse, so one has to specify full matrix it might be inconvenient /long/ but is very flexible.

This makes me realize that a user only inputting one of A -> B or B -> A command line inputs doesn't imply symmetry. It could be that the user just forgot to input the opposite. To avoid the ambiguity we either need to force both to be input (as it was before I suggested otherwise), or add a '-numa symmetric' type of property to enable the shortcut. I guess we should just avoid the shortcut, at least for now.

Quoted 20 lines · Andrew Jones

Quoted 12 lines · Igor Mammedov

Quoted 10 lines · On Wed, 29 Mar 2017 16:50:00 +0800 He Chen <[email protected]> wrote:

Quoted 7 lines

numa.c static void validate_numa_cpus(void) · line 328

Quoted 4 lines

numa.c +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+  

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

I am afraid that I may have a bad function name here, fill_numa_distance might be a better name?

Quoted 6 lines

Quoted 4 lines

Also, since the distance can be asymmetric, IMHO, providing a partial filled table looks fine. If we set many NUMA nodes e.g. 8 nodes for guest, the whole filled table would require many command lines which might be inconvenient and less flexible.

asymmetric doesn't imply sparse, so one has to specify full matrix it might be inconvenient /long/ but is very flexible.

Quoted 6 lines

This makes me realize that a user only inputting one of A -> B or B -> A command line inputs doesn't imply symmetry. It could be that the user just forgot to input the opposite. To avoid the ambiguity we either need to force both to be input (as it was before I suggested otherwise), or add a '-numa symmetric' type of property to enable the shortcut. I guess we should just avoid the shortcut, at least for now.

Is protecting the user from one very specific (and very rare[1]) mistake a good reason for making the automatic default less useful?

Requiring an explicit '-numa symmetric' option to enable the automatic default seems to defeat the purpose of having an automatic default, to me.

Quoted 28 lines · Eduardo Habkost

Quoted 20 lines · Andrew Jones

Quoted 12 lines · Igor Mammedov

Quoted 10 lines · On Wed, 29 Mar 2017 16:50:00 +0800 He Chen <[email protected]> wrote:

Quoted 7 lines

Review

Quoted 4 lines

(quoted diff) +4 -0

+static void default_numa_distance(void)
+{
+    int src, dst;
+  

fill in defaults only if there weren't any '-numa dist' on CLI and refuse to start if partial filled table were explicitly provided on CLI

I am afraid that I may have a bad function name here, fill_numa_distance might be a better name?

Quoted 6 lines

Quoted 4 lines

Also, since the distance can be asymmetric, IMHO, providing a partial filled table looks fine. If we set many NUMA nodes e.g. 8 nodes for guest, the whole filled table would require many command lines which might be inconvenient and less flexible.

asymmetric doesn't imply sparse, so one has to specify full matrix it might be inconvenient /long/ but is very flexible.

Quoted 6 lines

This makes me realize that a user only inputting one of A -> B or B -> A command line inputs doesn't imply symmetry. It could be that the user just forgot to input the opposite. To avoid the ambiguity we either need to force both to be input (as it was before I suggested otherwise), or add a '-numa symmetric' type of property to enable the shortcut. I guess we should just avoid the shortcut, at least for now.

Quoted 3 lines

Is protecting the user from one very specific (and very rare[1]) mistake a good reason for making the automatic default less useful?

Maybe not, but creating new interfaces with known ambiguities isn't ideal either.

Quoted 3 lines

Requiring an explicit '-numa symmetric' option to enable the automatic default seems to defeat the purpose of having an automatic default, to me.

We could reverse it, '-numa asymmetric' could turn off the defaults and abort on an incomplete table.

Or, we could leave it ambiguous, but improve the heuristic by requiring all directions be given if even one direction has an asymmetric reverse path given. With that, there's still a chance to let user error slip through, but much less. I could live with that.