Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.

[Paolo Bonzini <pbonzini@redhat.com>]

----- Messaggio originale -----
> Da: "Michael S. Tsirkin" <mst@redhat.com>
> A: "marcel a" <marcel.a@redhat.com>
> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, "Gal Hammer" <ghammer@redhat.com>, seabios@seabios.org,
> qemu-devel@nongnu.org
> Inviato: Mercoledì, 18 dicembre 2013 17:33:06
> Oggetto: Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
> 
> On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote:
> > On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote:
> > > On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote:
> > > > Il 11/12/2013 10:21, Gal Hammer ha scritto:
> > > > > Fix a bug that was introduced in commit c046e8c4. QEMU fails to
> > > > > resume from suspend mode (S3).
> > > > > 
> > > > > Signed-off-by: Gal Hammer <ghammer@redhat.com>
> > > > > ---
> > > > >  hw/acpi/piix4.c | 1 -
> > > > >  1 file changed, 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > > > index 93849c8..5c736a4 100644
> > > > > --- a/hw/acpi/piix4.c
> > > > > +++ b/hw/acpi/piix4.c
> > > > > @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque)
> > > > >      pci_conf[0x5b] = 0;
> > > > >  
> > > > >      pci_conf[0x40] = 0x01; /* PM io base read only bit */
> > > > > -    pci_conf[0x80] = 0;
> > > > >  
> > > > >      if (s->kvm_enabled) {
> > > > >          /* Mark SMM as already inited (until KVM supports SMM). */
> > > > 
> > > > Note this is not the APIC base address, that one is 80h on the ISA
> > > > bridge (function 0).  You're changing the behavior for 80h on the power
> > > > management function, which is function 3.  The register is "PMBA—POWER
> > > > MANAGEMENT BASE ADDRESS" and it is indeed initialized by SeaBIOS in
> > > > piix4_pm_setup (src/fw/pciinit.c).
> > > > 
> > > > Michael, perhaps a part of pci_setup (same file) should run on S3
> > > > resume?
> > > > 
> > > > Paolo
> > > 
> > > Seems reasonable: either seabios or guest OS must do it, and
> > > guest does not seem to.
> > I was looking into this today, but it seems that we have a problem.
> > We cannot run pci_setup() in init section:
> > .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from
> > ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup']
> > 
> > Any thoughts how to get around this?
> > Thanks,
> > Marcel
> 
> We defintely don't want to do full pci enumeration.
> Just pci_bios_init_platform or even less.

Or put an array of (bdf, offset, size, value) tuples somewhere in low memory,
fill it at startup, and reproduce it blindly at S3 resume time.  This is similar
to what UEFI does.

Paolo