* [Qemu-devel] [PATCH v2 for-2.1 0/2] pc: fix /etc/acpi/tables size in fw_cfg for -M pc-i440fx-2.0 @ 2014-07-24 14:32 Paolo Bonzini 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT Paolo Bonzini 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini 0 siblings, 2 replies; 8+ messages in thread From: Paolo Bonzini @ 2014-07-24 14:32 UTC (permalink / raw) To: qemu-devel; +Cc: amit.shah, peter.maydell, lersek, dgilbert, mst Addressing Laszlo and Igor's review points. Testing would be appreciated. :) Paolo Paolo Bonzini (2): acpi-dsdt: procedurally generate _PRT pc: hack for migration compatibility from QEMU 2.0 hw/i386/acpi-build.c | 71 +- 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 + 6 files changed, 240 insertions(+), 1856 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT 2014-07-24 14:32 [Qemu-devel] [PATCH v2 for-2.1 0/2] pc: fix /etc/acpi/tables size in fw_cfg for -M pc-i440fx-2.0 Paolo Bonzini @ 2014-07-24 14:32 ` Paolo Bonzini 2014-07-24 16:13 ` Laszlo Ersek 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2014-07-24 14:32 UTC (permalink / raw) To: qemu-devel; +Cc: amit.shah, peter.maydell, lersek, dgilbert, 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. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- include cpi-dsdt.hex.generated [Igor] hw/i386/acpi-dsdt.dsl | 90 +- hw/i386/acpi-dsdt.hex.generated | 1910 +++------------------------------------ 2 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, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT Paolo Bonzini @ 2014-07-24 16:13 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2014-07-24 16:13 UTC (permalink / raw) To: Paolo Bonzini, qemu-devel; +Cc: amit.shah, peter.maydell, dgilbert, mst On 07/24/14 16:32, Paolo Bonzini wrote: > 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. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > include cpi-dsdt.hex.generated [Igor] > > hw/i386/acpi-dsdt.dsl | 90 +- > hw/i386/acpi-dsdt.hex.generated | 1910 +++------------------------------------ > 2 files changed, 148 insertions(+), 1852 deletions(-) Compared with v1, v2 1/2 seems to reword the commit message a little bit, and add the generated file (requested by Igor, and mentioned by you anyway). Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 2014-07-24 14:32 [Qemu-devel] [PATCH v2 for-2.1 0/2] pc: fix /etc/acpi/tables size in fw_cfg for -M pc-i440fx-2.0 Paolo Bonzini 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT Paolo Bonzini @ 2014-07-24 14:32 ` Paolo Bonzini 2014-07-24 16:29 ` Laszlo Ersek 2014-07-28 11:45 ` Michael S. Tsirkin 1 sibling, 2 replies; 8+ messages in thread From: Paolo Bonzini @ 2014-07-24 14:32 UTC (permalink / raw) To: qemu-devel; +Cc: amit.shah, peter.maydell, lersek, dgilbert, 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. To fix this, hard-code 64k as the maximum ACPI table size, which (despite being an order of magnitude smaller than 640k) should be enough for everyone. To fix migration from QEMU 2.0, compute the payload size of QEMU 2.0 and always use that one. The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size should always be enough. 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. Even with this patch, QEMU 1.7 and 2.0 have two different ideas of "-M pc-i440fx-2.0" when there are PCI bridges. Igor sent a patch to adopt the QEMU 1.7 definition. I think distributions should apply it if they move directly from QEMU 1.7 to 2.1+ without ever packaging version 2.0. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- replace magic constants with #defines [Igor] remove stray line from comment [Laszlo] hw/i386/acpi-build.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++--- hw/i386/pc_piix.c | 19 ++++++++++++++ hw/i386/pc_q35.c | 5 ++++ include/hw/i386/pc.h | 1 + 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index ebc5f03..26d8dfa 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,16 @@ #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_CPU_AML_SIZE 97 +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875 + +#define ACPI_BUILD_TABLE_SIZE 0x10000 + typedef struct AcpiCpuInfo { DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT); } AcpiCpuInfo; @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState { struct AcpiBuildPciBusHotplugState *parent; } AcpiBuildPciBusHotplugState; +unsigned bsel_alloc; + static void acpi_get_dsdt(AcpiMiscInfo *info) { uint16_t *applesmc_sta; @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) static void acpi_set_pci_info(void) { PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ - unsigned bsel_alloc = 0; + assert(bsel_alloc == 0); if (bus) { /* Scan all PCI buses. Set property to enable acpi based hotplug. */ pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); @@ -1440,13 +1454,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 +1489,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,12 +1535,53 @@ 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 legacy_aml_len = + guest_info->legacy_acpi_table_size + + ACPI_BUILD_CPU_AML_SIZE * max_cpus + + ACPI_BUILD_BRIDGE_AML_SIZE * (MAX(bsel_alloc, 1) - 1); + int legacy_table_size = + ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len, 0x1000); + 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("Please report this to qemu-devel@nongnu.org."); + exit(1); + } + g_array_set_size(tables->table_data, legacy_table_size); + } else { + 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, 0x1000); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7081c08..c8a8090 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 6358. + */ + 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] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini @ 2014-07-24 16:29 ` Laszlo Ersek 2014-07-24 16:30 ` Paolo Bonzini 2014-07-28 11:45 ` Michael S. Tsirkin 1 sibling, 1 reply; 8+ messages in thread From: Laszlo Ersek @ 2014-07-24 16:29 UTC (permalink / raw) To: Paolo Bonzini, qemu-devel; +Cc: amit.shah, peter.maydell, dgilbert, mst On 07/24/14 16:32, 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. > > To fix this, hard-code 64k as the maximum ACPI table size, which > (despite being an order of magnitude smaller than 640k) should be enough > for everyone. > > To fix migration from QEMU 2.0, compute the payload size of QEMU 2.0 > and always use that one. The previous patch shrunk the ACPI tables > enough that the QEMU 2.0 size should always be enough. > > 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. > > Even with this patch, QEMU 1.7 and 2.0 have two different ideas of > "-M pc-i440fx-2.0" when there are PCI bridges. Igor sent a patch to > adopt the QEMU 1.7 definition. I think distributions should apply > it if they move directly from QEMU 1.7 to 2.1+ without ever packaging > version 2.0. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > replace magic constants with #defines [Igor] > remove stray line from comment [Laszlo] I compared this too with its v1 counterpart, and it looks good. I have one question (just curiosity): the following paragraph was dropped from the commit message -- why? -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. I think this remains true in v2 as well: - "aml_len" and "legacy_aml_len" still "only" cover the DSDT and the SSDT, and - the non-AML tables (eg. the MADT, now spelled out in the commit message), although they may grow with the number of CPUs, continue to remain the same between 2.0 and 2.1. IOW, I think you could have kept this paragraph if you wanted to. Was it an oversight to drop it, or did the paragraph contain something incorrect (in v1) that I'm unaware of? Or is it just redundant? Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 2014-07-24 16:29 ` Laszlo Ersek @ 2014-07-24 16:30 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2014-07-24 16:30 UTC (permalink / raw) To: Laszlo Ersek, qemu-devel; +Cc: amit.shah, peter.maydell, dgilbert, mst Il 24/07/2014 18:29, Laszlo Ersek ha scritto: > I compared this too with its v1 counterpart, and it looks good. I have > one question (just curiosity): the following paragraph was dropped from > the commit message -- why? > > -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. > > I think this remains true in v2 as well: > - "aml_len" and "legacy_aml_len" still "only" cover the DSDT and the > SSDT, and > - the non-AML tables (eg. the MADT, now spelled out in the commit > message), although they may grow with the number of CPUs, continue to > remain the same between 2.0 and 2.1. > > IOW, I think you could have kept this paragraph if you wanted to. Was it > an oversight to drop it, or did the paragraph contain something > incorrect (in v1) that I'm unaware of? Or is it just redundant? An oversight. I had added it to the mail before sending it, not directly in the commit message. I'll add it back for the pull request (tomorrow morning). Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini 2014-07-24 16:29 ` Laszlo Ersek @ 2014-07-28 11:45 ` Michael S. Tsirkin 2014-07-28 12:40 ` Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2014-07-28 11:45 UTC (permalink / raw) To: Paolo Bonzini; +Cc: amit.shah, peter.maydell, lersek, qemu-devel, dgilbert On Thu, Jul 24, 2014 at 04:32:09PM +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. > > To fix this, hard-code 64k as the maximum ACPI table size, which > (despite being an order of magnitude smaller than 640k) should be enough > for everyone. > > To fix migration from QEMU 2.0, compute the payload size of QEMU 2.0 > and always use that one. The previous patch shrunk the ACPI tables > enough that the QEMU 2.0 size should always be enough. > > 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. > > Even with this patch, QEMU 1.7 and 2.0 have two different ideas of > "-M pc-i440fx-2.0" when there are PCI bridges. Igor sent a patch to > adopt the QEMU 1.7 definition. I think distributions should apply > it if they move directly from QEMU 1.7 to 2.1+ without ever packaging > version 2.0. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > replace magic constants with #defines [Igor] > remove stray line from comment [Laszlo] > > hw/i386/acpi-build.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++--- > hw/i386/pc_piix.c | 19 ++++++++++++++ > hw/i386/pc_q35.c | 5 ++++ > include/hw/i386/pc.h | 1 + > 4 files changed, 92 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index ebc5f03..26d8dfa 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,16 @@ > #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. Let's just say 2.0 and earlier? > 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_CPU_AML_SIZE 97 > +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875 Let's put _LEGACY_ somewhere here? > + > +#define ACPI_BUILD_TABLE_SIZE 0x10000 > + > typedef struct AcpiCpuInfo { > DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT); > } AcpiCpuInfo; > @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState { > struct AcpiBuildPciBusHotplugState *parent; > } AcpiBuildPciBusHotplugState; > > +unsigned bsel_alloc; > + Patch will be better contained if instead of using a global bsel_alloc, we actually go and count the devices that have ACPI_PCIHP_PROP_BSEL. You can just scan all devices, or all pci devices, it should not matter. This way, this code will be local to the legacy path. > static void acpi_get_dsdt(AcpiMiscInfo *info) > { > uint16_t *applesmc_sta; > @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > static void acpi_set_pci_info(void) > { > PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > - unsigned bsel_alloc = 0; > > + assert(bsel_alloc == 0); > if (bus) { > /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > @@ -1440,13 +1454,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 +1489,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,12 +1535,53 @@ 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 legacy_aml_len = > + guest_info->legacy_acpi_table_size + > + ACPI_BUILD_CPU_AML_SIZE * max_cpus + > + ACPI_BUILD_BRIDGE_AML_SIZE * (MAX(bsel_alloc, 1) - 1); > + int legacy_table_size = > + ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len, 0x1000); > + 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("Please report this to qemu-devel@nongnu.org."); Downstreams would need to patch this line out? Let's just drop the second line. > + exit(1); so what happens here is 2.x -> 2.0 migration becomes broken, but 2.0 -> 2.x could still work. I think I would prefer a warning instead. > + } > + g_array_set_size(tables->table_data, legacy_table_size); > + } else { > + 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); Let's split the idea to use 64K always out, to a separate patch, ok? > + } > > acpi_align_size(tables->linker, 0x1000); > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 7081c08..c8a8090 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 6358. > + */ > + 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] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 2014-07-28 11:45 ` Michael S. Tsirkin @ 2014-07-28 12:40 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2014-07-28 12:40 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: amit.shah, peter.maydell, lersek, qemu-devel, dgilbert Il 28/07/2014 13:45, 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. > > Let's just say 2.0 and earlier? This would give the idea that 1.6 is broken, but it isn't. >> 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_CPU_AML_SIZE 97 >> +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875 > > Let's put _LEGACY_ somewhere here? Ok. >> + >> +#define ACPI_BUILD_TABLE_SIZE 0x10000 >> + >> typedef struct AcpiCpuInfo { >> DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT); >> } AcpiCpuInfo; >> @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState { >> struct AcpiBuildPciBusHotplugState *parent; >> } AcpiBuildPciBusHotplugState; >> >> +unsigned bsel_alloc; >> + > > Patch will be better contained if instead of using a global > bsel_alloc, we actually go and count the devices that > have ACPI_PCIHP_PROP_BSEL. > You can just scan all devices, or all pci devices, it > should not matter. > This way, this code will be local to the legacy path. Ok. > >> static void acpi_get_dsdt(AcpiMiscInfo *info) >> { >> uint16_t *applesmc_sta; >> @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) >> static void acpi_set_pci_info(void) >> { >> PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ >> - unsigned bsel_alloc = 0; >> >> + assert(bsel_alloc == 0); >> if (bus) { >> /* Scan all PCI buses. Set property to enable acpi based hotplug. */ >> pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); >> @@ -1440,13 +1454,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 +1489,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,12 +1535,53 @@ 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 legacy_aml_len = >> + guest_info->legacy_acpi_table_size + >> + ACPI_BUILD_CPU_AML_SIZE * max_cpus + >> + ACPI_BUILD_BRIDGE_AML_SIZE * (MAX(bsel_alloc, 1) - 1); >> + int legacy_table_size = >> + ROUND_UP(tables->table_data->len - aml_len + legacy_aml_len, 0x1000); >> + 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("Please report this to qemu-devel@nongnu.org."); > > Downstreams would need to patch this line out? Let's just drop the > second line. This should never happen really, so... >> + exit(1); > > > so what happens here is 2.x -> 2.0 migration becomes broken, but > 2.0 -> 2.x could still work. Only with Igor's patches. > I think I would prefer a warning instead. > >> + } >> + g_array_set_size(tables->table_data, legacy_table_size); >> + } else { >> + 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); > > > Let's split the idea to use 64K always out, to a separate patch, ok? Ok. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-07-28 12:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-24 14:32 [Qemu-devel] [PATCH v2 for-2.1 0/2] pc: fix /etc/acpi/tables size in fw_cfg for -M pc-i440fx-2.0 Paolo Bonzini 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 1/2] acpi-dsdt: procedurally generate _PRT Paolo Bonzini 2014-07-24 16:13 ` Laszlo Ersek 2014-07-24 14:32 ` [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0 Paolo Bonzini 2014-07-24 16:29 ` Laszlo Ersek 2014-07-24 16:30 ` Paolo Bonzini 2014-07-28 11:45 ` Michael S. Tsirkin 2014-07-28 12:40 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).