On Fri, Jun 10, 2016 at 07:40:20PM +0200, Andrew Jones wrote: [...] > @@ -1940,9 +1943,10 @@ static void pc_machine_reset(void) > > static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) > { > + CPUState *cs = first_cpu;
Eww. > X86CPUTopoInfo topo; > - x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index, > - &topo); > + > + x86_topo_ids_from_idx(cs->nr_cores, cs->nr_threads, cpu_index, &topo); If first_cpu is already available[1], then current_machine would also be available. Better to use the new MachineState fields you added, than the CPUState::nr_{cores,threads} fields that shouldn't have existed in the first place. [1] Is it really available? This function is called very early, from parse_numa_opts(). I think current_machine is already available, though. But we need to ensure parse_numa_opts() will be called only after SMP option parsing is done and SMP topology data in MachineState is already populated. Later, we could also make the NUMA option parsing not depend on SMP parsing to be done first. The cpu_index_to_socket_id() magic can be moved to numa_post_machine_init(), probably. -- Eduardo