On Thu, Feb 22, 2018 at 08:40:09PM -0500, Kevin O'Connor wrote: > 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 <[email protected]> > > 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 <[email protected]> > > > > 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, I've tested this patch and confirm it fixes the behaviour I've been seeing. > commit 42812e062a77b27b0544c8e0d46d206afc3b2fae (HEAD -> master) > Author: Kevin O'Connor <[email protected]> > 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é <[email protected]> > Signed-off-by: Kevin O'Connor <[email protected]> > > 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 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ SeaBIOS mailing list [email protected] https://mail.coreboot.org/mailman/listinfo/seabios
