[Qemu-devel] [RFC for-2.1 for for-2.0-stable] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled

[Qemu-devel] [RFC for-2.1 for for-2.0-stable] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled

[Igor Mammedov]

Fixes migration regression from QEMU-1.7 to a newer QEMUs.
SSDT table size in QEMU-1.7 doesn't change regardless of
a number of PCI bridge devices present at startup.

However in QEMU-2.0 since addition of hotplug on PCI bridges,
each PCI bridge adds ~1875 bytes to SSDT table, including
pc-i440fx-1.7 machine type where PCI bridge hotplug disabled
via compat property.
It breaks migration from "QEMU-1.7" to "QEMU-2.[01] -M pc-i440fx-1.7"
since RAMBlock size of ACPI tables on target becomes larger
then on source and migration fails with:

"Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000"

error.

Fix this by generating AML only for PCI0 bus if
hotplug on PCI bridges is disabled and preserves PCI brigde
description in AML as it was done in QEMU-1.7 for pc-i440fx-1.7.

It will help to maintain size of SSDT static regardless of
number of PCI bridges on startup for pc-i440fx-1.7 machine type.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/acpi-build.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..ecd6d3a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -64,6 +64,7 @@ typedef struct AcpiMcfgInfo {
 typedef struct AcpiPmInfo {
     bool s3_disabled;
     bool s4_disabled;
+    bool pcihp_bridge_en;
     uint8_t s4_val;
     uint16_t sci_int;
     uint8_t acpi_enable_cmd;
@@ -85,6 +86,7 @@ typedef struct AcpiBuildPciBusHotplugState {
     GArray *device_table;
     GArray *notify_table;
     struct AcpiBuildPciBusHotplugState *parent;
+    bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
 static void acpi_get_dsdt(AcpiMiscInfo *info)
@@ -188,6 +190,9 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
                                            NULL);
     pm->gpe0_blk_len = object_property_get_int(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
                                                NULL);
+    pm->pcihp_bridge_en =
+        object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
+                                 NULL);
 }
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -768,11 +773,13 @@ static void acpi_set_pci_info(void)
 }
 
 static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
-                                     AcpiBuildPciBusHotplugState *parent)
+                                     AcpiBuildPciBusHotplugState *parent,
+                                     bool pcihp_bridge_en)
 {
     state->parent = parent;
     state->device_table = build_alloc_array();
     state->notify_table = build_alloc_array();
+    state->pcihp_bridge_en = pcihp_bridge_en;
 }
 
 static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
@@ -786,7 +793,7 @@ static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
     AcpiBuildPciBusHotplugState *parent = parent_state;
     AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
 
-    build_pci_bus_state_init(child, parent);
+    build_pci_bus_state_init(child, parent, parent->pcihp_bridge_en);
 
     return child;
 }
@@ -807,6 +814,14 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
     GArray *method;
     bool bus_hotplug_support = false;
 
+    /*
+        skip bridge subtree creation if bridge hotplug is disabled
+        to make it compatible with 1.7 machine type
+    */
+    if (!child->pcihp_bridge_en && bus->parent_dev) {
+        return;
+    }
+
     if (bus->parent_dev) {
         op = 0x82; /* DeviceOp */
         build_append_nameseg(bus_table, "S%.02X_",
@@ -853,7 +868,8 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
         pc = PCI_DEVICE_GET_CLASS(pdev);
         dc = DEVICE_GET_CLASS(pdev);
 
-        if (pc->class_id == PCI_CLASS_BRIDGE_ISA || pc->is_bridge) {
+        if (pc->class_id == PCI_CLASS_BRIDGE_ISA ||
+            (pc->is_bridge && child->pcihp_bridge_en)) {
             set_bit(slot, slot_device_system);
         }
 
@@ -865,7 +881,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
             }
         }
 
-        if (!dc->hotpluggable || pc->is_bridge) {
+        if (!dc->hotpluggable || (pc->is_bridge && child->pcihp_bridge_en)) {
             clear_bit(slot, slot_hotplug_enable);
         }
     }
@@ -1130,7 +1146,7 @@ build_ssdt(GArray *table_data, GArray *linker,
                 bus = PCI_HOST_BRIDGE(pci_host)->bus;
             }
 
-            build_pci_bus_state_init(&hotplug_state, NULL);
+            build_pci_bus_state_init(&hotplug_state, NULL, pm->pcihp_bridge_en);
 
             if (bus) {
                 /* Scan all PCI buses. Generate tables to support hotplug. */
-- 
1.8.3.1

Re: [Qemu-devel] [RFC for-2.1 for for-2.0-stable] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled

[Paolo Bonzini]

Il 24/07/2014 11:07, Igor Mammedov ha scritto:
> Fixes migration regression from QEMU-1.7 to a newer QEMUs.
> SSDT table size in QEMU-1.7 doesn't change regardless of
> a number of PCI bridge devices present at startup.
> 
> However in QEMU-2.0 since addition of hotplug on PCI bridges,
> each PCI bridge adds ~1875 bytes to SSDT table, including
> pc-i440fx-1.7 machine type where PCI bridge hotplug disabled
> via compat property.
> It breaks migration from "QEMU-1.7" to "QEMU-2.[01] -M pc-i440fx-1.7"
> since RAMBlock size of ACPI tables on target becomes larger
> then on source and migration fails with:
> 
> "Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000"
> 
> error.

In principle, this definitely makes sense.  With QEMU-2.0 and 2.0.1 out
in the wild (and packaged into Ubuntu), however, it's difficult to
decide whether to include this patch or not.

I feel more like having downstreams include it if they skipped QEMU 2.0
and went straight from 1.7 to 2.1.

Paolo