> -----Original Message-----
> From: Xu, Tao3
> Sent: Tuesday, July 16, 2019 10:51 PM
> To: imamm...@redhat.com; ebl...@redhat.com; ehabk...@redhat.com
> Cc: Xu, Tao3 <tao3...@intel.com>; Liu, Jingqi <jingqi....@intel.com>;
> Williams, Dan J <dan.j.willi...@intel.com>;
> jonathan.came...@huawei.com; Du, Fan <fan...@intel.com>; qemu-devel@nongnu.org
> Subject: [PATCH v7 05/11] numa: Extend CLI to provide initiator information
> for numa nodes
>
> In ACPI 6.3 chapter 5.2.27 Heterogeneous Memory Attribute Table (HMAT), The
> initiator represents processor which access to memory. And
> in 5.2.27.3 Memory Proximity Domain Attributes Structure, the attached
> initiator is defined as where the memory controller responsible for
> a memory proximity domain. With attached initiator information, the topology
> of heterogeneous memory can be described.
>
> Extend CLI of "-numa node" option to indicate the initiator numa node-id.
> In the linux kernel, the codes in drivers/acpi/hmat/hmat.c parse and report
> the platform's HMAT tables.
>
> Suggested-by: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Tao Xu <tao3...@intel.com>
Looks good for me.
Reviewed-by: Jingqi Liu <jingqi....@intel.com>
Thanks
Jingqi Liu
> ---
>
> No changes in v7.
>
> Changes in v6:
> - Add the version designator (since 4.2) after @initiator (Eric)
> ---
> hw/core/machine.c | 24 ++++++++++++++++++++++++
> hw/core/numa.c | 13 +++++++++++++
> include/sysemu/numa.h | 3 +++
> qapi/machine.json | 6 +++++-
> qemu-options.hx | 27 +++++++++++++++++++++++----
> 5 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c index
> 4228bcd2a2..063cb7923c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -653,6 +653,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
> const CpuInstanceProperties *props, Error
> **errp) {
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> + NodeInfo *numa_info = machine->numa_state->nodes;
> bool match = false;
> int i;
>
> @@ -722,6 +723,16 @@ void machine_set_cpu_numa_node(MachineState *machine,
> match = true;
> slot->props.node_id = props->node_id;
> slot->props.has_node_id = props->has_node_id;
> +
> + if (numa_info[props->node_id].initiator_valid &&
> + (props->node_id != numa_info[props->node_id].initiator)) {
> + error_setg(errp, "The initiator of CPU NUMA node %" PRId64
> + " should be itself.", props->node_id);
> + return;
> + }
> + numa_info[props->node_id].initiator_valid = true;
> + numa_info[props->node_id].has_cpu = true;
> + numa_info[props->node_id].initiator = props->node_id;
> }
>
> if (!match) {
> @@ -1063,6 +1074,7 @@ static void machine_numa_finish_cpu_init(MachineState
> *machine)
> GString *s = g_string_new(NULL);
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> const CPUArchIdList *possible_cpus = mc->possible_cpu_arch_ids(machine);
> + NodeInfo *numa_info = machine->numa_state->nodes;
>
> assert(machine->numa_state->num_nodes);
> for (i = 0; i < possible_cpus->len; i++) { @@ -1096,6 +1108,18 @@ static
> void machine_numa_finish_cpu_init(MachineState *machine)
> machine_set_cpu_numa_node(machine, &props, &error_fatal);
> }
> }
> +
> + for (i = 0; i < machine->numa_state->num_nodes; i++) {
> + if (numa_info[i].initiator_valid &&
> + !numa_info[numa_info[i].initiator].has_cpu) {
> + error_report("The initiator-id %"PRIu16 " of NUMA node %d"
> + " does not exist.", numa_info[i].initiator, i);
> + error_printf("\n");
> +
> + exit(1);
> + }
> + }
> +
> if (s->len && !qtest_enabled()) {
> warn_report("CPU(s) not present in any NUMA nodes: %s",
> s->str);
> diff --git a/hw/core/numa.c b/hw/core/numa.c index 8fcbba05d6..cfb6339810
> 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -128,6 +128,19 @@ static void parse_numa_node(MachineState *ms,
> NumaNodeOptions *node,
> numa_info[nodenr].node_mem = object_property_get_uint(o, "size",
> NULL);
> numa_info[nodenr].node_memdev = MEMORY_BACKEND(o);
> }
> +
> + if (node->has_initiator) {
> + if (numa_info[nodenr].initiator_valid &&
> + (node->initiator != numa_info[nodenr].initiator)) {
> + error_setg(errp, "The initiator of NUMA node %" PRIu16 " has
> been "
> + "set to node %" PRIu16, nodenr,
> + numa_info[nodenr].initiator);
> + return;
> + }
> +
> + numa_info[nodenr].initiator_valid = true;
> + numa_info[nodenr].initiator = node->initiator;
> + }
> numa_info[nodenr].present = true;
> max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
> ms->numa_state->num_nodes++;
> diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index
> 76da3016db..46ad06e000 100644
> --- a/include/sysemu/numa.h
> +++ b/include/sysemu/numa.h
> @@ -10,6 +10,9 @@ struct NodeInfo {
> uint64_t node_mem;
> struct HostMemoryBackend *node_memdev;
> bool present;
> + bool has_cpu;
> + bool initiator_valid;
> + uint16_t initiator;
> uint8_t distance[MAX_NODES];
> };
>
> diff --git a/qapi/machine.json b/qapi/machine.json index
> 6db8a7e2ec..05e367d26a 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -414,6 +414,9 @@
> # @memdev: memory backend object. If specified for one node,
> # it must be specified for all nodes.
> #
> +# @initiator: the initiator numa nodeid that is closest (as in directly
> +# attached) to this numa node (since 4.2)
> +#
> # Since: 2.1
> ##
> { 'struct': 'NumaNodeOptions',
> @@ -421,7 +424,8 @@
> '*nodeid': 'uint16',
> '*cpus': ['uint16'],
> '*mem': 'size',
> - '*memdev': 'str' }}
> + '*memdev': 'str',
> + '*initiator': 'uint16' }}
>
> ##
> # @NumaDistOptions:
> diff --git a/qemu-options.hx b/qemu-options.hx index 9621e934c0..c480781992
> 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -161,14 +161,14 @@ If any on the three values is given, the total number
> of CPUs @var{n} can be omi 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"
> + "-numa
> node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=node]\n"
> + "-numa
> node[,memdev=id][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=node]\n"
> "-numa dist,src=source,dst=destination,val=distance\n"
> "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n",
> 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}]
> +@item -numa
> +node[,mem=@var{size}][,cpus=@var{firstcpu}[-@var{lastcpu}]][,nodeid=@va
> +r{node}][,initiator=@var{initiator}]
> +@itemx -numa
> +node[,memdev=@var{id}][,cpus=@var{firstcpu}[-@var{lastcpu}]][,nodeid=@v
> +ar{node}][,initiator=@var{initiator}]
> @itemx -numa dist,src=@var{source},dst=@var{destination},val=@var{distance}
> @itemx -numa
> cpu,node-id=@var{node}[,socket-id=@var{x}][,core-id=@var{y}][,thread-id=@var{z}]
> @findex -numa
> @@ -215,6 +215,25 @@ 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.
>
> +@samp{initiator} indicate the initiator NUMA @var{initiator} that is
> +closest (as in directly attached) to this NUMA @var{node}.
> +
> +For example, the following option assigns 2 NUMA nodes, node 0 has CPU.
> +node 1 has only memory, and its' initiator is node 0. Note that because
> +node 0 has CPU, by default the initiator of node 0 is itself and must
> +be itself.
> +@example
> +-M pc \
> +-m 2G,slots=2,maxmem=4G \
> +-object memory-backend-ram,size=1G,id=m0 \ -object
> +memory-backend-ram,size=1G,id=m1 \ -numa node,nodeid=0,memdev=m0 \
> +-numa node,nodeid=1,memdev=m1,initiator=0 \ -smp 2,sockets=2,maxcpus=2
> +\ -numa cpu,node-id=0,socket-id=0 \ -numa cpu,node-id=0,socket-id=1 \
> +@end example
> +
> @var{source} and @var{destination} are NUMA node IDs.
> @var{distance} is the NUMA distance from @var{source} to @var{destination}.
> The distance from a node to itself is always 10. If any pair of nodes is
> --
> 2.20.1
