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

2013-12-18 Thread Paolo Bonzini
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 
> ---
>  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

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-12-18 Thread Michael S. Tsirkin
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 
> > ---
> >  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.

-- 
MST

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

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

2013-12-18 Thread Gal Hammer

On 18/12/2013 16:22, 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 
---
  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).


I think we both made a mistake and the right name is 
"PMREGMISC—MISCELLANEOUS POWER MANAGEMENT (FUNCTION 3)" :-).



Michael, perhaps a part of pci_setup (same file) should run on S3 resume?

Paolo



Gal.


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2013-12-18 Thread Paolo Bonzini
Il 18/12/2013 16:59, Gal Hammer ha scritto:
>>
>> 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).
> 
> I think we both made a mistake and the right name is
> "PMREGMISC—MISCELLANEOUS POWER MANAGEMENT (FUNCTION 3)" :-).

Yeah. :)  Still, both PMBA and PMREGMISC need to be initialized by SeaBIOS.

Paolo

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios