Attachment 886964 Details for Bug 1027565 – 4 patches for approach #1 (illustration only), mbox format

4 patches for approach #1 (illustration only), mbox format

approach_1.mbox (text/plain), 17.54 KB, created by Laszlo Ersek on 2014-04-16 17:45:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Laszlo Ersek

Created: 2014-04-16 17:45:22 UTC

Size: 17.54 KB

patch

obsolete

>From cd185a86f9632efee453d5f487e6c465dc53b747 Mon Sep 17 00:00:00 2001
>From: Laszlo Ersek <lersek@redhat.com>
>Date: Sun, 13 Apr 2014 01:21:57 +0200
>Subject: [RHEL-7.0 qemu-kvm PATCH 1/4] pc_sysfw: control parent MemoryRegion
> of isa-bios and pc.rom (RHEL only)
>
>(You *must* refer to bug 1027565 comment 20 (and then potentially to
>comment 16 too) for understanding my motivation for this patch.)
>
>In RHEL-6 qemu-kvm, the following two memory regions are in *RAM* address
>space for the guest:
>- the "pc.rom" RAMBlock, mapped from guest-phys 768 KB to 896 KB,
>- the appearance of the "pc.bios" RAMBlock that is mapped from guest-phys
>  896 KB to 1 MB.
>
>The appearance of the "pc.bios" RAMBlock that is mapped from guest-phys
>4GB-128KB to 4GB is in PCI address space (IO_MEM_ROM).
>
>In RHEL-7, the rhel6.x.0 machine types end up with a different memory map:
>- The "pc.rom" RAMBlock, mapped at the same guest-phys address range as on
>  a RHEL-6 host, is a child of "rom_memory". Since in RHEL-7 we always set
>  pci_enabled, rom_memory means pci_memory. That is, "pc.rom" ends up in
>  *PCI* address space.
>
>- The "pc.bios" RAMBlock is only mapped at 4GB-128KB, in PCI address space
>  (as a child of rom_memory). This is good (same as RHEL-6). Its
>  visibility under 1 MB is ensured with an alias called "isa-bios" that is
>  however the child of rom_memory -- the low mapping is visible in *PCI*
>  address space.
>
>This breaks the visibility of the BIOS in rhel6.x.0 machines that are
>migrated from RHEL-6 qemu-kvm to RHEL-7 qemu-kvm. At that point, the PAM
>registers say "pam-ram" and "pam-rom" (rather than "pam-pci"), which makes
>the "isa-bios" window (into "pc.bios") unreachable, and makes "pc.ram"
>visible.
>
>However, before the migration, SeaBIOS shadowed itself into "pc.bios" on
>the RHEL-6 source host, not "pc.ram". This means that rebooting a
>rhel6.x.0 VM after migration from RHEL-6 to RHEL-7 jumps into a bunch of
>zeroes, rather than the shadowed copy of SeaBIOS.
>
>This downstream-only patch makes the parent of "pc.rom" and "isa-bios"
>explicitly controllable ("pc.bios" is already fine). The patch has no
>observable effect, because:
>- "guest_info->isapc_ram_fw" is constant false for q35,
>- "guest_info->isapc_ram_fw" is (!pci_enabled) for piix4, which is
>  constant false in RHEL-7,
>- hence "low_fw_memory" will alias "rom_memory".
>
>Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>---
> include/hw/i386/pc.h |  1 +
> hw/i386/pc.c         |  7 +++++--
> hw/i386/pc_sysfw.c   | 12 ++++++++----
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>index 8a69347..f321bc1 100644
>--- a/include/hw/i386/pc.h
>+++ b/include/hw/i386/pc.h
>@@ -230,6 +230,7 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
> 
> /* pc_sysfw.c */
> void pc_system_firmware_init(MemoryRegion *rom_memory,
>+                             MemoryRegion *low_fw_memory,
>                              bool isapc_ram_fw);
> 
> /* pvpanic.c */
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index 1468d50..b973d2e 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -1136,6 +1136,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>     int linux_boot, i;
>     MemoryRegion *ram, *option_rom_mr;
>     MemoryRegion *ram_below_4g, *ram_above_4g;
>+    MemoryRegion *low_fw_memory;
>     FWCfgState *fw_cfg;
> 
>     linux_boot = (kernel_filename != NULL);
>@@ -1163,12 +1164,14 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> 
> 
>     /* Initialize PC system firmware */
>-    pc_system_firmware_init(rom_memory, guest_info->isapc_ram_fw);
>+    low_fw_memory = guest_info->isapc_ram_fw ? ram : rom_memory;
>+    pc_system_firmware_init(rom_memory, low_fw_memory,
>+                            guest_info->isapc_ram_fw);
> 
>     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>     memory_region_init_ram(option_rom_mr, "pc.rom", PC_ROM_SIZE);
>     vmstate_register_ram_global(option_rom_mr);
>-    memory_region_add_subregion_overlap(rom_memory,
>+    memory_region_add_subregion_overlap(low_fw_memory,
>                                         PC_ROM_MIN_VGA,
>                                         option_rom_mr,
>                                         1);
>diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>index 6f5ecde..ae24dbe 100644
>--- a/hw/i386/pc_sysfw.c
>+++ b/hw/i386/pc_sysfw.c
>@@ -170,7 +170,9 @@ static void pc_system_flash_init(MemoryRegion *rom_memory)
>     }
> }
> 
>-static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>+static void old_pc_system_rom_init(MemoryRegion *rom_memory,
>+                                   MemoryRegion *low_fw_memory,
>+                                   bool isapc_ram_fw)
> {
>     char *filename;
>     MemoryRegion *bios, *isa_bios;
>@@ -215,7 +217,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>     isa_bios = g_malloc(sizeof(*isa_bios));
>     memory_region_init_alias(isa_bios, "isa-bios", bios,
>                              bios_size - isa_bios_size, isa_bios_size);
>-    memory_region_add_subregion_overlap(rom_memory,
>+    memory_region_add_subregion_overlap(low_fw_memory,
>                                         0x100000 - isa_bios_size,
>                                         isa_bios,
>                                         1);
>@@ -229,7 +231,9 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>                                 bios);
> }
> 
>-void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>+void pc_system_firmware_init(MemoryRegion *rom_memory,
>+                             MemoryRegion *low_fw_memory,
>+                             bool isapc_ram_fw)
> {
>     DriveInfo *pflash_drv;
> 
>@@ -237,7 +241,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
> 
>     if (isapc_ram_fw || pflash_drv == NULL) {
>         /* When a pflash drive is not found, use rom-mode */
>-        old_pc_system_rom_init(rom_memory, isapc_ram_fw);
>+        old_pc_system_rom_init(rom_memory, low_fw_memory, isapc_ram_fw);
>         return;
>     }
> 
>-- 
>1.8.3.1
>
>
>From 83158fd193801b347012a26a0d09430b08309368 Mon Sep 17 00:00:00 2001
>From: Laszlo Ersek <lersek@redhat.com>
>Date: Tue, 15 Apr 2014 20:36:24 +0200
>Subject: [RHEL-7.0 qemu-kvm PATCH 2/4] piix: allow guest_info to disable PAM
> and SMRAM emulation (RHEL only)
>
>The mapping of the "pc.bios" RAMBlock (thru the "isa-bios" alias) and the
>mapping of the "pc.rom" RAMBlock into the Upper Memory Area walk hand in
>hand with the accuracy of the PAM registers' emulation.
>
>The previous patch allows "isa-bios" and "pc.rom" to appear as children of
>"pc.ram". In parallel, we must disable the PAM register emulation (bring
>it down to the RHEL-6 level), otherwise pam_update() will still be able to
>hide them, by switching to pam-pci.
>
>In addition, the RHEL-6 emulator's i440fx_update_memory_mappings()
>function is a no-op wrt. SMRAM visibility too (when running on KVM); SMRAM
>is never visible. Similarly, for a rhel6.x.0 machine type, the RHEL-7
>emulator must always hide SMRAM, with the "smram-region" alias being a
>window into PCI address space at all the times. In RHEL-7 it's not enough
>to short-circuit i440fx_update_memory_mappings() for this purpose, we must
>also change the default SMRAM visibility (which is the RHEL-6 emulator's
>opposite), and short-circuit another path to smram_update() that is new in
>RHEL-7:
>
>i440fx_set_smm()
>  smram_set_smm()
>    smram_update()
>
>(In the RHEL-6 emulator, i440fx_set_smm() goes through
>i440fx_update_memory_mappings().)
>
>Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>---
> include/hw/i386/pc.h |  3 ++-
> hw/i386/pc_piix.c    |  2 +-
> hw/pci-host/piix.c   | 18 +++++++++++++++---
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>index f321bc1..67ddddb 100644
>--- a/include/hw/i386/pc.h
>+++ b/include/hw/i386/pc.h
>@@ -191,7 +191,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
>                     hwaddr pci_hole_size,
>                     ram_addr_t above_4g_mem_size,
>                     MemoryRegion *pci_memory,
>-                    MemoryRegion *ram_memory);
>+                    MemoryRegion *ram_memory,
>+                    PcGuestInfo *guest_info);
> 
> PCIBus *find_i440fx(void);
> /* piix4.c */
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index 940816f..5555d57 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -167,7 +167,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>                               below_4g_mem_size,
>                               0x100000000ULL - below_4g_mem_size,
>                               above_4g_mem_size,
>-                              pci_memory, ram_memory);
>+                              pci_memory, ram_memory, guest_info);
>     } else {
>         pci_bus = NULL;
>         i440fx_state = NULL;
>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>index 5757b1d..3824f14 100644
>--- a/hw/pci-host/piix.c
>+++ b/hw/pci-host/piix.c
>@@ -104,6 +104,7 @@ struct PCII440FXState {
>     PAMMemoryRegion pam_regions[13];
>     MemoryRegion smram_region;
>     uint8_t smm_enabled;
>+    bool disable_pam_smram_on_kvm;
> };
> 
> 
>@@ -130,6 +131,10 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
> {
>     int i;
> 
>+    if (kvm_enabled() && d->disable_pam_smram_on_kvm) {
>+        return;
>+    }
>+
>     memory_region_transaction_begin();
>     for (i = 0; i < 13; i++) {
>         pam_update(&d->pam_regions[i], i,
>@@ -142,9 +147,14 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
> static void i440fx_set_smm(int val, void *arg)
> {
>     PCII440FXState *d = arg;
>+    unsigned clear = 0;
>+
>+    if (kvm_enabled() && d->disable_pam_smram_on_kvm) {
>+        clear = SMRAM_G_SMRAME | SMRAM_D_OPEN;
>+    }
> 
>     memory_region_transaction_begin();
>-    smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM],
>+    smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM] & ~clear,
>                   &d->smram_region);
>     memory_region_transaction_commit();
> }
>@@ -309,7 +319,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>                     hwaddr pci_hole_size,
>                     ram_addr_t above_4g_mem_size,
>                     MemoryRegion *pci_address_space,
>-                    MemoryRegion *ram_memory)
>+                    MemoryRegion *ram_memory,
>+                    PcGuestInfo *guest_info)
> {
>     DeviceState *dev;
>     PCIBus *b;
>@@ -360,7 +371,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>                              f->pci_address_space, 0xa0000, 0x20000);
>     memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
>                                         &f->smram_region, 1);
>-    memory_region_set_enabled(&f->smram_region, false);
>+    f->disable_pam_smram_on_kvm = guest_info->isapc_ram_fw;
>+    memory_region_set_enabled(&f->smram_region, f->disable_pam_smram_on_kvm);
>     init_pam(f->ram_memory, f->system_memory, f->pci_address_space,
>              &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
>     for (i = 0; i < 12; ++i) {
>-- 
>1.8.3.1
>
>
>From 19937d5a7b49f7a548f966a2d0a023b6c193c554 Mon Sep 17 00:00:00 2001
>From: Laszlo Ersek <lersek@redhat.com>
>Date: Wed, 16 Apr 2014 14:59:01 +0200
>Subject: [RHEL-7.0 qemu-kvm PATCH 3/4] pc_sysfw: control rewriting of pc.bios
> at reset (RHEL only)
>
>When old_pc_system_rom_init() receives a "true" argument for the
>"isapc_ram_fw" parameter, it leaves "pc.bios" writeable (which matches our
>intent, for matching RHEL-6).
>
>However, the "pc.bios" region being r/w causes rom_load_all() to set the
>corresponding "rom->isrom" field to false, which further triggers
>rom_reset() to overwrite "pc.bios" at reboot. "pc.bios" is never meant to
>be rewritten at reboot; in both RHEL-6 and RHEL-7 emulators. (RHEL-6
>checks for IO_MEM_ROM, which is set for the high-mapped pc.bios.)
>
>Add a rom_reset() override for the case when we leave "pc.bios" writeable
>to the guest.
>
>Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>---
> include/hw/loader.h |  3 +++
> hw/core/loader.c    | 11 ++++++++++-
> hw/i386/pc_sysfw.c  |  8 +++++++-
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
>diff --git a/include/hw/loader.h b/include/hw/loader.h
>index a5e02ce..f3553e7 100644
>--- a/include/hw/loader.h
>+++ b/include/hw/loader.h
>@@ -29,6 +29,9 @@ extern bool rom_file_has_mr;
> int rom_add_file(const char *file, const char *fw_dir,
>                  hwaddr addr, int32_t bootindex,
>                  bool option_rom);
>+int rom_add_file2(const char *file, const char *fw_dir,
>+                 hwaddr addr, int32_t bootindex,
>+                 bool option_rom, bool write_once);
> void *rom_add_blob(const char *name, const void *blob, size_t len,
>                    hwaddr addr, const char *fw_file_name,
>                    FWCfgReadCallback fw_callback, void *callback_opaque);
>diff --git a/hw/core/loader.c b/hw/core/loader.c
>index 9309b8c..c9afa3b 100644
>--- a/hw/core/loader.c
>+++ b/hw/core/loader.c
>@@ -547,6 +547,7 @@ struct Rom {
>     uint8_t *data;
>     MemoryRegion *mr;
>     int isrom;
>+    bool write_once;
>     char *fw_dir;
>     char *fw_file;
> 
>@@ -594,6 +595,13 @@ int rom_add_file(const char *file, const char *fw_dir,
>                  hwaddr addr, int32_t bootindex,
>                  bool option_rom)
> {
>+    return rom_add_file2(file, fw_dir, addr, bootindex, option_rom, false);
>+}
>+
>+int rom_add_file2(const char *file, const char *fw_dir,
>+                 hwaddr addr, int32_t bootindex,
>+                 bool option_rom, bool write_once)
>+{
>     Rom *rom;
>     int rc, fd = -1;
>     char devpath[100];
>@@ -628,6 +636,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>         goto err;
>     }
>     close(fd);
>+    rom->write_once = write_once;
>     rom_insert(rom);
>     if (rom->fw_file && fw_cfg) {
>         const char *basename;
>@@ -748,7 +757,7 @@ static void rom_reset(void *unused)
>         } else {
>             cpu_physical_memory_write_rom(rom->addr, rom->data, rom->datasize);
>         }
>-        if (rom->isrom) {
>+        if (rom->isrom || rom->write_once) {
>             /* rom needs to be written only once */
>             g_free(rom->data);
>             rom->data = NULL;
>diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>index ae24dbe..3ef6318 100644
>--- a/hw/i386/pc_sysfw.c
>+++ b/hw/i386/pc_sysfw.c
>@@ -199,7 +199,13 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory,
>     if (!isapc_ram_fw) {
>         memory_region_set_readonly(bios, true);
>     }
>-    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
>+    ret = rom_add_file2(bios_name,              /* file */
>+                        NULL,                   /* fw_dir */
>+                        (uint32_t)(-bios_size), /* addr */
>+                        -1,                     /* bootindex */
>+                        false,                  /* option_rom */
>+                        isapc_ram_fw            /* write_once */
>+                        );
>     if (ret != 0) {
>     bios_error:
>         fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
>-- 
>1.8.3.1
>
>
>From 3d00b4675c709fd309480340652be214a831a027 Mon Sep 17 00:00:00 2001
>From: Laszlo Ersek <lersek@redhat.com>
>Date: Sun, 13 Apr 2014 01:48:35 +0200
>Subject: [RHEL-7.0 qemu-kvm PATCH 4/4] pc: force isapc_ram_fw for rhel6.x.0
> machine types (RHEL only)
>
>With the following consequences:
>
>    pc_init1() [hw/i386/pc_piix.c]
>        pc_memory_init() [hw/i386/pc.c]
>            pc_system_firmware_init() [hw/i386/pc_sysfw.c]
>                old_pc_system_rom_init()
>                    // "pc.bios" becomes writeable, matching RHEL-6
>                    // the "isa-bios" window into "pc.bios" becomes a
>                    //      child of "pc.ram", matching RHEL-6
>                    // "isa-bios" becomes writeable, matching RHEL-6
>            // "pc.rom" becomes a child of "pc.ram", matching RHEL-6
>        i440fx_init()
>            // makes "smram-region" a permanent window into PCI address
>            //     space, matching RHEL-6
>            // turns i440fx_update_memory_mappings() into a no-op, just
>            //     like it is on RHEL-6
>
>Note that this change doesn't effect the
>pc_system_firmware_init()-->old_pc_system_rom_init() call itself -- that
>is, this patch doesn't change the
>
>    (isapc_ram_fw || pflash_drv == NULL)
>
>condition in pc_system_firmware_init().
>
>That condition would be evaluated differently (ie. it would go from false
>to true as a consequence of this patch) *only* if the following held:
>
>    (isapc_ram_fw && pflash_drv != NULL)
>
>But that is false in all of the following cases:
>- pc-q35-rhel7.x.0 machine types (first requirement fails).
>- pc-i440fx-rhel7.x.0 machine types (first requirement fails).
>- rhel6.x.0 machine types (second requirement fails).
>
>Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>---
> hw/i386/pc_piix.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index 5555d57..7d96938 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -67,6 +67,8 @@ static bool has_acpi_build = true;
>  */
> static bool gigabyte_align = true;
> 
>+static bool force_isapc_ram_fw;
>+
> /* PC hardware initialisation */
> static void pc_init1(QEMUMachineInitArgs *args,
>                      MemoryRegion *system_memory,
>@@ -141,7 +143,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>     guest_info->has_acpi_build = has_acpi_build;
> 
>     guest_info->has_pci_info = has_pci_info;
>-    guest_info->isapc_ram_fw = !pci_enabled;
>+    guest_info->isapc_ram_fw = !pci_enabled || force_isapc_ram_fw;
> 
>     /* allocate ram and load rom/bios */
>     if (!xen_enabled()) {
>@@ -954,6 +956,7 @@ static void pc_compat_rhel650(QEMUMachineInitArgs *args)
>     rom_file_has_mr = false; 
>     has_acpi_build = false;
>     gigabyte_align = false;
>+    force_isapc_ram_fw = true;
> }
> 
> static void pc_init_rhel650(QEMUMachineInitArgs *args)
>-- 
>1.8.3.1
>

Actions: View

Attachments on bug 1027565: 885244 | 885245 | 885252 | 885649 | 885650 | 885795 | 885838 | 886494 | 886593 | 886840 | 886841 | 886964 | 888022 | 888273