On 10/10/19 11:10 PM, Eduardo Habkost wrote: > 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. Simplified apic id decoding in v3 version. We don't need these changes anymore. > >> >> 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); >> >> >