diff mbox

[v3,2/8] cpu: introduce possible-cpus interface

Message ID 1456243560-67516-3-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov Feb. 23, 2016, 4:05 p.m. UTC
on x86 currently range 0..max_cpus is used to generate
architecture-dependent CPU ID (APIC Id) for each present
and possible CPUs. However architecture-dependent CPU IDs
list could be sparse and code that needs to enumerate
all IDs (ACPI) ended up doing guess work enumerating all
possible and impossible IDs up to
  apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).

That leads to creation of MADT/SRAT entries and Processor
objects in ACPI tables for not possible CPUs.
Fix it by allowing board specify a concrete list of
CPU IDs accourding its own rules (which for x86 depends
on topology). So that code that needs this list could
request it from board instead of trying to figure out
what IDs are correct on its own.

This interface will also allow to help making AML
part of CPU hotplug target independent so it could
be reused for ARM target.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
CC: afaerber@suse.de
---
 hw/i386/pc.c         | 46 ++++++++++++++++++++++++++++++++++++++++++----
 include/hw/boards.h  |  1 +
 include/hw/i386/pc.h |  1 +
 include/qom/cpu.h    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 qom/cpu.c            |  7 +++++++
 5 files changed, 98 insertions(+), 4 deletions(-)

Comments

Eduardo Habkost Feb. 23, 2016, 9:38 p.m. UTC | #1
On Tue, Feb 23, 2016 at 05:05:54PM +0100, Igor Mammedov wrote:
> on x86 currently range 0..max_cpus is used to generate
> architecture-dependent CPU ID (APIC Id) for each present
> and possible CPUs. However architecture-dependent CPU IDs
> list could be sparse and code that needs to enumerate
> all IDs (ACPI) ended up doing guess work enumerating all
> possible and impossible IDs up to
>   apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
> 
> That leads to creation of MADT/SRAT entries and Processor
> objects in ACPI tables for not possible CPUs.
> Fix it by allowing board specify a concrete list of
> CPU IDs accourding its own rules (which for x86 depends
> on topology). So that code that needs this list could
> request it from board instead of trying to figure out
> what IDs are correct on its own.
> 
> This interface will also allow to help making AML
> part of CPU hotplug target independent so it could
> be reused for ARM target.

Do you expect any non-TYPE_MACHINE class to implement this
interface? If not, why not just make it a simple MachineClass
field? Machines that don't implement the get_possible_cpus_list()
method would just have it set to NULL.
Igor Mammedov Feb. 24, 2016, 8:36 a.m. UTC | #2
On Tue, 23 Feb 2016 18:38:33 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Feb 23, 2016 at 05:05:54PM +0100, Igor Mammedov wrote:
> > on x86 currently range 0..max_cpus is used to generate
> > architecture-dependent CPU ID (APIC Id) for each present
> > and possible CPUs. However architecture-dependent CPU IDs
> > list could be sparse and code that needs to enumerate
> > all IDs (ACPI) ended up doing guess work enumerating all
> > possible and impossible IDs up to
> >   apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).
> > 
> > That leads to creation of MADT/SRAT entries and Processor
> > objects in ACPI tables for not possible CPUs.
> > Fix it by allowing board specify a concrete list of
> > CPU IDs accourding its own rules (which for x86 depends
> > on topology). So that code that needs this list could
> > request it from board instead of trying to figure out
> > what IDs are correct on its own.
> > 
> > This interface will also allow to help making AML
> > part of CPU hotplug target independent so it could
> > be reused for ARM target.  
> 
> Do you expect any non-TYPE_MACHINE class to implement this
> interface? If not, why not just make it a simple MachineClass
> field? Machines that don't implement the get_possible_cpus_list()
> method would just have it set to NULL.
I redid it with interface so that not clatter MachineClass with fields
that won't be used by all(most) machines, as they don't care about
hotplug. There is no other reason than that.
If you think it's better to do with MachineClass hook I can quickly
respin series with it.

Marcel,
what is your opinion on it?
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 151a64c..bfc5bae 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1132,10 +1132,17 @@  void pc_cpus_init(PCMachineState *pcms)
         exit(1);
     }
 
-    for (i = 0; i < smp_cpus; i++) {
-        cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
-                         &error_fatal);
-        object_unref(OBJECT(cpu));
+    pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                                    sizeof(CPUArchId) * (max_cpus - 1));
+    for (i = 0; i < max_cpus; i++) {
+        pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+        pcms->possible_cpus->len++;
+        if (i < smp_cpus) {
+            cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
+                             &error_fatal);
+            pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
+            object_unref(OBJECT(cpu));
+        }
     }
 
     /* tell smbios about cpuid version and features */
@@ -1658,9 +1665,19 @@  static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
     error_propagate(errp, local_err);
 }
 
