qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Cc: pbonzini@redhat.com, marcel.a@redhat.com, mst@redhat.com
Subject: [Qemu-devel] [PATCH 2/2] pc: acpi-build: make linker & RSDP tables dynamic
Date: Wed, 10 Dec 2014 16:06:56 +0000	[thread overview]
Message-ID: <1418227616-18149-3-git-send-email-imammedo@redhat.com> (raw)
In-Reply-To: <1418227616-18149-1-git-send-email-imammedo@redhat.com>

linker and RSDP tables are build only once, so if later
during rebuild sizes of other ACPI tables change
pointers will be patched incorrectly due to wrong
offsets.

To fix it rebuild linker and RSDP tables along with
the rest of ACPI tables so that they would have correct
offsets.

Here is a simple reproducer:
 1: hotplug bridge using command:
     device_add pci-bridge,chassis_nr=1
 2: reset system from monitor:
     system_reset

As result pointers to ACPI tables are not correct
and guest can't read/parse ACPI tables.

Windows guests just refuse to boot and
Linux guests are more resilient and try to boot without
ACPI, sometimes successfully.

PS:
 keep brokenness in 2.2 and older machine types for the sake
 of migration

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Tested backwards migration from QEMU 2.3 to 2.2 with
pc-i440fx-2.2 machine type

 hw/i386/acpi-build.c | 30 +++++++++++++++++++++++-------
 hw/i386/pc_piix.c    |  3 +++
 hw/i386/pc_q35.c     |  3 +++
 include/hw/i386/pc.h |  1 +
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b37a397..4d2452d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1509,6 +1509,10 @@ struct AcpiBuildState {
     /* Copy of table in RAM (for patching). */
     ram_addr_t table_ram;
     uint32_t table_size;
+    ram_addr_t linker_ram;
+    uint32_t linker_size;
+    ram_addr_t rsdp_ram;
+    uint32_t rsdp_size;
     /* Is table patched? */
     uint8_t patched;
     PcGuestInfo *guest_info;
@@ -1714,6 +1718,10 @@ static void acpi_build_update(void *build_opaque, uint32_t offset)
     assert(acpi_data_len(tables.table_data) == build_state->table_size);
     memcpy(qemu_get_ram_ptr(build_state->table_ram), tables.table_data->data,
            build_state->table_size);
+    memcpy(qemu_get_ram_ptr(build_state->linker_ram), tables.linker->data,
+           build_state->linker_size);
+    memcpy(qemu_get_ram_ptr(build_state->rsdp_ram), tables.rsdp->data,
+           build_state->rsdp_size);
 
     cpu_physical_memory_set_dirty_range_nocode(build_state->table_ram,
                                                build_state->table_size);
@@ -1779,17 +1787,25 @@ void acpi_setup(PcGuestInfo *guest_info)
     assert(build_state->table_ram != RAM_ADDR_MAX);
     build_state->table_size = acpi_data_len(tables.table_data);
 
-    acpi_add_rom_blob(NULL, tables.linker, "etc/table-loader");
+    build_state->linker_ram = acpi_add_rom_blob(build_state, tables.linker,
+                                                "etc/table-loader");
+    build_state->linker_size = acpi_data_len(tables.linker);
 
     fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
-    /*
-     * RSDP is small so it's easy to keep it immutable, no need to
-     * bother with ROM blobs.
-     */
-    fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_RSDP_FILE,
-                    tables.rsdp->data, acpi_data_len(tables.rsdp));
+    if (guest_info->has_imutable_rsdp) {
+        /*
+         * RSDP is small so it's easy to keep it immutable, no need to
+         * bother with ROM blobs.
+         */
+        fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_RSDP_FILE,
+                        tables.rsdp->data, acpi_data_len(tables.rsdp));
+    } else {
+        build_state->rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp,
+                                                  ACPI_BUILD_RSDP_FILE);
+        build_state->rsdp_size = acpi_data_len(tables.rsdp);
+    }
 
     qemu_register_reset(acpi_build_reset, build_state);
     acpi_build_reset(build_state);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 685fa54..69d4de2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,6 +60,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_acpi_build = true;
+static bool has_imutable_rsdp;
 static int legacy_acpi_table_size;
 static bool smbios_defaults = true;
 static bool smbios_legacy_mode;
@@ -168,6 +169,7 @@ static void pc_init1(MachineState *machine,
 
     guest_info->isapc_ram_fw = !pci_enabled;
     guest_info->has_reserved_memory = has_reserved_memory;
+    guest_info->has_imutable_rsdp = has_imutable_rsdp;
 
     if (smbios_defaults) {
         MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -310,6 +312,7 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_2(MachineState *machine)
 {
+    has_imutable_rsdp = true;
 }
 
 static void pc_compat_2_1(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 121f620..6bbc100 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,6 +50,7 @@
 #define MAX_SATA_PORTS     6
 
 static bool has_acpi_build = true;
+static bool has_imutable_rsdp;
 static bool smbios_defaults = true;
 static bool smbios_legacy_mode;
 static bool smbios_uuid_encoded = true;
@@ -154,6 +155,7 @@ static void pc_q35_init(MachineState *machine)
     guest_info->isapc_ram_fw = false;
     guest_info->has_acpi_build = has_acpi_build;
     guest_info->has_reserved_memory = has_reserved_memory;
+    guest_info->has_imutable_rsdp = has_imutable_rsdp;
 
     /* Migration was not supported in 2.0 for Q35, so do not bother
      * with this hack (see hw/i386/acpi-build.c).
@@ -289,6 +291,7 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_2(MachineState *machine)
 {
+    has_imutable_rsdp = true;
 }
 
 static void pc_compat_2_1(MachineState *machine)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 69d9cf8..acc95ea 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -104,6 +104,7 @@ struct PcGuestInfo {
     int legacy_acpi_table_size;
     bool has_acpi_build;
     bool has_reserved_memory;
+    bool has_imutable_rsdp;
 };
 
 /* parallel.c */
-- 
1.8.3.1

  parent reply	other threads:[~2014-12-10 16:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10 16:06 [Qemu-devel] [PATCH 0/2] pc: fix ACPI tables corruption after bridge hotplug Igor Mammedov
2014-12-10 16:06 ` [Qemu-devel] [PATCH 1/2] pc: Add pc-[i440fx|q35]-2.3 machine type and pc_compat_2_2() function Igor Mammedov
2014-12-10 16:06 ` Igor Mammedov [this message]
2014-12-10 16:24   ` [Qemu-devel] [PATCH 2/2] pc: acpi-build: make linker & RSDP tables dynamic Michael S. Tsirkin
2014-12-10 16:45     ` Igor Mammedov
2014-12-10 17:13       ` Michael S. Tsirkin
2014-12-10 17:39         ` Igor Mammedov
2014-12-10 18:28           ` Michael S. Tsirkin
2014-12-10 19:43             ` Igor Mammedov
2014-12-10 20:18               ` Michael S. Tsirkin
2014-12-11  8:36                 ` Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418227616-18149-3-git-send-email-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).