| Message ID | 156779720803.21957.8389712174989601936.stgit@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | APIC ID fixes for AMD EPYC CPU models | expand |
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.moger@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); > >
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.moger@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); >> >> >
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);
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 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.moger@amd.com> --- hw/core/machine.c | 24 ++++++++++++++++++++++++ hw/core/numa.c | 6 +++++- include/hw/boards.h | 4 ++++ 3 files changed, 33 insertions(+), 1 deletion(-)