On QEMU it's necessary to manually reset the BIOS memory region between 0xc0000-0x100000 on a reboot. After this manual memory reset is completed, it's not valid to use the generic reset mechanisms. Rename qemu_prep_reset() to qemu_reboot() and change the function to immediately reboot after the code memcpy.
This fixes a bug that could cause code corruption on reboots - calling the udelay() function (as invoked by i8042_reboot and/or pci_reboot) was not valid after the BIOS was memcpy'd.
Reported-by: "Dr. David Alan Gilbert" dgilbert@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net ---
This patch is based on Paolo's recommendation of first attempting a PCI style reboot on QEMU. However, instead of next attempting a keyboard reset, this patch attempts to signal an "INIT" via a triple fault (as I think that's a bit simpler and less likely to fail).
--- src/fw/shadow.c | 14 +++++++++++++- src/hw/pci.c | 1 - src/hw/pci.h | 2 ++ src/resume.c | 4 ++-- src/util.h | 2 +- 5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/fw/shadow.c b/src/fw/shadow.c index cd02d3a..c80b266 100644 --- a/src/fw/shadow.c +++ b/src/fw/shadow.c @@ -167,7 +167,7 @@ make_bios_readonly(void) }
void -qemu_prep_reset(void) +qemu_reboot(void) { if (!CONFIG_QEMU || runningOnXen()) return; @@ -187,4 +187,16 @@ qemu_prep_reset(void) memcpy(hrp + 4, hrp + 4 + BIOS_SRC_OFFSET, cend - (hrp + 4)); barrier(); HaveRunPost = 0; + barrier(); + + // Request a QEMU system reset. Do the reset in this function as + // the BIOS code was overwritten above and not all BIOS + // functionality may be available. + + // Attempt PCI style reset + outb(0x02, PORT_PCI_REBOOT); + outb(0x06, PORT_PCI_REBOOT); + + // Next try triple faulting the CPU to force a reset + asm volatile("int3"); } diff --git a/src/hw/pci.c b/src/hw/pci.c index 506ee56..8e3d617 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -12,7 +12,6 @@ #include "x86.h" // outl
#define PORT_PCI_CMD 0x0cf8 -#define PORT_PCI_REBOOT 0x0cf9 #define PORT_PCI_DATA 0x0cfc
void pci_config_writel(u16 bdf, u32 addr, u32 val) diff --git a/src/hw/pci.h b/src/hw/pci.h index bf50430..ee6e196 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -3,6 +3,8 @@
#include "types.h" // u32
+#define PORT_PCI_REBOOT 0x0cf9 + static inline u8 pci_bdf_to_bus(u16 bdf) { return bdf >> 8; } diff --git a/src/resume.c b/src/resume.c index 99fa34f..fb0b8a8 100644 --- a/src/resume.c +++ b/src/resume.c @@ -125,8 +125,8 @@ tryReboot(void) { dprintf(1, "Attempting a hard reboot\n");
- // Setup for reset on qemu. - qemu_prep_reset(); + // Use a QEMU specific reboot on QEMU + qemu_reboot();
// Reboot using ACPI RESET_REG acpi_reboot(); diff --git a/src/util.h b/src/util.h index 336eaaf..8269057 100644 --- a/src/util.h +++ b/src/util.h @@ -123,7 +123,7 @@ void pirtable_setup(void); // fw/shadow.c void make_bios_writable(void); void make_bios_readonly(void); -void qemu_prep_reset(void); +void qemu_reboot(void);
// fw/smbios.c void smbios_legacy_setup(void);
* Kevin O'Connor (kevin@koconnor.net) wrote:
On QEMU it's necessary to manually reset the BIOS memory region between 0xc0000-0x100000 on a reboot. After this manual memory reset is completed, it's not valid to use the generic reset mechanisms. Rename qemu_prep_reset() to qemu_reboot() and change the function to immediately reboot after the code memcpy.
This fixes a bug that could cause code corruption on reboots - calling the udelay() function (as invoked by i8042_reboot and/or pci_reboot) was not valid after the BIOS was memcpy'd.
Reported-by: "Dr. David Alan Gilbert" dgilbert@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net
This patch is based on Paolo's recommendation of first attempting a PCI style reboot on QEMU. However, instead of next attempting a keyboard reset, this patch attempts to signal an "INIT" via a triple fault (as I think that's a bit simpler and less likely to fail).
Thanks, I've given this a few hours of testing and it seems to work (this is testing on head rather than on our downstream). Curiously I do occasionally get a KVM_EXIT_SHUTDOWN but I think that's the triple fault you added rather than anything bad happening - it goes away if I add a debug print before it; here's a kvm trace from it:
qemu-system-x86-2998 [000] 347148.144515: kvm_exit: reason EXIT_IOIO rip 0xf524a info cf90110 f524b qemu-system-x86-2998 [000] 347148.144517: kvm_pio: pio_write at 0xcf9 size 1 count 1 val 0x2 qemu-system-x86-2998 [000] 347148.144519: kvm_userspace_exit: reason KVM_EXIT_IO (2) qemu-system-x86-2998 [000] 347148.144528: kvm_entry: vcpu 0 qemu-system-x86-2998 [000] 347148.144529: kvm_exit: reason EXIT_IOIO rip 0xf524d info cf90110 f524e qemu-system-x86-2998 [000] 347148.144530: kvm_pio: pio_write at 0xcf9 size 1 count 1 val 0x6 qemu-system-x86-2998 [000] 347148.144531: kvm_userspace_exit: reason KVM_EXIT_IO (2) qemu-system-x86-2998 [000] 347148.144715: kvm_entry: vcpu 0 qemu-system-x86-2998 [000] 347148.144718: kvm_exit: reason EXIT_SHUTDOWN rip 0xf524e info 0 0 qemu-system-x86-2998 [000] 347148.144727: kvm_userspace_exit: reason KVM_EXIT_SHUTDOWN (8) qemu-system-x86-2993 [003] 347148.144881: kvm_set_irq: gsi 8 level 0 source 0
so it does look like it's directly after that outb.
f5243: ba f9 0c 00 00 mov $0xcf9,%edx f5248: b0 02 mov $0x2,%al f524a: ee out %al,(%dx) f524b: b0 06 mov $0x6,%al f524d: ee out %al,(%dx) f524e: cc int3
So: Tested-by: Dr. David Alan Gilbert dgilbert@redhat.com
Dave
src/fw/shadow.c | 14 +++++++++++++- src/hw/pci.c | 1 - src/hw/pci.h | 2 ++ src/resume.c | 4 ++-- src/util.h | 2 +- 5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/fw/shadow.c b/src/fw/shadow.c index cd02d3a..c80b266 100644 --- a/src/fw/shadow.c +++ b/src/fw/shadow.c @@ -167,7 +167,7 @@ make_bios_readonly(void) }
void -qemu_prep_reset(void) +qemu_reboot(void) { if (!CONFIG_QEMU || runningOnXen()) return; @@ -187,4 +187,16 @@ qemu_prep_reset(void) memcpy(hrp + 4, hrp + 4 + BIOS_SRC_OFFSET, cend - (hrp + 4)); barrier(); HaveRunPost = 0;
- barrier();
- // Request a QEMU system reset. Do the reset in this function as
- // the BIOS code was overwritten above and not all BIOS
- // functionality may be available.
- // Attempt PCI style reset
- outb(0x02, PORT_PCI_REBOOT);
- outb(0x06, PORT_PCI_REBOOT);
- // Next try triple faulting the CPU to force a reset
- asm volatile("int3");
} diff --git a/src/hw/pci.c b/src/hw/pci.c index 506ee56..8e3d617 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -12,7 +12,6 @@ #include "x86.h" // outl
#define PORT_PCI_CMD 0x0cf8 -#define PORT_PCI_REBOOT 0x0cf9 #define PORT_PCI_DATA 0x0cfc
void pci_config_writel(u16 bdf, u32 addr, u32 val) diff --git a/src/hw/pci.h b/src/hw/pci.h index bf50430..ee6e196 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -3,6 +3,8 @@
#include "types.h" // u32
+#define PORT_PCI_REBOOT 0x0cf9
static inline u8 pci_bdf_to_bus(u16 bdf) { return bdf >> 8; } diff --git a/src/resume.c b/src/resume.c index 99fa34f..fb0b8a8 100644 --- a/src/resume.c +++ b/src/resume.c @@ -125,8 +125,8 @@ tryReboot(void) { dprintf(1, "Attempting a hard reboot\n");
- // Setup for reset on qemu.
- qemu_prep_reset();
// Use a QEMU specific reboot on QEMU
qemu_reboot();
// Reboot using ACPI RESET_REG acpi_reboot();
diff --git a/src/util.h b/src/util.h index 336eaaf..8269057 100644 --- a/src/util.h +++ b/src/util.h @@ -123,7 +123,7 @@ void pirtable_setup(void); // fw/shadow.c void make_bios_writable(void); void make_bios_readonly(void); -void qemu_prep_reset(void); +void qemu_reboot(void);
// fw/smbios.c void smbios_legacy_setup(void); -- 2.5.5
-- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Mon, Mar 06, 2017 at 01:49:26PM +0000, Dr. David Alan Gilbert wrote:
- Kevin O'Connor (kevin@koconnor.net) wrote:
On QEMU it's necessary to manually reset the BIOS memory region between 0xc0000-0x100000 on a reboot. After this manual memory reset is completed, it's not valid to use the generic reset mechanisms. Rename qemu_prep_reset() to qemu_reboot() and change the function to immediately reboot after the code memcpy.
This fixes a bug that could cause code corruption on reboots - calling the udelay() function (as invoked by i8042_reboot and/or pci_reboot) was not valid after the BIOS was memcpy'd.
Reported-by: "Dr. David Alan Gilbert" dgilbert@redhat.com Signed-off-by: Kevin O'Connor kevin@koconnor.net
This patch is based on Paolo's recommendation of first attempting a PCI style reboot on QEMU. However, instead of next attempting a keyboard reset, this patch attempts to signal an "INIT" via a triple fault (as I think that's a bit simpler and less likely to fail).
Thanks, I've given this a few hours of testing and it seems to work (this is testing on head rather than on our downstream).
Thanks. I committed this patch.
-Kevin
-
Dr. David Alan Gilbert -
Kevin O'Connor