On Mon, Nov 09, 2015 at 03:06:18PM -0500, Kevin O'Connor wrote:
On Mon, Nov 09, 2015 at 08:32:53AM -0500, Kevin O'Connor wrote:
On Fri, Nov 06, 2015 at 09:12:34AM +0000, Xulei (Stone) wrote:
On Wed, Nov 04, 2015 at 08:48:20AM +0800, Gonglei wrote: I'm surprised you would see the above on a recent qemu/kvm though - as on a newer KVM I think the second reset would have to happen after HaveAttemptedReboot is set and prior to the memcpy in qemu_prep_reset() completing. Can you verify your KVM version?
I've tested on KVM-3.6 and KVM-4.1.3. On both of these versions, i can see this problem. I do like this: put a HA and a watchdog mechanism in a VM. Deliberately, let this VM lose heartbeat and don't feed dog. Then, after 2 minutes, a self-defined timeout, HA mechnism will issue a internal reboot command to the VM and watchdog mechanism will issue a "virsh reset" from the host. Then, aforementioned problem will occurs in high probability.
Ah, okay. I'm not sure what the best solution to this problem is.
After thinking about this further, I think we can move the HaveAttemptedReboot assignment after the memcpy.
The previous patch could cause corruption if the memcpy() failed. I think the new SeaBIOS patch below should be okay though.
-Kevin
commit 8a6e44ad5c953266d2339b3299f5fb4ff32c8cbb Author: Kevin O'Connor kevin@koconnor.net Date: Mon Nov 9 15:00:19 2015 -0500
resume: Make KVM soft reboot loop detection more flexible
Move the check for soft reboot loops from resume.c to shadow.c and directly check for the case where the memcpy fails. This prevents a hang if an external reboot request occurs during the BIOS memcpy.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
diff --git a/src/fw/shadow.c b/src/fw/shadow.c index ee87d36..b2f2dd8 100644 --- a/src/fw/shadow.c +++ b/src/fw/shadow.c @@ -156,6 +156,8 @@ make_bios_readonly(void) make_bios_readonly_intel(ShadowBDF, Q35_HOST_BRIDGE_PAM0); }
+static u8 AttemptingReboot; + void qemu_prep_reset(void) { @@ -164,6 +166,19 @@ qemu_prep_reset(void) // QEMU doesn't map 0xc0000-0xfffff back to the original rom on a // reset, so do that manually before invoking a hard reset. make_bios_writable(); + AttemptingReboot = 1; + barrier(); + if (!AttemptingReboot) + goto fail; + barrier(); memcpy(VSYMBOL(code32flat_start), VSYMBOL(code32flat_start) + BIOS_SRC_OFFSET , SYMBOL(code32flat_end) - SYMBOL(code32flat_start)); + barrier(); + if (AttemptingReboot) + goto fail; + return; +fail: + // Attempt to restore code has failed - try to shutdown machine. + dprintf(1, "Unable to hard-reboot machine - attempting shutdown.\n"); + apm_shutdown(); } diff --git a/src/resume.c b/src/resume.c index a5465d8..afeadcf 100644 --- a/src/resume.c +++ b/src/resume.c @@ -114,19 +114,10 @@ s3_resume(void) farcall16big(&br); }
-u8 HaveAttemptedReboot VARLOW; - // Attempt to invoke a hard-reboot. static void tryReboot(void) { - if (HaveAttemptedReboot) { - // Hard reboot has failed - try to shutdown machine. - dprintf(1, "Unable to hard-reboot machine - attempting shutdown.\n"); - apm_shutdown(); - } - HaveAttemptedReboot = 1; - dprintf(1, "Attempting a hard reboot\n");
// Setup for reset on qemu.