[SeaBIOS] varlow/extrastack vs code

[Laszlo Ersek]

On 02/14/17 20:14, Kevin O'Connor wrote:
> On Tue, Feb 14, 2017 at 07:52:01PM +0100, Laszlo Ersek wrote:
>> On 02/14/17 19:43, Kevin O'Connor wrote:
>>> On Tue, Feb 14, 2017 at 07:04:05PM +0100, Laszlo Ersek wrote:
>>>> On 02/14/17 18:16, Kevin O'Connor wrote:
>>>>> Also, the PAM registers on real hardware support a mode where reads to
>>>>> 0xf0000 return the pristine copy of the bios while writes update
>>>>> memory.  I didn't think there was any interest in implementing that on
>>>>> QEMU (nor do I think it would be particularly helpful to have).
>>>>
>>>> Hmmm, I thought this was implemented with the four modes visible in
>>>> init_pam() and switched by pam_update(), in "hw/pci-host/pam.c".
>>>>
>>>> Based on the remaining "XXX" comments though, and the wording of commit
>>>> 175f099b30d47 ("pam: partly fix write-only mode"), it seems that the
>>>> emulation is not complete just yet?...
>>>>
>>>> Perhaps this helps Dave identify what should be fixed in QEMU...
>>>
>>> I don't think anything in QEMU needs to be "fixed" - the bug is
>>> definitely in SeaBIOS.  The QEMU pam stuff is definitely quirky, but
>>> even if we updated qemu we'd still have to fix seabios for old
>>> versions of qemu.
>>>
>>> Just for historical perspective - the reason I think qemu didn't
>>> implement the pam "read from rom and write to memory" mode is that I
>>> don't think there's a good way to emulate that with page tables (and
>>> the range needs to be executable so just making it all device memory
>>> isn't practical).  Even if it were implemented, though, I doubt it
>>> would help much.
>>
>> There are two questions here, AIUI:
>> (1) resetting the PAM config on qemu system reset,
>> (2) implementing all four PAM modes in QEMU, faithfully to phys hw.
>>
>> I agree that item (2) is not important.
>>
>> However, in order to fix SeaBIOS, item (1) should be fixed in QEMU,
>> shouldn't it? You wrote,
>>
>>> The root of the problem is that QEMU does not reset the f-segment on a
>>> reboot signal and so SeaBIOS must manually reset that memory. [...]
> 
> Sorry, I didn't word that paragraph well.  I was trying to give
> context for why the seabios code was memcpy'ing over itself.
> 
>> If item (1) is fixed in QEMU, then the above "root cause" goes away, and
>> the workaround in SeaBIOS can be conditionalized. Am I wrong?
> 
> I'm not sure.  If I recall correctly, there are different resets on
> the x86 - some only reset the cpu and some do a "full machine reset".
> SeaBIOS attempts a variety of different reset mechanisms to reboot and
> I'm not sure which are supposed to do the full reset.  If seabios does
> a "reset cpu" mechanism before a "reset machine" mechanism, then qemu
> resetting the pam may not help.

To my knowledge, QEMU implements only one kind of system reset, with
qemu_system_reset_request(). It is supposed to
- reset all VCPUs (it puts all APs back into "wait for
  INIT-SIPI-SIPI"),
- reset all chipset registers,
- reset all devices,
- not touch guest RAM.

(Device models are responsible for registering their reset handlers with
qemu_register_reset().)

See e.g. commit 1ec4ba741630 ("PIIX3: reset the VM when the Reset
Control Register's RCPU bit gets set", 2013-01-24) and commit
0e98b436eceb ("ICH9 LPC: Reset Control Register, basic implementation",
2013-02-20).

(I'm sure there are other good or even better examples, I just recall
these because I happen to have written them.)

> Thinking about this further, I think the real problem is that after
> seabios copies over itself it goes back into its generic reboot
> handling code.  The udelay() is actually fine on real hardware - it's
> just not okay after seabios has memcpy'd itself.  So, I think if
> seabios immediately did a reboot after the memcpy this problem would
> go away.  Something like the patch below.

I'll let Dave explore the various SeaBIOS and/or QEMU changes! :)

> What's the best way to force a reboot on QEMU?  The mechanism really
> should reset all cpus in a multi-cpu machine, ideally it would work on
> different machine types (piix4, q35, isapc, etc.), and ideally it
> would reset the hardware as well.

In SeaBIOS's tryReboot() [src/resume.c], we have:

(1) acpi_reboot(): it uses the register identified by the FADT.RESET_REG
field, if the FADT is large / recent enough.

Currently QEMU does not generate a FADT that is large / recent enough,
so acpi_reboot() should be a no-op, at the moment. (See
find_acpi_features() [src/fw/biostables.c].)

There's an ongoing development to implement this for QEMU (msgid
<1484739954-86833-1-git-send-email-phil at philjordan.eu>,
<https://www.mail-archive.com/qemu-devel@nongnu.org/msg423451.html>),
which would mean advertising ICH9_RST_CNT_IOPORT (0xCF9) in FADT.RESET_REG.

That in turn will lead to the above two commits I listed (i.e., on both
i440fx and q35), hence qemu_system_reset_request().

(2) i8042_reboot(): ultimately this writes value 0xFE to IO port 0x64.
In QEMU, this is handled by kbd_write_command() [hw/input/pckbd.c], case
label KBD_CCMD_RESET; the result is a call to qemu_system_reset_request().

(3) pci_reboot() is just a more direct way (without ACPI) to do (1),
hence qemu_system_reset_request().

(4) Triple fault: I have some trouble to track this down (I think I
might have to look into KVM too for this), but:
- check_exception() [target/i386/excp_helper.c] and
- kvm_arch_process_async_events() [target/i386/kvm.c]
both seem to imply that a triple fault -- i.e., one after EXCP08_DBLE?
-- is handled again with qemu_system_reset_request().

Thus, I think from the four reboot methods seen in tryReboot(), the last
three do the same on QEMU, and once implemented in QEMU, the first one
will do the same as well. And, I think qemu_system_reset_request() does
what you suggest it should.

> --- a/src/fw/shadow.c
> +++ b/src/fw/shadow.c
> @@ -187,4 +187,8 @@ qemu_prep_reset(void)
>      memcpy(hrp + 4, hrp + 4 + BIOS_SRC_OFFSET, cend - (hrp + 4));
>      barrier();
>      HaveRunPost = 0;
> +    barrier();
> +
> +    // Force QEMU reboot
> +    asm volatile("int3");
>  }
> 

Dave, can you try this perhaps?

(Nonetheless, I think QEMU should reset the PAM registers on system
reset -- maybe tie this to machine type versions / compat properties?)

Thanks
Laszlo