On 02/23/2018 06:36 PM, David Hildenbrand wrote: > Right now it is possible to crash QEMU for s390x by providing e.g. > -numa node,nodeid=0,cpus=0-1 > > Problem is, that numa.c uses mc->cpu_index_to_instance_props as an > indicator whether NUMA is supported by a machine type. We don't > implement NUMA on s390x (and that concept also doesn't really exist). > We need mc->cpu_index_to_instance_props for query-cpus.
Looks like we assert because of machine->possible_cpus == 0. Later during boot this is created in s390_possible_cpu_arch_ids. (via s390_init_cpus). What we (in the future) actually could provide is a cpu topology. So something like this also fixes the bug diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fd5bfcdaa5..d981335ca9 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "cpu.h" +#include "sysemu/numa.h" #include "hw/boards.h" #include "exec/address-spaces.h" #include "hw/s390x/s390-virtio-hcall.h" @@ -393,11 +394,20 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, static CpuInstanceProperties s390_cpu_index_to_props(MachineState *machine, unsigned cpu_index) { + MachineClass *mc = MACHINE_GET_CLASS(machine); + + /* make sure possible_cpu are intialized */ + mc->possible_cpu_arch_ids(machine); g_assert(machine->possible_cpus && cpu_index < machine->possible_cpus->len); return machine->possible_cpus->cpus[cpu_index].props; } +static int64_t s390_get_default_cpu_node_id(const MachineState *ms, int idx) +{ + return idx / smp_cpus % nb_numa_nodes; +} + static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms) { int i; @@ -473,6 +483,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) mc->get_hotplug_handler = s390_get_hotplug_handler; mc->cpu_index_to_instance_props = s390_cpu_index_to_props; mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids; + mc->get_default_cpu_node_id = s390_get_default_cpu_node_id; /* it is overridden with 'host' cpu *in kvm_arch_init* */ mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu"); hc->plug = s390_machine_device_plug; and it would allow us to extend things later on. On the other hand, my fix does not implement anything so your fix is "more correct". > > So let's fix this case. > > qemu-system-s390x: -numa node,nodeid=0,cpus=0-1: NUMA is not supported by > this machine-type > > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > numa.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/numa.c b/numa.c > index 7e0e789b02..3b9be613d9 100644 > --- a/numa.c > +++ b/numa.c > @@ -80,10 +80,16 @@ static void parse_numa_node(MachineState *ms, > NumaNodeOptions *node, > return; > } > > +#ifdef TARGET_S390X > + /* s390x provides cpu_index_to_instance_props but has no NUMA */ > + error_report("NUMA is not supported by this machine-type"); > + exit(1); > +#else > if (!mc->cpu_index_to_instance_props) { > error_report("NUMA is not supported by this machine-type"); > exit(1); > } > +#endif > for (cpus = node->cpus; cpus; cpus = cpus->next) { > CpuInstanceProperties props; > if (cpus->value >= max_cpus) { >