* [Qemu-devel] [PATCH 2/3] pc: Split pc_init_pci_1_0() off pc_init_pci_1_2()
2013-04-12 15:25 [Qemu-devel] [PATCH 0/3] Kill the "use flash device for BIOS unless KVM" misfeature Markus Armbruster
2013-04-12 15:25 ` [Qemu-devel] [PATCH 1/3] pc: Inline pc_init_pci_1_3() into pc_init_pci_1_2() Markus Armbruster
@ 2013-04-12 15:25 ` Markus Armbruster
2013-04-12 15:25 ` [Qemu-devel] [PATCH 3/3] pc: Kill the "use flash device for BIOS unless KVM" misfeature Markus Armbruster
2013-04-25 20:55 ` [Qemu-devel] [PATCH 0/3] " Anthony Liguori
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2013-04-12 15:25 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, jljusten
Just to make the next commit easier to review.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/i386/pc_piix.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index da34d3b..f2d3d27 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -240,7 +240,7 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
pc_init_pci(args);
}
-/* PC machine init function for pc-0.14 to pc-1.2 */
+/* PC machine init function for pc-1.1 to pc-1.2 */
static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
{
disable_kvm_pv_eoi();
@@ -248,6 +248,14 @@ static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
pc_init_pci(args);
}
+/* PC machine init function for pc-0.14 to pc-1.0 */
+static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
+{
+ disable_kvm_pv_eoi();
+ enable_compat_apic_id_mode();
+ pc_init_pci(args);
+}
+
/* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
{
@@ -461,7 +469,7 @@ static QEMUMachine pc_machine_v1_1 = {
static QEMUMachine pc_machine_v1_0 = {
.name = "pc-1.0",
.desc = "Standard PC",
- .init = pc_init_pci_1_2,
+ .init = pc_init_pci_1_0,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_0,
@@ -477,7 +485,7 @@ static QEMUMachine pc_machine_v1_0 = {
static QEMUMachine pc_machine_v0_15 = {
.name = "pc-0.15",
.desc = "Standard PC",
- .init = pc_init_pci_1_2,
+ .init = pc_init_pci_1_0,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_15,
@@ -510,7 +518,7 @@ static QEMUMachine pc_machine_v0_15 = {
static QEMUMachine pc_machine_v0_14 = {
.name = "pc-0.14",
.desc = "Standard PC",
- .init = pc_init_pci_1_2,
+ .init = pc_init_pci_1_0,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_14,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 3/3] pc: Kill the "use flash device for BIOS unless KVM" misfeature
2013-04-12 15:25 [Qemu-devel] [PATCH 0/3] Kill the "use flash device for BIOS unless KVM" misfeature Markus Armbruster
2013-04-12 15:25 ` [Qemu-devel] [PATCH 1/3] pc: Inline pc_init_pci_1_3() into pc_init_pci_1_2() Markus Armbruster
2013-04-12 15:25 ` [Qemu-devel] [PATCH 2/3] pc: Split pc_init_pci_1_0() off pc_init_pci_1_2() Markus Armbruster
@ 2013-04-12 15:25 ` Markus Armbruster
2013-04-25 20:55 ` [Qemu-devel] [PATCH 0/3] " Anthony Liguori
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2013-04-12 15:25 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, jljusten
Use of a flash memory device for the BIOS was added in series "[PATCH
v10 0/8] PC system flash support", commit 4732dca..1b89faf, v1.1.
Flash vs. ROM is a guest-visible difference. Thus, flash use had to
be suppressed for machine types pc-1.0 and older. This was
accomplished by adding a dummy device "pc-sysfw" with property
"rom_only":
* Non-zero rom_only means "use ROM". Default for pc-1.0 and older.
* Zero rom_only means "maybe use flash". Default for newer machines.
Not only is the dummy device ugly, it was also retroactively added to
the older machine types! Fortunately, it's not guest-visible (thus no
immediate guest ABI breakage), and has no vmstate (thus no immediate
migration breakage). Breakage occurs only if the user unwisely
enables flash by setting rom_only to zero. Patch review FAIL #1.
Why "maybe use flash"? Flash didn't (and still doesn't) work with
KVM. Therefore, rom_only=0 really means "use flash, except when KVM
is enabled, use ROM". This is a Bad Idea, because it makes enabling/
disabling KVM guest-visible. Patch review FAIL #2.
Aside: it also precludes migrating between KVM on and off, but that's
not possible for other reasons anyway.
Fix as follows:
1. Change the meaning of rom_only=0 to mean "use flash, no ifs, buts,
or maybes" for pc-i440fx-1.5 and pc-q35-1.5. Don't change anything
for older machines (to remain bug-compatible).
2. Change the default value from 0 to 1 for these machines.
Necessary, because 0 doesn't work with KVM. Once it does, we can flip
the default back to 0.
3. Don't revert the retroactive addition of device "pc-sysfw" to older
machine types. Seems not worth the trouble.
4. Add a TODO comment asking for device "pc-sysfw" to be dropped once
flash works with KVM.
Net effect is that you get a BIOS ROM again even when KVM is disabled,
just like for machines predating the introduction of flash.
To get flash instead, use "--global pc-sysfw.rom_only=0".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/block/pc_sysfw.c | 20 ++++++++++++++++++--
hw/i386/pc_piix.c | 10 +++++++++-
hw/i386/pc_q35.c | 8 +++++++-
include/hw/i386/pc.h | 5 +++++
4 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/hw/block/pc_sysfw.c b/hw/block/pc_sysfw.c
index 0d95c8a..aad8614 100644
--- a/hw/block/pc_sysfw.c
+++ b/hw/block/pc_sysfw.c
@@ -194,11 +194,23 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
bios);
}
+/*
+ * Bug-compatible flash vs. ROM selection enabled?
+ * A few older machines enable this.
+ */
+bool pc_sysfw_flash_vs_rom_bug_compatible;
+
void pc_system_firmware_init(MemoryRegion *rom_memory)
{
DriveInfo *pflash_drv;
PcSysFwDevice *sysfw_dev;
+ /*
+ * TODO This device exists only so that users can switch between
+ * use of flash and ROM for the BIOS. The ability to switch was
+ * created because flash doesn't work with KVM. Once it does, we
+ * should drop this device for new machine types.
+ */
sysfw_dev = (PcSysFwDevice*) qdev_create(NULL, "pc-sysfw");
qdev_init_nofail(DEVICE(sysfw_dev));
@@ -212,7 +224,11 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
/* Currently KVM cannot execute from device memory.
Use old rom based firmware initialization for KVM. */
- if (kvm_enabled()) {
+ /*
+ * This is a Bad Idea, because it makes enabling/disabling KVM
+ * guest-visible. Do it only in bug-compatibility mode.
+ */
+ if (pc_sysfw_flash_vs_rom_bug_compatible && kvm_enabled()) {
if (pflash_drv != NULL) {
fprintf(stderr, "qemu: pflash cannot be used with kvm enabled\n");
exit(1);
@@ -239,7 +255,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
}
static Property pcsysfw_properties[] = {
- DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 0),
+ DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 1),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f2d3d27..9b01a0e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -234,9 +234,16 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
initrd_filename, cpu_model, 1, 1);
}
+static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+{
+ pc_sysfw_flash_vs_rom_bug_compatible = true;
+ pc_init_pci(args);
+}
+
static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
{
enable_compat_apic_id_mode();
+ pc_sysfw_flash_vs_rom_bug_compatible = true;
pc_init_pci(args);
}
@@ -245,6 +252,7 @@ static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
{
disable_kvm_pv_eoi();
enable_compat_apic_id_mode();
+ pc_sysfw_flash_vs_rom_bug_compatible = true;
pc_init_pci(args);
}
@@ -317,7 +325,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
static QEMUMachine pc_i440fx_machine_v1_4 = {
.name = "pc-i440fx-1.4",
.desc = "Standard PC (i440FX + PIIX, 1996)",
- .init = pc_init_pci,
+ .init = pc_init_pci_1_4,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_4,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6ac1a89..8f0c3f7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -209,6 +209,12 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
}
}
+static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
+{
+ pc_sysfw_flash_vs_rom_bug_compatible = true;
+ pc_q35_init(args);
+}
+
static QEMUMachine pc_q35_machine_v1_5 = {
.name = "pc-q35-1.5",
.alias = "q35",
@@ -221,7 +227,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
static QEMUMachine pc_q35_machine_v1_4 = {
.name = "pc-q35-1.4",
.desc = "Standard PC (Q35 + ICH9, 2009)",
- .init = pc_q35_init,
+ .init = pc_q35_init_1_4,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_4,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 5d40914..cf8819d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -176,6 +176,7 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
}
/* pc_sysfw.c */
+extern bool pc_sysfw_flash_vs_rom_bug_compatible;
void pc_system_firmware_init(MemoryRegion *rom_memory);
/* e820 types */
@@ -241,6 +242,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
.driver = "virtio-net-pci",\
.property = "romfile",\
.value = "pxe-virtio.rom",\
+ },{\
+ .driver = "pc-sysfw",\
+ .property = "rom_only",\
+ .value = stringify(0),\
}
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread