diff mbox

[RFC,09/16] hw/i386/pc: don't use smp_cores, smp_threads

Message ID 1465580427-13596-10-git-send-email-drjones@redhat.com
State New
Headers show

Commit Message

Andrew Jones June 10, 2016, 5:40 p.m. UTC 
Use CPUState nr_cores,nr_threads and MachineState
cores,threads instead.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/i386/pc.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

Comments

Eduardo Habkost July 14, 2016, 8:33 p.m. UTC | #1 
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.
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed533cc47..4fa86d6387ce9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -27,7 +27,6 @@ 
 #include "hw/char/serial.h"
 #include "hw/i386/apic.h"
 #include "hw/i386/topology.h"
-#include "sysemu/cpus.h"
 #include "hw/block/fdc.h"
 #include "hw/ide.h"
 #include "hw/pci/pci.h"
@@ -682,12 +681,14 @@  void enable_compat_apic_id_mode(void)
  * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of
  * all CPUs up to max_cpus.
  */
-static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
+static uint32_t x86_cpu_apic_id_from_index(MachineState *ms,
+                                           unsigned int cpu_index)
 {
     uint32_t correct_id;
     static bool warned;
 
-    correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index);
+    correct_id = x86_apicid_from_cpu_idx(ms->cores, ms->threads,
+                                         cpu_index);
     if (compat_apic_id_mode) {
         if (cpu_index != correct_id && !warned && !qtest_enabled()) {
             error_report("APIC IDs set in compatibility mode, "
@@ -778,7 +779,7 @@  static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
     numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
     numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
     for (i = 0; i < max_cpus; i++) {
-        unsigned int apic_id = x86_cpu_apic_id_from_index(i);
+        unsigned int apic_id = x86_cpu_apic_id_from_index(MACHINE(pcms), i);
         assert(apic_id < pcms->apic_id_limit);
         for (j = 0; j < nb_numa_nodes; j++) {
             if (test_bit(i, numa_info[j].node_cpu)) {
@@ -1066,7 +1067,7 @@  void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
     X86CPU *cpu;
     MachineState *machine = MACHINE(qdev_get_machine());
-    int64_t apic_id = x86_cpu_apic_id_from_index(id);
+    int64_t apic_id = x86_cpu_apic_id_from_index(machine, id);
     Error *local_err = NULL;
 
     if (id < 0) {
@@ -1123,7 +1124,7 @@  void pc_cpus_init(PCMachineState *pcms)
      *
      * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
      */
-    pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
+    pcms->apic_id_limit = x86_cpu_apic_id_from_index(machine, max_cpus - 1) + 1;
     if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
         error_report("max_cpus is too large. APIC ID of last CPU is %u",
                      pcms->apic_id_limit - 1);
@@ -1133,10 +1134,12 @@  void pc_cpus_init(PCMachineState *pcms)
     pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
                                     sizeof(CPUArchId) * max_cpus);
     for (i = 0; i < max_cpus; i++) {
-        pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+        pcms->possible_cpus->cpus[i].arch_id =
+                x86_cpu_apic_id_from_index(machine, i);
         pcms->possible_cpus->len++;
         if (i < smp_cpus) {
-            cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
+            cpu = pc_new_cpu(machine->cpu_model,
+                             x86_cpu_apic_id_from_index(machine, i),
                              &error_fatal);
             pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
             object_unref(OBJECT(cpu));
@@ -1193,7 +1196,7 @@  void pc_guest_info_init(PCMachineState *pcms)
                                      sizeof *pcms->node_cpu);
 
     for (i = 0; i < max_cpus; i++) {
-        unsigned int apic_id = x86_cpu_apic_id_from_index(i);
+        unsigned int apic_id = x86_cpu_apic_id_from_index(MACHINE(pcms), i);
         assert(apic_id < pcms->apic_id_limit);
         for (j = 0; j < nb_numa_nodes; j++) {
             if (test_bit(i, numa_info[j].node_cpu)) {
@@ -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;
     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);
     return topo.pkg_id;
 }