[5/9] pc: acpi: create Processor and Notify objects only for valid lapics

Commit Message

Igor Mammedov Feb. 4, 2016, 11:47 a.m. UTC

do not assume that all lapics in range 0..apic_id_limit
are valid and do not create Processor and Notify objects
for not possible lapics.

Signed-off-by: Igor Mammedov <[e-mail]>
---
 hw/i386/acpi-build.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

Comments

Eduardo Habkost Feb. 5, 2016, 3:17 p.m. UTC | #1

On Thu, Feb 04, 2016 at 12:47:31PM +0100, Igor Mammedov wrote:
> do not assume that all lapics in range 0..apic_id_limit
> are valid and do not create Processor and Notify objects
> for not possible lapics.
> 
> Signed-off-by: Igor Mammedov <[e-mail]>

Any specific reason you split the Processor/Notify changes and
the MADT/lapic entries into patches 5 and 6? Won't guests be
confused if the Processor entries are missing but the lapic
entries are still there?

I wouldn't mind merging patches 4-6 into a single patch, just to
avoid risking unnecessary bisectability issues.

Reviewed-by: Eduardo Habkost <[e-mail]>

Igor Mammedov Feb. 5, 2016, 3:43 p.m. UTC | #2

On Fri, 5 Feb 2016 13:17:14 -0200
Eduardo Habkost <[e-mail]> wrote:

> On Thu, Feb 04, 2016 at 12:47:31PM +0100, Igor Mammedov wrote:
> > do not assume that all lapics in range 0..apic_id_limit
> > are valid and do not create Processor and Notify objects
> > for not possible lapics.
> > 
> > Signed-off-by: Igor Mammedov <[e-mail]>  
> 
> Any specific reason you split the Processor/Notify changes and
> the MADT/lapic entries into patches 5 and 6? Won't guests be
> confused if the Processor entries are missing but the lapic
> entries are still there?
Guest shouldn't be confused as MADT is used during while
Processor during hotplug.

> 
> I wouldn't mind merging patches 4-6 into a single patch, just to
> avoid risking unnecessary bisectability issues.
Patches were split to make them smaller and more reviewable
but I'm ok with merging them together

> 
> Reviewed-by: Eduardo Habkost <[e-mail]>
>

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3077061..df13c7d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -960,7 +960,8 @@  static Aml *build_crs(PCIHostState *host,
 }
 
 static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
-                                    AcpiCpuInfo *cpu, AcpiPmInfo *pm)
+                                    AcpiCpuInfo *cpu, AcpiPmInfo *pm,
+                                    MachineState *machine)
 {
     int i;
     Aml *dev;
@@ -969,6 +970,8 @@  static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
     Aml *field;
     Aml *ifctx;
     Aml *method;
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    GArray *apic_id_list = mc->possible_cpu_arch_ids();
 
     /* The current AML generator can cover the APIC ID range [0..255],
      * inclusive, for VCPU hotplug. */
@@ -998,22 +1001,27 @@  static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
     aml_append(sb_scope, field);
 
     /* build Processor object for each processor */
-    for (i = 0; i < acpi_cpus; i++) {
-        dev = aml_processor(i, 0, 0, "CP%.02X", i);
+    for (i = 0; i < apic_id_list->len; i++) {
+        CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+        int apic_id = id.arch_id;
+
+        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+        dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
 
         method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(i))));
+            aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id))));
         aml_append(dev, method);
 
         method = aml_method("_STA", 0, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(i))));
+            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
         aml_append(dev, method);
 
         method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(i), aml_arg(0)))
+            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+                aml_arg(0)))
         );
         aml_append(dev, method);
 
@@ -1025,10 +1033,13 @@  static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
      */
     /* Arg0 = Processor ID = APIC ID */
     method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
-    for (i = 0; i < acpi_cpus; i++) {
-        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
+    for (i = 0; i < apic_id_list->len; i++) {
+        CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+        int apic_id = id.arch_id;
+
+        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
         aml_append(ifctx,
-            aml_notify(aml_name("CP%.02X", i), aml_arg(1))
+            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
         );
         aml_append(method, ifctx);
     }
@@ -1049,6 +1060,7 @@  static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
         aml_append(pkg, aml_int(b));
     }
     aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
+    g_array_free(apic_id_list, true);
 }
 
 static void build_memory_devices(Aml *sb_scope, int nr_mem,
@@ -2244,7 +2256,8 @@  build_dsdt(GArray *table_data, GArray *linker,
 
     sb_scope = aml_scope("\\_SB");
     {
-        build_processor_devices(sb_scope, guest_info->apic_id_limit, cpu, pm);
+        build_processor_devices(sb_scope, guest_info->apic_id_limit, cpu, pm,
+                                machine);
 
         build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
                              pm->mem_hp_io_len);