On Mon, Feb 05, 2018 at 12:46:41PM +0000, Daniel P. Berrangé wrote:
> I was experimenting with old MS-DOS 6.22 under QEMU with the 'isapc' machine
> type, and found it is not able to reboot the guest. Bisecting QEMU blamed
> the QEMU change that rebased from "rel-1.9.3", to "rel-1.10.0". Further
> bisecting SeaBIOS blames this change:
> 
>   commit b837e68d5a6c1a5945513f1995875445a1594c8a (refs/bisect/bad)
>   Author: Kevin O'Connor <ke...@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 copy of the BIOS in flash
>     appears to be a memory alias instead.  This prevents a hang if an
>     external reboot request occurs during the BIOS memcpy.
>     
>     Signed-off-by: Kevin O'Connor <ke...@koconnor.net>
> 
> I was testing as follows:
> 
>   $ qemu-system-x86_64 -machine isapc -monitor stdio demo.img

I finally got this to work locally.  Apparently the isapc machine type
requires a rom that is 128K in size.

This does appear to be a regression.  I think the patch below should
fix it.

Thanks,
-Kevin


commit 42812e062a77b27b0544c8e0d46d206afc3b2fae (HEAD -> master)
Author: Kevin O'Connor <ke...@koconnor.net>
Date:   Thu Feb 22 20:29:27 2018 -0500

    shadow: Don't invoke a shutdown on reboot unless in a reboot loop
    
    Old versions of KVM would map the same writable copy of the BIOS at
    both 0x000f0000 and 0xffff0000.  As a result, a reboot on these
    machines would result in a reboot loop.  So, the code attempts to
    check for that situation and invoke a shutdown instead.
    
    Commit b837e68d changed the check to run prior to the first reboot.
    However, this broke reboots on the QEMU isapc machine type.  Change
    the reboot loop check to only be invoked after at least one reboot has
    been attempted.
    
    Reported-by: Daniel P. Berrangé <berra...@redhat.com>
    Signed-off-by: Kevin O'Connor <ke...@koconnor.net>

diff --git a/src/fw/shadow.c b/src/fw/shadow.c
index c80b266..987eaf4 100644
--- a/src/fw/shadow.c
+++ b/src/fw/shadow.c
@@ -176,18 +176,23 @@ qemu_reboot(void)
     void *cstart = VSYMBOL(code32flat_start), *cend = VSYMBOL(code32flat_end);
     void *hrp = &HaveRunPost;
     if (readl(hrp + BIOS_SRC_OFFSET)) {
-        // Some old versions of KVM don't store a pristine copy of the
-        // BIOS in high memory.  Try to shutdown the machine instead.
-        dprintf(1, "Unable to hard-reboot machine - attempting shutdown.\n");
-        apm_shutdown();
+        // There isn't a pristine copy of the BIOS at 0xffff0000 to copy
+        if (HaveRunPost == 3) {
+            // In a reboot loop.  Try to shutdown the machine instead.
+            dprintf(1, "Unable to hard-reboot machine - attempting 
shutdown.\n");
+            apm_shutdown();
+        }
+        make_bios_writable();
+        HaveRunPost = 3;
+    } else {
+        // Copy the BIOS making sure to only reset HaveRunPost at end
+        make_bios_writable();
+        memcpy(cstart, cstart + BIOS_SRC_OFFSET, hrp - cstart);
+        memcpy(hrp + 4, hrp + 4 + BIOS_SRC_OFFSET, cend - (hrp + 4));
+        barrier();
+        HaveRunPost = 0;
+        barrier();
     }
-    // Copy the BIOS making sure to only reset HaveRunPost at end
-    make_bios_writable();
-    memcpy(cstart, cstart + BIOS_SRC_OFFSET, hrp - cstart);
-    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

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Reply via email to