[SeaBIOS] varlow/extrastack vs code

[Dr. David Alan Gilbert]

* Kevin O'Connor (kevin at koconnor.net) 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.

I'd have sympathy if you just told the QEMU users to get a particular
fix once we fix it.

> 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.

In the principal of removing our quirks, the following seems to work for me,
Kevin, do you agree it's the right behaviour?

Dave

From fffd898e1ef87d6e404179d860db26304308268b Mon Sep 17 00:00:00 2001
From: "Dr. David Alan Gilbert" <dgilbert at redhat.com>
Date: Wed, 15 Feb 2017 10:42:28 +0000
Subject: [PATCH] x86: Reset PAM registers on reset

On a reset the bridge code doesn't reset the PAM registers, the
effect is to leave the RAM copy of the BIOS mapped rather than the
flash copy.  The RAM copy might be in an inconsistent state.
SeaBIOS has some workarounds for this, but they're not always
100% succesful.

Signed-off-by: Dr. David Alan Gilbert <dgilbert at redhat.com>
---
 hw/pci-host/piix.c | 17 +++++++++++++++++
 hw/pci-host/q35.c  |  7 +++++++
 2 files changed, 24 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa..8c7d741 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -603,6 +603,22 @@ static bool piix3_rcr_needed(void *opaque)
     return (piix3->rcr != 0);
 }
 
+static void i440fx_reset(DeviceState *qdev)
+{
+    PCII440FXState *d = I440FX_PCI_DEVICE(qdev);
+    PCIDevice *pd = PCI_DEVICE(d);
+
+    pd->config[I440FX_PAM + 0] = 0; /* 0x59 */
+    pd->config[I440FX_PAM + 1] = 0;
+    pd->config[I440FX_PAM + 2] = 0;
+    pd->config[I440FX_PAM + 3] = 0;
+    pd->config[I440FX_PAM + 4] = 0;
+    pd->config[I440FX_PAM + 5] = 0;
+    pd->config[I440FX_PAM + 6] = 0; /* 0x5F */
+
+    i440fx_update_memory_mappings(d);
+}
+
 static const VMStateDescription vmstate_piix3_rcr = {
     .name = "PIIX3/rcr",
     .version_id = 1,
@@ -741,6 +757,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_BRIDGE_HOST;
     dc->desc = "Host bridge";
     dc->vmsd = &vmstate_i440fx;
+    dc->reset = i440fx_reset;
     /*
      * PCI-facing part of the host bridge, not usable without the
      * host-facing part, which can't be device_add'ed, yet.
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b..9bc05dc 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -456,6 +456,13 @@ static void mch_reset(DeviceState *qdev)
     d->config[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_DEFAULT;
     d->wmask[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_WMASK;
     d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = MCH_HOST_BRIDGE_ESMRAMC_WMASK;
+    d->config[MCH_HOST_BRIDGE_PAM0] = 0; /* 0x90 */
+    d->config[MCH_HOST_BRIDGE_PAM1] = 0;
+    d->config[MCH_HOST_BRIDGE_PAM2] = 0;
+    d->config[MCH_HOST_BRIDGE_PAM3] = 0;
+    d->config[MCH_HOST_BRIDGE_PAM4] = 0;
+    d->config[MCH_HOST_BRIDGE_PAM5] = 0;
+    d->config[MCH_HOST_BRIDGE_PAM6] = 0; /* 0x96 */
 
     mch_update(mch);
 }
-- 
2.9.3

> -Kevin
--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK