All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1
@ 2014-07-28 14:02 Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

v2->v3:
	fix tests/acpi-test-data/pc/DSDT [Peter]
	track down "make check" failure, fix it [patch 4, me]
	split patch 2 in two parts [mst]
	do not make bsel_alloc global [mst]
	include Igor's bridge patch [mst, as discussed on IRC]

Igor Mammedov (1):
  pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is
    disabled

Paolo Bonzini (4):
  acpi-dsdt: procedurally generate _PRT
  pc: hack for migration compatibility from QEMU 2.0
  pc: future-proof migration-compatibility of ACPI tables
  bios-tables-test: fix ASL normalization false positive

 hw/i386/acpi-build.c            |  119 ++-
 hw/i386/acpi-dsdt.dsl           |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++------------------------------------
 hw/i386/pc_piix.c               |   19 +
 hw/i386/pc_q35.c                |    5 +
 include/hw/i386/pc.h            |    1 +
 tests/acpi-test-data/pc/DSDT    |  Bin 4499 -> 2807 bytes
 tests/bios-tables-test.c        |    6 +-
 8 files changed, 288 insertions(+), 1862 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT
  2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
@ 2014-07-28 14:02 ` Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

This replaces the _PRT constant with a method that computes it.

The problem is that the DSDT+SSDT have grown from 2.0 to 2.1,
enough to cross the 8k barrier (we align the ACPI tables to 4k
before putting them in fw_cfg).  This causes problems with
migration and the pc-i440fx-2.0 machine type.

The solution to the problem is to hardcode 64k as the limit,
but this doesn't solve the bug with pc-i440fx-2.0.  The fix will be
for QEMU 2.1 to use exactly the same size as QEMU 2.0 for the
ACPI tables.  First, however, we must make the actual AML
equal or smaller; to do this, rewrite _PRT in a way that saves
over 1k of bytecode.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-dsdt.dsl           |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++------------------------------------
 tests/acpi-test-data/pc/DSDT    |  Bin 4499 -> 2807 bytes
 3 files changed, 148 insertions(+), 1852 deletions(-)

diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 3cc0ea0..6ba0170 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -181,57 +181,45 @@ DefinitionBlock (
 
     Scope(\_SB) {
         Scope(PCI0) {
-            Name(_PRT, Package() {
-                /* PCI IRQ routing table, example from ACPI 2.0a specification,
-                   section 6.2.8.1 */
-                /* Note: we provide the same info as the PCI routing
-                   table of the Bochs BIOS */
-
-#define prt_slot(nr, lnk0, lnk1, lnk2, lnk3) \
-    Package() { nr##ffff, 0, lnk0, 0 }, \
-    Package() { nr##ffff, 1, lnk1, 0 }, \
-    Package() { nr##ffff, 2, lnk2, 0 }, \
-    Package() { nr##ffff, 3, lnk3, 0 }
-
-#define prt_slot0(nr) prt_slot(nr, LNKD, LNKA, LNKB, LNKC)
-#define prt_slot1(nr) prt_slot(nr, LNKA, LNKB, LNKC, LNKD)
-#define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA)
-#define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB)
-
-                prt_slot0(0x0000),
-                /* Device 1 is power mgmt device, and can only use irq 9 */
-                prt_slot(0x0001, LNKS, LNKB, LNKC, LNKD),
-                prt_slot2(0x0002),
-                prt_slot3(0x0003),
-                prt_slot0(0x0004),
-                prt_slot1(0x0005),
-                prt_slot2(0x0006),
-                prt_slot3(0x0007),
-                prt_slot0(0x0008),
-                prt_slot1(0x0009),
-                prt_slot2(0x000a),
-                prt_slot3(0x000b),
-                prt_slot0(0x000c),
-                prt_slot1(0x000d),
-                prt_slot2(0x000e),
-                prt_slot3(0x000f),
-                prt_slot0(0x0010),
-                prt_slot1(0x0011),
-                prt_slot2(0x0012),
-                prt_slot3(0x0013),
-                prt_slot0(0x0014),
-                prt_slot1(0x0015),
-                prt_slot2(0x0016),
-                prt_slot3(0x0017),
-                prt_slot0(0x0018),
-                prt_slot1(0x0019),
-                prt_slot2(0x001a),
-                prt_slot3(0x001b),
-                prt_slot0(0x001c),
-                prt_slot1(0x001d),
-                prt_slot2(0x001e),
-                prt_slot3(0x001f),
-            })
+            Method (_PRT, 0) {
+                Store(Package(128) {}, Local0)
+                Store(Zero, Local1)
+                While(LLess(Local1, 128)) {
+                    // slot = pin >> 2
+                    Store(ShiftRight(Local1, 2), Local2)
+
+                    // lnk = (slot + pin) & 3
+                    Store(And(Add(Local1, Local2), 3), Local3)
+                    If (LEqual(Local3, 0)) {
+                        Store(Package(4) { Zero, Zero, LNKD, Zero }, Local4)
+                    }
+                    If (LEqual(Local3, 1)) {
+                        // device 1 is the power-management device, needs SCI
+                        If (LEqual(Local1, 4)) {
+                            Store(Package(4) { Zero, Zero, LNKS, Zero }, Local4)
+                        } Else {
+                            Store(Package(4) { Zero, Zero, LNKA, Zero }, Local4)
+                        }
+                    }
+                    If (LEqual(Local3, 2)) {
+                        Store(Package(4) { Zero, Zero, LNKB, Zero }, Local4)
+                    }
+                    If (LEqual(Local3, 3)) {
+                        Store(Package(4) { Zero, Zero, LNKC, Zero }, Local4)
+                    }
+
+                    // Complete the interrupt routing entry:
+                    //    Package(4) { 0x[slot]FFFF, [pin], [link], 0) }
+
+                    Store(Or(ShiftLeft(Local2, 16), 0xFFFF), Index(Local4, 0))
+                    Store(And(Local1, 3),                    Index(Local4, 1))
+                    Store(Local4,                            Index(Local0, Local1))
+
+                    Increment(Local1)
+                }
+
+                Return(Local0)
+            }
         }
 
         Field(PCI0.ISA.P40C, ByteAcc, NoLock, Preserve) {
diff --git a/hw/i386/acpi-dsdt.hex.generated b/hw/i386/acpi-dsdt.hex.generated
index ee490e8..6c8a1fc 100644
--- a/hw/i386/acpi-dsdt.hex.generated
+++ b/hw/i386/acpi-dsdt.hex.generated
@@ -3,12 +3,12 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x53,
 0x44,
 0x54,
-0x93,
-0x11,
+0xf7,
+0xa,
 0x0,
 0x0,
 0x1,
-0xf5,
+0x2e,
 0x42,
 0x58,
 0x50,
@@ -31,9 +31,9 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x4e,
 0x54,
 0x4c,
-0x15,
-0x11,
 0x13,
+0x9,
+0x12,
 0x20,
 0x10,
 0x49,
@@ -1439,143 +1439,123 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0xa4,
 0x0,
 0x10,
-0x4a,
-0xa0,
+0x4e,
+0x36,
 0x5f,
 0x53,
 0x42,
 0x5f,
 0x10,
-0x47,
-0x74,
+0x4b,
+0xa,
 0x50,
 0x43,
 0x49,
 0x30,
-0x8,
+0x14,
+0x44,
+0xa,
 0x5f,
 0x50,
 0x52,
 0x54,
-0x12,
-0x4b,
-0x73,
-0x80,
-0x12,
-0xb,
-0x4,
-0xb,
-0xff,
-0xff,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
 0x0,
+0x70,
 0x12,
-0xb,
-0x4,
-0xb,
-0xff,
-0xff,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
+0x2,
+0x80,
+0x60,
+0x70,
 0x0,
-0x12,
-0xc,
-0x4,
-0xb,
-0xff,
-0xff,
+0x61,
+0xa2,
+0x42,
+0x9,
+0x95,
+0x61,
+0xa,
+0x80,
+0x70,
+0x7a,
+0x61,
 0xa,
 0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
 0x0,
-0x12,
-0xc,
-0x4,
-0xb,
-0xff,
-0xff,
+0x62,
+0x70,
+0x7b,
+0x72,
+0x61,
+0x62,
+0x0,
 0xa,
 0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
 0x0,
+0x63,
+0xa0,
+0x10,
+0x93,
+0x63,
+0x0,
+0x70,
 0x12,
-0xd,
+0x9,
 0x4,
-0xc,
-0xff,
-0xff,
-0x1,
 0x0,
 0x0,
 0x4c,
 0x4e,
 0x4b,
-0x53,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1,
+0x44,
 0x0,
+0x64,
+0xa0,
+0x24,
+0x93,
+0x63,
 0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
+0xa0,
+0x11,
+0x93,
+0x61,
+0xa,
+0x4,
+0x70,
 0x12,
-0xe,
+0x9,
 0x4,
-0xc,
-0xff,
-0xff,
-0x1,
 0x0,
-0xa,
-0x2,
+0x0,
 0x4c,
 0x4e,
 0x4b,
-0x43,
+0x53,
 0x0,
+0x64,
+0xa1,
+0xd,
+0x70,
 0x12,
-0xe,
+0x9,
 0x4,
-0xc,
-0xff,
-0xff,
-0x1,
 0x0,
-0xa,
-0x3,
+0x0,
 0x4c,
 0x4e,
 0x4b,
-0x44,
+0x41,
 0x0,
+0x64,
+0xa0,
+0x11,
+0x93,
+0x63,
+0xa,
+0x2,
+0x70,
 0x12,
-0xd,
+0x9,
 0x4,
-0xc,
-0xff,
-0xff,
-0x2,
 0x0,
 0x0,
 0x4c,
@@ -1583,1732 +1563,60 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x4b,
 0x42,
 0x0,
+0x64,
+0xa0,
+0x11,
+0x93,
+0x63,
+0xa,
+0x3,
+0x70,
 0x12,
-0xd,
+0x9,
 0x4,
-0xc,
-0xff,
-0xff,
-0x2,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
 0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x2,
 0x0,
-0xa,
-0x2,
 0x4c,
 0x4e,
 0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x2,
+0x43,
 0x0,
+0x64,
+0x70,
+0x7d,
+0x79,
+0x62,
 0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
+0x10,
 0x0,
-0x12,
-0xd,
-0x4,
-0xc,
+0xb,
 0xff,
 0xff,
-0x3,
 0x0,
+0x88,
+0x64,
 0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
 0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
+0x70,
+0x7b,
+0x61,
+0xa,
 0x3,
 0x0,
+0x88,
+0x64,
 0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x3,
 0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x3,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x4,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x4,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x4,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x4,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x5,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x5,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x5,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x5,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x6,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x6,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x6,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x6,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x7,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x7,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x7,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x7,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x8,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x8,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x8,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x8,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x9,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x9,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x9,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x9,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xa,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xa,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xa,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xa,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xb,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xb,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xb,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xb,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xc,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xc,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xc,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xc,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xd,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xd,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xd,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xd,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xe,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xe,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xe,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xe,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xf,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0xf,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xf,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0xf,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x10,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x10,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x10,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x10,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x11,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x11,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x11,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x11,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x12,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x12,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x12,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x12,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x13,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x13,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x13,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x13,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x14,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x14,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x14,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x14,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x15,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x15,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x15,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x15,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x16,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x16,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x16,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x16,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x17,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x17,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x17,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x17,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x18,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x18,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x18,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x18,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x19,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x19,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x19,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x19,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1a,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1a,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1a,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1a,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1b,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1b,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1b,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1b,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1c,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1c,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1c,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1c,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1d,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1d,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1d,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1d,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1e,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x42,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1e,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1e,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1e,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1f,
-0x0,
-0x0,
-0x4c,
-0x4e,
-0x4b,
-0x43,
-0x0,
-0x12,
-0xd,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1f,
-0x0,
-0x1,
-0x4c,
-0x4e,
-0x4b,
-0x44,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1f,
-0x0,
-0xa,
-0x2,
-0x4c,
-0x4e,
-0x4b,
-0x41,
-0x0,
-0x12,
-0xe,
-0x4,
-0xc,
-0xff,
-0xff,
-0x1f,
-0x0,
-0xa,
-0x3,
-0x4c,
-0x4e,
-0x4b,
-0x42,
+0x70,
+0x64,
+0x88,
+0x60,
+0x61,
 0x0,
+0x75,
+0x61,
+0xa4,
+0x60,
 0x5b,
 0x81,
 0x24,
diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index 7ed03fd37effb1b0b2c19f49b5cbff168a29f9bc..d37ec34454e6f3db5e91b777f94e03be67a5f583 100644
GIT binary patch
delta 204
zcmX}lzY2m-9L4eDpN4)FXw(zb0|Y^PdY3Rn(p|6K2e<?cC6S~1rDltIfatwi9nBWU
z^Wh6e&FU$!kVQ2CTJfe^@8YlG(1|UN0(Y~RFp7g+1Uc4=M1nt@{*_Z9Pn>k5GT-wQ
zWFV_7m9<gd1h&4%AgZ~_w8~bT%#K5d`>0#%HnLSldMD0bmMs|_xe)SvOv%EaX;2D4
V=Da;Dg$60ggyX=U$>%$KnLkXQHJAVZ