+static int pc_apic_cmp(const void *a, const void *b)
+{
+   CPUArchId *apic_a = (CPUArchId *)a;
+   CPUArchId *apic_b = (CPUArchId *)b;
+
+   return apic_a->arch_id - apic_b->arch_id;
+}
+
 static void pc_cpu_plug(HotplugHandler *hotplug_dev,
                         DeviceState *dev, Error **errp)
 {
+    CPUClass *cc = CPU_GET_CLASS(dev);
+    CPUArchId apic_id, *found_cpu;
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
@@ -1683,6 +1700,13 @@  static void pc_cpu_plug(HotplugHandler *hotplug_dev,
 
     /* increment the number of CPUs */
     rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
+
+    apic_id.arch_id = cc->get_arch_id(CPU(dev));
+    found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
+        pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
+        pc_apic_cmp);
+    assert(found_cpu);
+    found_cpu->cpu = CPU(dev);
 out:
     error_propagate(errp, local_err);
 }
@@ -1925,11 +1949,23 @@  static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
     return topo.pkg_id;
 }
 
+static CPUArchIdList *pc_get_possible_cpus_list(PossibleCpus *pcs)
+{
+    PCMachineState *pcms = PC_MACHINE(pcs);
+    int len = sizeof(CPUArchIdList) +
+              sizeof(CPUArchId) * (pcms->possible_cpus->len - 1);
+    CPUArchIdList *list = g_malloc(len);
+
+    memcpy(list, pcms->possible_cpus, len);
+    return list;
+}
+
 static void pc_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+    PossibleCpusClass *psc = POSSIBLE_CPUS_CLASS(oc);
 
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
     pcmc->pci_enabled = true;
@@ -1954,6 +1990,7 @@  static void pc_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = pc_machine_device_plug_cb;
     hc->unplug_request = pc_machine_device_unplug_request_cb;
     hc->unplug = pc_machine_device_unplug_cb;
+    psc->get_possible_cpus_list = pc_get_possible_cpus_list;
 }
 
 static const TypeInfo pc_machine_info = {
@@ -1966,6 +2003,7 @@  static const TypeInfo pc_machine_info = {
     .class_init = pc_machine_class_init,
     .interfaces = (InterfaceInfo[]) {
          { TYPE_HOTPLUG_HANDLER },
+         { TYPE_POSSIBLE_CPUS },
          { }
     },
 };
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..90a9d15 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -8,6 +8,7 @@ 
 #include "sysemu/accel.h"
 #include "hw/qdev.h"
 #include "qom/object.h"
+#include "qom/cpu.h"
 
 void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
                                           const char *name,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8b3546e..3e09232 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -65,6 +65,7 @@  struct PCMachineState {
     /* CPU and apic information: */
     bool apic_xrupt_override;
     unsigned apic_id_limit;
+    CPUArchIdList *possible_cpus;
 
     /* NUMA information: */
     uint64_t numa_nodes;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index ff54600..eedd3f2 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -837,4 +837,51 @@  extern const struct VMStateDescription vmstate_cpu_common;
     .offset = 0,                                                            \
 }
 
+#define TYPE_POSSIBLE_CPUS "possible-cpus"
+
+#define POSSIBLE_CPUS_CLASS(klass) \
+     OBJECT_CLASS_CHECK(PossibleCpusClass, (klass), \
+                        TYPE_POSSIBLE_CPUS)
+#define POSSIBLE_CPUS_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(PossibleCpusClass, (obj), \
+                      TYPE_POSSIBLE_CPUS)
+#define POSSIBLE_CPUS(obj) \
+     INTERFACE_CHECK(PossibleCpus, (obj), \
+                     TYPE_POSSIBLE_CPUS)
+
+typedef struct PossibleCpus {
+    /* <private> */
+    Object Parent;
+} PossibleCpus;
+
+/**
+ * CPUArchId:
+ * @arch_id - architecture-dependent CPU ID of present or possible CPU
+ * @cpu - pointer to corresponding CPU object ii it's present on NULL otherwise
+ */
+typedef struct {
+    uint64_t arch_id;
+    struct CPUState *cpu;
+} CPUArchId;
+
+typedef struct {
+    int len;
+    CPUArchId cpus[1];
+} CPUArchIdList;
+
+/**
+ * PossibleCpusClass:
+ * @get_possible_cpus_list:
+ *    Returns asorted by arch_id array of possible CPUs,
+ *    which includes CPU IDs for present and possible to hotplug CPUs.
+ *    Caller is responsible for freeing returned list.
+ */
+typedef struct PossibleCpusClass {
+    /* <private> */
+    InterfaceClass parent_class;
+
+    /* <public> */
+    CPUArchIdList *(*get_possible_cpus_list)(PossibleCpus *pcs);
+} PossibleCpusClass;
+
 #endif
diff --git a/qom/cpu.c b/qom/cpu.c
index aeb32f1..8e9968f 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -384,9 +384,16 @@  static const TypeInfo cpu_type_info = {
     .class_init = cpu_class_init,
 };
 
+static const TypeInfo possible_cpus_interface_info = {
+    .name          = TYPE_POSSIBLE_CPUS,
+    .parent        = TYPE_INTERFACE,
+    .class_size = sizeof(PossibleCpusClass),
+};
+
 static void cpu_register_types(void)
 {
     type_register_static(&cpu_type_info);
+    type_register_static(&possible_cpus_interface_info);
 }
 
 type_init(cpu_register_types)