On Fri, Sep 06, 2019 at 07:13:29PM +0000, Moger, Babu wrote:
> Current topology id match will not work for epyc mode when setting
> the node id. In epyc mode, ids like smt_id, thread_id, core_id,
> ccx_id, socket_id can be same for more than one CPUs with across
> two numa nodes.
>
> For example, we can have two CPUs with following ids on two different node.
> 1. smt_id=0, thread_id=0, core_id=0, ccx_id=0, socket_id=0, node_id=0
> 2. smt_id=0, thread_id=0, core_id=0, ccx_id=0, socket_id=0, node_id=1
I don't understand how that could happen. If all IDs are the
same, how is it possible that those two cases should match two
different CPUs?
cpu_index_to_instance_props() must always return an unique
identifier for a single CPU.
>
> The function machine_set_cpu_numa_node will fail to find a match to assign
> the node. Added new function machine_set_cpu_numa_node_epyc to set the node_id
> directly in epyc mode.
>
> Signed-off-by: Babu Moger <babu.mo...@amd.com>
> ---
> hw/core/machine.c | 24 ++++++++++++++++++++++++
> hw/core/numa.c | 6 +++++-
> include/hw/boards.h | 4 ++++
> 3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 9a8586cf30..6bceefc6f3 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -741,6 +741,30 @@ void machine_set_cpu_numa_node(MachineState *machine,
> }
> }
>
> +void machine_set_cpu_numa_node_epyc(MachineState *machine,
> + const CpuInstanceProperties *props,
> + unsigned index,
> + Error **errp)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(machine);
> + CPUArchId *slot;
> +
> + if (!mc->possible_cpu_arch_ids) {
> + error_setg(errp, "mapping of CPUs to NUMA node is not supported");
> + return;
> + }
> +
> + /* disabling node mapping is not supported, forbid it */
> + assert(props->has_node_id);
> +
> + /* force board to initialize possible_cpus if it hasn't been done yet */
> + mc->possible_cpu_arch_ids(machine);
> +
> + slot = &machine->possible_cpus->cpus[index];
> + slot->props.node_id = props->node_id;
> + slot->props.has_node_id = props->has_node_id;
> +}
> +
> static void smp_parse(MachineState *ms, QemuOpts *opts)
> {
> if (opts) {
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 27fa6b5e1d..a9e835aea6 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -247,7 +247,11 @@ void set_numa_node_options(MachineState *ms, NumaOptions
> *object, Error **errp)
> props = mc->cpu_index_to_instance_props(ms, cpus->value);
> props.node_id = nodenr;
> props.has_node_id = true;
> - machine_set_cpu_numa_node(ms, &props, &err);
> + if (ms->epyc) {
> + machine_set_cpu_numa_node_epyc(ms, &props, cpus->value, &err);
> + } else {
> + machine_set_cpu_numa_node(ms, &props, &err);
> + }
> if (err) {
> error_propagate(errp, err);
> return;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 0001d42e50..ec1b1c5a85 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -74,6 +74,10 @@ HotpluggableCPUList
> *machine_query_hotpluggable_cpus(MachineState *machine);
> void machine_set_cpu_numa_node(MachineState *machine,
> const CpuInstanceProperties *props,
> Error **errp);
> +void machine_set_cpu_numa_node_epyc(MachineState *machine,
> + const CpuInstanceProperties *props,
> + unsigned index,
> + Error **errp);
>
> void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char
> *type);
>
>
--
Eduardo