delta 1909
zcmY+^Nlqg{5Czb*U7o7SQ+b{jeSy9LV5!wj3n7~&k`Dp<<PJ!zd(#I%9FAr2GqS?O
z-}&3|o%tLpe@<V2q=(bt;dSI(?f9O)HT|D|=hO9h`0M%p=J(Ie`TqD&+&;Y&S(25d
z`+IkLaPf5wudiH@hxgMu!nKA5!i_6>NnVyU7dod)3M~xw<7d*ts75q>I6$g(sbaWF
zp{lcWCsm9}6)_x&>&R?FntFwX3r!tu#3n|ii5QxCO5%KFlrKu8s<TO~>h%$=v?+(`
z6`Io2(PnJw?GZy$p)CvbWlO3$+hQrAm9|ORCTW|ch*sK>re2|Qf%>qS4oeZOv`f-1
zNxLjXw9?c{%aovLY<gW3vXy2e%}AQD6wyj^lI9m_faVNE^j6v<X^*5mmLghdpQL?~
z_G43_hz9D*0Z9iW9k3M9N{1vJl61&YL<_~^Up+sv{}D+?EJd`^f}{mW3zi}pX*nk7
zI6{5(V2)XeXr&X9PDnaoDWa85FVe98DM6=bdi)Oh>>*p}jHEM?&RB|QrE`+bqqKs~
z8S0U(bV1SuNf#_dw9+L>mn2=rra}=FD*IoNbVbq?OA)PfP0}?<*DOV}&`pG5|MJ(n
wA?b#th*r8K>6WBhmLjS&?0-kl9h#5d>)){y(MtCu-IH|BQp70zIJW*LU%2GTp#T5?

-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0
  2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT Paolo Bonzini
@ 2014-07-28 14:02 ` Paolo Bonzini
  2014-07-28 15:05   ` Michael S. Tsirkin
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

Changing the ACPI table size causes migration to break, and the memory
hotplug work opened our eyes on how horribly we were breaking things in
2.0 already.

The ACPI table size is rounded to the next 4k, which one would think
gives some headroom.  In practice this is not the case, because the user
can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
8 to the MADT) and so some "-smp" values will break the 4k boundary and
fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.

This patch concerns itself with fixing migration from QEMU 2.0.  It
computes the payload size of QEMU 2.0 and always uses that one.
The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size
should always be enough; non-AML tables can change depending on the
configuration (especially MADT, SRAT, HPET) but they remain the same
between QEMU 2.0 and 2.1, so we only compute our padding based on the
sizes of the SSDT and DSDT.

Migration from QEMU 1.7 should work for guests that have a number of CPUs
other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
broken from QEMU 1.7 to QEMU 2.0 in the same way, though.

The amount of AML for a bridge varies a little bit between 1872 and 1875
due to optimized number encodings.  Use the smallest value, and let any
extra chew away at the slack left by the shrinking of the DSDT.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-build.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/i386/pc_piix.c    | 19 ++++++++++++
 hw/i386/pc_q35.c     |  5 ++++
 include/hw/i386/pc.h |  1 +
 4 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..7d2251f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -25,7 +25,9 @@
 #include <glib.h>
 #include "qemu-common.h"
 #include "qemu/bitmap.h"
+#include "qemu/osdep.h"
 #include "qemu/range.h"
+#include "qemu/error-report.h"
 #include "hw/pci/pci.h"
 #include "qom/cpu.h"
 #include "hw/i386/pc.h"
@@ -52,6 +54,15 @@
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 
+/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
+ * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
+ * a little bit, there should be plenty of free space since the DSDT
+ * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
+ */
+#define ACPI_BUILD_LEGACY_CPU_AML_SIZE    97
+#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872
+#define ACPI_BUILD_ALIGN_SIZE             0x1000
+
 typedef struct AcpiCpuInfo {
     DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -737,6 +748,27 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
     ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
 }
 
+static void *pci_for_each_bus_incr_func(PCIBus *bus, void *opaque)
+{
+    unsigned *bsel_alloc = opaque;
+
+    if (bus->qbus.allow_hotplug) {
+        (*bsel_alloc)++;
+    }
+
+    return bsel_alloc;
+}
+
+static unsigned acpi_count_hotpluggable_pci_buses(void)
+{
+    PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
+    unsigned bus_count = 0;
+
+    pci_for_each_bus_depth_first(bus, pci_for_each_bus_incr_func,
+                                 NULL, &bus_count);
+    return bus_count;
+}
+
 /* Assign BSEL property to all buses.  In the future, this can be changed
  * to only assign to buses that support hotplug.
  */
@@ -1440,13 +1472,14 @@ static
 void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 {
     GArray *table_offsets;
-    unsigned facs, dsdt, rsdt;
+    unsigned facs, ssdt, dsdt, rsdt;
     AcpiCpuInfo cpu;
     AcpiPmInfo pm;
     AcpiMiscInfo misc;
     AcpiMcfgInfo mcfg;
     PcPciInfo pci;
     uint8_t *u;
+    size_t aml_len = 0;
 
     acpi_get_cpu_info(&cpu);
     acpi_get_pm_info(&pm);
@@ -1474,13 +1507,20 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
     dsdt = tables->table_data->len;
     build_dsdt(tables->table_data, tables->linker, &misc);
 
+    /* Count the size of the DSDT and SSDT, we will need it for legacy
+     * sizing of ACPI tables.
+     */
+    aml_len += tables->table_data->len - dsdt;
+
     /* ACPI tables pointed to by RSDT */
     acpi_add_table(table_offsets, tables->table_data);
     build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
 
+    ssdt = tables->table_data->len;
     acpi_add_table(table_offsets, tables->table_data);
     build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
                guest_info);
+    aml_len += tables->table_data->len - ssdt;
 
     acpi_add_table(table_offsets, tables->table_data);
     build_madt(tables->table_data, tables->linker, &cpu, guest_info);
@@ -1513,14 +1553,50 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
     /* RSDP is in FSEG memory, so allocate it separately */
     build_rsdp(tables->rsdp, tables->linker, rsdt);
 
-    /* We'll expose it all to Guest so align size to reduce
+    /* We'll expose it all to Guest so we want to reduce
      * chance of size changes.
      * RSDP is small so it's easy to keep it immutable, no need to
      * bother with alignment.
+     *
+     * We used to align the tables to 4k, but of course this would
+     * too simple to be enough.  4k turned out to be too small an
+     * alignment very soon, and in fact it is almost impossible to
+     * keep the table size stable for all (max_cpus, max_memory_slots)
+     * combinations.  So the table size is always 64k for pc-i440fx-2.1
+     * and we give an error if the table grows beyond that limit.
+     *
+     * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
+     * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
+     * than 2.0 and we can always pad the smaller tables with zeros.  We can
+     * then use the exact size of the 2.0 tables.
+     *
+     * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
      */
-    acpi_align_size(tables->table_data, 0x1000);
+    if (guest_info->legacy_acpi_table_size) {
+        /* Subtracting aml_len gives the size of fixed tables.  Then add the
+         * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
+         */
+        int bus_count = acpi_count_hotpluggable_pci_buses();
+        int legacy_aml_len =
+            guest_info->legacy_acpi_table_size +
+            ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus +
+            ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE * (MAX(bus_count, 1) - 1);
+        int legacy_table_size =
+            ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len,
+                     ACPI_BUILD_ALIGN_SIZE);
+        if (tables->table_data->len > legacy_table_size) {
+            /* -M pc-i440fx-2.0 doesn't support memory hotplug, so this should
+             * never happen.
+             */
+            error_report("This configuration is not supported with -M pc-i440fx-2.0.");
+            error_report("Migration may not work to older versions of QEMU.");
+        }
+        g_array_set_size(tables->table_data, legacy_table_size);
+    } else {
+        acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
+    }
 
-    acpi_align_size(tables->linker, 0x1000);
+    acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
 
     /* Cleanup memory that's no longer used. */
     g_array_free(table_offsets, true);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..4524e6b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
+static int legacy_acpi_table_size;
 static bool smbios_defaults = true;
 static bool smbios_legacy_mode;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
@@ -163,6 +164,7 @@ static void pc_init1(MachineState *machine,
     guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
 
     guest_info->has_acpi_build = has_acpi_build;
+    guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
 
     guest_info->has_pci_info = has_pci_info;
     guest_info->isapc_ram_fw = !pci_enabled;
@@ -297,6 +299,23 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_0(MachineState *machine)
 {
+    /* This value depends on the actual DSDT and SSDT compiled into
+     * the source QEMU; unfortunately it depends on the binary and
+     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
+     * both QEMU 1.7 and QEMU 2.0.
+     *
+     * Large variations cause migration to fail for more than one
+     * consecutive value of the "-smp" maxcpus option.
+     *
+     * For small variations of the kind caused by different iasl versions,
+     * the 4k rounding usually leaves slack.  However, there could be still
+     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
+     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
+     *
+     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
+     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
+     */
+    legacy_acpi_table_size = 6652;
     smbios_legacy_mode = true;
     has_reserved_memory = false;
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f551961..c39ee98 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -155,6 +155,11 @@ static void pc_q35_init(MachineState *machine)
     guest_info->has_acpi_build = has_acpi_build;
     guest_info->has_reserved_memory = has_reserved_memory;
 
+    /* Migration was not supported in 2.0 for Q35, so do not bother
+     * with this hack (see hw/i386/acpi-build.c).
+     */
+    guest_info->legacy_acpi_table_size = 0;
+
     if (smbios_defaults) {
         MachineClass *mc = MACHINE_GET_CLASS(machine);
         /* These values are guest ABI, do not change */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1c0c382..f4b9b2b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -94,6 +94,7 @@ struct PcGuestInfo {
     uint64_t *node_mem;
     uint64_t *node_cpu;
     FWCfgState *fw_cfg;
+    int legacy_acpi_table_size;
     bool has_acpi_build;
     bool has_reserved_memory;
 };
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables
  2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini
@ 2014-07-28 14:02 ` Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled Paolo Bonzini
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

This patch avoids that similar changes break QEMU again in the future.
QEMU will now hard-code 64k as the maximum ACPI table size, which
(despite being an order of magnitude smaller than 640k) should be enough
for everyone.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-build.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7d2251f..8d42eaf 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -63,6 +63,8 @@
 #define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1875
 #define ACPI_BUILD_ALIGN_SIZE             0x1000
 
+#define ACPI_BUILD_TABLE_SIZE             0x10000
+
 typedef struct AcpiCpuInfo {
     DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1593,7 +1595,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         }
         g_array_set_size(tables->table_data, legacy_table_size);
     } else {
-        acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
+        if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
+            /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots.  */
+            error_report("ACPI tables are larger than 64k.  Please remove");
+            error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
+            exit(1);
+        }
+        g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
     }
 
     acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive
  2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables Paolo Bonzini
@ 2014-07-28 14:02 ` Paolo Bonzini
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled Paolo Bonzini
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

My version of IASL (from RHEL7) puts two newlines between the head comment
and the DefinitionBlock property.  One was already removed because the
test uses sizeof instead of strlen, but the extra one breaks the detection
of DefinitionBlock.  Killing all newlines after the comment drops the
warning.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/bios-tables-test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 62771f7..045eb27 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -487,7 +487,11 @@ static GString *normalize_asl(gchar *asl_code)
     /* strip comments (different generation days) */
     comment = g_strstr_len(asl->str, asl->len, COMMENT_END);
     if (comment) {
-        asl = g_string_erase(asl, 0, comment + sizeof(COMMENT_END) - asl->str);
+        comment += strlen(COMMENT_END);
+        while (*comment == '\n') {
+            comment++;
+        }
+        asl = g_string_erase(asl, 0, comment - asl->str);
     }
 
     /* strip def block name (it has file path in it) */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled
  2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive Paolo Bonzini
@ 2014-07-28 14:02 ` Paolo Bonzini
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 14:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

From: Igor Mammedov <imammedo@redhat.com>

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>
[Affect bus_count too. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-build.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8d42eaf..90f1b90 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -77,6 +77,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;
@@ -98,6 +99,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)
@@ -201,6 +203,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)
@@ -802,11 +807,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)
@@ -820,7 +827,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;
 }
@@ -841,6 +848,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_",
@@ -887,7 +902,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);
         }
 
@@ -899,7 +915,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);
         }
     }
@@ -1164,7 +1180,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. */
@@ -1578,7 +1594,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         /* Subtracting aml_len gives the size of fixed tables.  Then add the
          * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
          */
-        int bus_count = acpi_count_hotpluggable_pci_buses();
+        int bus_count =
+            pm.pcihp_bridge_en ? acpi_count_hotpluggable_pci_buses() : 1;
         int legacy_aml_len =
             guest_info->legacy_acpi_table_size +
             ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus +
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0
  2014-07-28 14:02 ` [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini
@ 2014-07-28 15:05   ` Michael S. Tsirkin
  2014-07-28 15:06     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2014-07-28 15:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: imammedo, lersek, qemu-devel

On Mon, Jul 28, 2014 at 04:02:12PM +0200, Paolo Bonzini wrote:
> Changing the ACPI table size causes migration to break, and the memory
> hotplug work opened our eyes on how horribly we were breaking things in
> 2.0 already.
> 
> The ACPI table size is rounded to the next 4k, which one would think
> gives some headroom.  In practice this is not the case, because the user
> can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
> 8 to the MADT) and so some "-smp" values will break the 4k boundary and
> fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.
> 
> This patch concerns itself with fixing migration from QEMU 2.0.  It
> computes the payload size of QEMU 2.0 and always uses that one.
> The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size
> should always be enough; non-AML tables can change depending on the
> configuration (especially MADT, SRAT, HPET) but they remain the same
> between QEMU 2.0 and 2.1, so we only compute our padding based on the
> sizes of the SSDT and DSDT.
> 
> Migration from QEMU 1.7 should work for guests that have a number of CPUs
> other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
> broken from QEMU 1.7 to QEMU 2.0 in the same way, though.
> 
> The amount of AML for a bridge varies a little bit between 1872 and 1875
> due to optimized number encodings.  Use the smallest value, and let any
> extra chew away at the slack left by the shrinking of the DSDT.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/acpi-build.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  hw/i386/pc_piix.c    | 19 ++++++++++++
>  hw/i386/pc_q35.c     |  5 ++++
>  include/hw/i386/pc.h |  1 +
>  4 files changed, 105 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebc5f03..7d2251f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -25,7 +25,9 @@
>  #include <glib.h>
>  #include "qemu-common.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/osdep.h"
>  #include "qemu/range.h"
> +#include "qemu/error-report.h"
>  #include "hw/pci/pci.h"
>  #include "qom/cpu.h"
>  #include "hw/i386/pc.h"
> @@ -52,6 +54,15 @@
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
>  
> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> + * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> + * a little bit, there should be plenty of free space since the DSDT
> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> + */
> +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE    97
> +#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872


Hmm this is wrong if some slot is occupied by a
non hotpluggable device, is it not?

> +#define ACPI_BUILD_ALIGN_SIZE             0x1000
> +
>  typedef struct AcpiCpuInfo {
>      DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>  } AcpiCpuInfo;
> @@ -737,6 +748,27 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
>      ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
>  }
>  
> +static void *pci_for_each_bus_incr_func(PCIBus *bus, void *opaque)
> +{
> +    unsigned *bsel_alloc = opaque;
> +
> +    if (bus->qbus.allow_hotplug) {
> +        (*bsel_alloc)++;
> +    }

Hmm I don't know why we do allow_hotplug in the
place where you copied this from :(

> +
> +    return bsel_alloc;
> +}
> +
> +static unsigned acpi_count_hotpluggable_pci_buses(void)
> +{
> +    PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
> +    unsigned bus_count = 0;
> +
> +    pci_for_each_bus_depth_first(bus, pci_for_each_bus_incr_func,
> +                                 NULL, &bus_count);
> +    return bus_count;
> +}
> +
>  /* Assign BSEL property to all buses.  In the future, this can be changed
>   * to only assign to buses that support hotplug.
>   */
> @@ -1440,13 +1472,14 @@ static
>  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>  {
>      GArray *table_offsets;
> -    unsigned facs, dsdt, rsdt;
> +    unsigned facs, ssdt, dsdt, rsdt;
>      AcpiCpuInfo cpu;
>      AcpiPmInfo pm;
>      AcpiMiscInfo misc;
>      AcpiMcfgInfo mcfg;
>      PcPciInfo pci;
>      uint8_t *u;
> +    size_t aml_len = 0;
>  
>      acpi_get_cpu_info(&cpu);
>      acpi_get_pm_info(&pm);
> @@ -1474,13 +1507,20 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      dsdt = tables->table_data->len;
>      build_dsdt(tables->table_data, tables->linker, &misc);
>  
> +    /* Count the size of the DSDT and SSDT, we will need it for legacy
> +     * sizing of ACPI tables.
> +     */
> +    aml_len += tables->table_data->len - dsdt;
> +
>      /* ACPI tables pointed to by RSDT */
>      acpi_add_table(table_offsets, tables->table_data);
>      build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
>  
> +    ssdt = tables->table_data->len;
>      acpi_add_table(table_offsets, tables->table_data);
>      build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
>                 guest_info);
> +    aml_len += tables->table_data->len - ssdt;
>  
>      acpi_add_table(table_offsets, tables->table_data);
>      build_madt(tables->table_data, tables->linker, &cpu, guest_info);
> @@ -1513,14 +1553,50 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      /* RSDP is in FSEG memory, so allocate it separately */
>      build_rsdp(tables->rsdp, tables->linker, rsdt);
>  
> -    /* We'll expose it all to Guest so align size to reduce
> +    /* We'll expose it all to Guest so we want to reduce
>       * chance of size changes.
>       * RSDP is small so it's easy to keep it immutable, no need to
>       * bother with alignment.
> +     *
> +     * We used to align the tables to 4k, but of course this would
> +     * too simple to be enough.  4k turned out to be too small an
> +     * alignment very soon, and in fact it is almost impossible to
> +     * keep the table size stable for all (max_cpus, max_memory_slots)
> +     * combinations.  So the table size is always 64k for pc-i440fx-2.1
> +     * and we give an error if the table grows beyond that limit.
> +     *
> +     * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
> +     * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
> +     * than 2.0 and we can always pad the smaller tables with zeros.  We can
> +     * then use the exact size of the 2.0 tables.
> +     *
> +     * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
>       */
> -    acpi_align_size(tables->table_data, 0x1000);
> +    if (guest_info->legacy_acpi_table_size) {
> +        /* Subtracting aml_len gives the size of fixed tables.  Then add the
> +         * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
> +         */
> +        int bus_count = acpi_count_hotpluggable_pci_buses();
> +        int legacy_aml_len =
> +            guest_info->legacy_acpi_table_size +
> +            ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus +
> +            ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE * (MAX(bus_count, 1) - 1);
> +        int legacy_table_size =
> +            ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len,
> +                     ACPI_BUILD_ALIGN_SIZE);
> +        if (tables->table_data->len > legacy_table_size) {
> +            /* -M pc-i440fx-2.0 doesn't support memory hotplug, so this should
> +             * never happen.
> +             */
> +            error_report("This configuration is not supported with -M pc-i440fx-2.0.");
> +            error_report("Migration may not work to older versions of QEMU.");
> +        }
> +        g_array_set_size(tables->table_data, legacy_table_size);
> +    } else {
> +        acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
> +    }
>  
> -    acpi_align_size(tables->linker, 0x1000);
> +    acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
>  
>      /* Cleanup memory that's no longer used. */
>      g_array_free(table_offsets, true);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7081c08..4524e6b 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -61,6 +61,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>  
>  static bool has_pci_info;
>  static bool has_acpi_build = true;
> +static int legacy_acpi_table_size;
>  static bool smbios_defaults = true;
>  static bool smbios_legacy_mode;
>  /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
> @@ -163,6 +164,7 @@ static void pc_init1(MachineState *machine,
>      guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
>  
>      guest_info->has_acpi_build = has_acpi_build;
> +    guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
>  
>      guest_info->has_pci_info = has_pci_info;
>      guest_info->isapc_ram_fw = !pci_enabled;
> @@ -297,6 +299,23 @@ static void pc_init_pci(MachineState *machine)
>  
>  static void pc_compat_2_0(MachineState *machine)
>  {
> +    /* This value depends on the actual DSDT and SSDT compiled into
> +     * the source QEMU; unfortunately it depends on the binary and
> +     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
> +     * both QEMU 1.7 and QEMU 2.0.
> +     *
> +     * Large variations cause migration to fail for more than one
> +     * consecutive value of the "-smp" maxcpus option.
> +     *
> +     * For small variations of the kind caused by different iasl versions,
> +     * the 4k rounding usually leaves slack.  However, there could be still
> +     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
> +     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
> +     *
> +     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
> +     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
> +     */
> +    legacy_acpi_table_size = 6652;
>      smbios_legacy_mode = true;
>      has_reserved_memory = false;
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f551961..c39ee98 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -155,6 +155,11 @@ static void pc_q35_init(MachineState *machine)
>      guest_info->has_acpi_build = has_acpi_build;
>      guest_info->has_reserved_memory = has_reserved_memory;
>  
> +    /* Migration was not supported in 2.0 for Q35, so do not bother
> +     * with this hack (see hw/i386/acpi-build.c).
> +     */
> +    guest_info->legacy_acpi_table_size = 0;
> +
>      if (smbios_defaults) {
>          MachineClass *mc = MACHINE_GET_CLASS(machine);
>          /* These values are guest ABI, do not change */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1c0c382..f4b9b2b 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -94,6 +94,7 @@ struct PcGuestInfo {
>      uint64_t *node_mem;
>      uint64_t *node_cpu;
>      FWCfgState *fw_cfg;
> +    int legacy_acpi_table_size;
>      bool has_acpi_build;
>      bool has_reserved_memory;
>  };
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0
  2014-07-28 15:05   ` Michael S. Tsirkin
@ 2014-07-28 15:06     ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 15:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: imammedo, lersek, qemu-devel

Il 28/07/2014 17:05, Michael S. Tsirkin ha scritto:
>> > +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>> > + * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
>> > + * a little bit, there should be plenty of free space since the DSDT
>> > + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
>> > + */
>> > +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE    97
>> > +#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872
> 
> Hmm this is wrong if some slot is occupied by a
> non hotpluggable device, is it not?

I honestly have no idea.  If even more complicated code is needed, I'd
rather get rid of all this bridge stuff altogether (as far as computing
the AML size goes), and declare 2.0->2.1 migration broken if the VM has
bridges.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables
  2014-07-28 16:08     ` Paolo Bonzini
@ 2014-07-28 21:20       ` Michael S. Tsirkin
  0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2014-07-28 21:20 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: imammedo, lersek, qemu-devel

On Mon, Jul 28, 2014 at 06:08:53PM +0200, Paolo Bonzini wrote:
> Il 28/07/2014 17:59, Michael S. Tsirkin ha scritto:
> > On Mon, Jul 28, 2014 at 05:34:16PM +0200, Paolo Bonzini wrote:
> >> This patch avoids that similar changes break QEMU again in the future.
> >> QEMU will now hard-code 64k as the maximum ACPI table size, which
> >> (despite being an order of magnitude smaller than 640k) should be enough
> >> for everyone.
> > 
> > Famous last words :) So what worries me here, is that we
> > are potentially breaking legal configurations for the
> > benefit of the minority that cares about cross-version
> > migration.
> > 
> > So I'm inclined to apply everything except this patch, and
> > instead, use the patches that I sent to make the
> > ram block very large, something like 1 Megabyte.
> 
> Even just 128k are enough for 160 VCPUs, 255 memory slots and 35-40 PCI
> bridges.  And for 2.2 I'd rather move to the other model where all
> user-defined elements (MADT, SSDT) are in a separate file and we
> guarantee that *all* changes are versioned by machine type.
> 
> What do you think about just changing 64k->128k?  Your patch is a huge
> amount of code for -rc4.
> 
> Paolo

True ...

OK I applied this, and made minor tweaks on top.
Any reviewers?

-- 
MST

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables
  2014-07-28 15:59   ` Michael S. Tsirkin
@ 2014-07-28 16:08     ` Paolo Bonzini
  2014-07-28 21:20       ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 16:08 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: imammedo, lersek, qemu-devel

Il 28/07/2014 17:59, Michael S. Tsirkin ha scritto:
> On Mon, Jul 28, 2014 at 05:34:16PM +0200, Paolo Bonzini wrote:
>> This patch avoids that similar changes break QEMU again in the future.
>> QEMU will now hard-code 64k as the maximum ACPI table size, which
>> (despite being an order of magnitude smaller than 640k) should be enough
>> for everyone.
> 
> Famous last words :) So what worries me here, is that we
> are potentially breaking legal configurations for the
> benefit of the minority that cares about cross-version
> migration.
> 
> So I'm inclined to apply everything except this patch, and
> instead, use the patches that I sent to make the
> ram block very large, something like 1 Megabyte.

Even just 128k are enough for 160 VCPUs, 255 memory slots and 35-40 PCI
bridges.  And for 2.2 I'd rather move to the other model where all
user-defined elements (MADT, SSDT) are in a separate file and we
guarantee that *all* changes are versioned by machine type.

What do you think about just changing 64k->128k?  Your patch is a huge
amount of code for -rc4.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables
  2014-07-28 15:34 ` [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables Paolo Bonzini
@ 2014-07-28 15:59   ` Michael S. Tsirkin
  2014-07-28 16:08     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2014-07-28 15:59 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: imammedo, lersek, qemu-devel

On Mon, Jul 28, 2014 at 05:34:16PM +0200, Paolo Bonzini wrote:
> This patch avoids that similar changes break QEMU again in the future.
> QEMU will now hard-code 64k as the maximum ACPI table size, which
> (despite being an order of magnitude smaller than 640k) should be enough
> for everyone.

Famous last words :) So what worries me here, is that we
are potentially breaking legal configurations for the
benefit of the minority that cares about cross-version
migration.

So I'm inclined to apply everything except this patch, and
instead, use the patches that I sent to make the
ram block very large, something like 1 Megabyte.

This localizes the pain to cross-version migration.


> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/acpi-build.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a3d5822..25cf297 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -62,6 +62,8 @@
>  #define ACPI_BUILD_LEGACY_CPU_AML_SIZE    97
>  #define ACPI_BUILD_ALIGN_SIZE             0x1000
>  
> +#define ACPI_BUILD_TABLE_SIZE             0x10000
> +
>  typedef struct AcpiCpuInfo {
>      DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>  } AcpiCpuInfo;
> @@ -1569,7 +1571,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          }
>          g_array_set_size(tables->table_data, legacy_table_size);
>      } else {
> -        acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
> +        if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
> +            /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots.  */
> +            error_report("ACPI tables are larger than 64k.  Please remove");
> +            error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
> +            exit(1);
> +        }
> +        g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
>      }
>  
>      acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables
  2014-07-28 15:34 [Qemu-devel] [PATCH v4 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
@ 2014-07-28 15:34 ` Paolo Bonzini
  2014-07-28 15:59   ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2014-07-28 15:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: imammedo, lersek, mst

This patch avoids that similar changes break QEMU again in the future.
QEMU will now hard-code 64k as the maximum ACPI table size, which
(despite being an order of magnitude smaller than 640k) should be enough
for everyone.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-build.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a3d5822..25cf297 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -62,6 +62,8 @@
 #define ACPI_BUILD_LEGACY_CPU_AML_SIZE    97
 #define ACPI_BUILD_ALIGN_SIZE             0x1000
 
+#define ACPI_BUILD_TABLE_SIZE             0x10000
+
 typedef struct AcpiCpuInfo {
     DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1569,7 +1571,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         }
         g_array_set_size(tables->table_data, legacy_table_size);
     } else {
-        acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
+        if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
+            /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots.  */
+            error_report("ACPI tables are larger than 64k.  Please remove");
+            error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
+            exit(1);
+        }
+        g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
     }
 
     acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-07-28 21:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-28 14:02 [Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
2014-07-28 14:02 ` [Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT Paolo Bonzini
2014-07-28 14:02 ` [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini
2014-07-28 15:05   ` Michael S. Tsirkin
2014-07-28 15:06     ` Paolo Bonzini
2014-07-28 14:02 ` [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables Paolo Bonzini
2014-07-28 14:02 ` [Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive Paolo Bonzini
2014-07-28 14:02 ` [Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled Paolo Bonzini
2014-07-28 15:34 [Qemu-devel] [PATCH v4 0/5] ACPI fixes for QEMU 2.1 Paolo Bonzini
2014-07-28 15:34 ` [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables Paolo Bonzini
2014-07-28 15:59   ` Michael S. Tsirkin
2014-07-28 16:08     ` Paolo Bonzini
2014-07-28 21:20       ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.