[RFC] acpi: Do not advertise S3

[Luiz Capitulino]

S3 is broken in qemu in the following ways:

1. S3 is immediately followed by a resume (or it doesn't even
    take place)
 2. The screen goes black after S3
 3. Some people reported a hang (I couldn't reproduce it myself)

Chances are we can get these bugs fixed for 1.1, until there let's
not advertise S3 to prevent things like qemu-ga from breaking qemu.

Signed-off-by: Luiz Capitulino 
---
 src/acpi-dsdt.dsl |    3 +++
 src/acpi-dsdt.hex |   16 ++--------------
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 7082b65..2bd8d95 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -599,6 +599,8 @@ DefinitionBlock (
      * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
      * must match piix4 emulation.
      */
+#if 0
+    S3 is br0ken in qemu, do not advertise it
     Name (_S3, Package (0x04)
     {
         0x01,  /* PM1a_CNT.SLP_TYP */
@@ -606,6 +608,7 @@ DefinitionBlock (
         Zero,  /* reserved */
         Zero   /* reserved */
     })
+#endif
     Name (_S4, Package (0x04)
     {
         Zero,  /* PM1a_CNT.SLP_TYP */
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index 5dc7bb4..9dcef05 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
 0x53,
 0x44,
 0x54,
-0xd3,
+0xc7,
 0x10,
 0x0,
 0x0,
 0x1,
-0x2d,
+0xa3,
 0x42,
 0x58,
 0x50,
@@ -3850,18 +3850,6 @@ static unsigned char AmlCode[] = {
 0x8,
 0x5f,
 0x53,
-0x33,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x1,
-0x1,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
 0x34,
 0x5f,
 0x12,

[Kevin O'Connor]

On Fri, Jan 13, 2012 at 05:11:27PM -0200, Luiz Capitulino wrote:
> S3 is broken in qemu in the following ways:
> S3 is immediately followed by a resume (or it doesn't even
> take place)
> The screen goes black after S3
> Some people reported a hang (I couldn't reproduce it myself)
>
> Chances are we can get these bugs fixed for 1.1, until there let's
> not advertise S3 to prevent things like qemu-ga from breaking qemu.

This is really a call for the qemu/kvm maintainers.  If one of them
will 'ack' this patch, then it's fine with me.

-Kevin

[Gleb Natapov]

On Fri, Jan 13, 2012 at 05:11:27PM -0200, Luiz Capitulino wrote:
> S3 is broken in qemu in the following ways:
> S3 is immediately followed by a resume (or it doesn't even
> take place)

The only real problem. I am not aware of "it doesn't even take place"
part. If there are such cases they are guest problems. Guest can refuse
to do S3 for number of reasons. This has nothing to do with QEMU.
> The screen goes black after S3

Guest problem.
> Some people reported a hang (I couldn't reproduce it myself)

Guest problem.
> Chances are we can get these bugs fixed for 1.1, until there let's
> not advertise S3 to prevent things like qemu-ga from breaking qemu.

I am OK with this, but make it "#if DSDT_S3", so that it can be enabled
without changing the source.
> Signed-off-by: Luiz Capitulino
>
> src/acpi-dsdt.dsl |    3 +++
>  src/acpi-dsdt.hex |   16 ++--------------
>  2 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> index 7082b65..2bd8d95 100644
> --- a/src/acpi-dsdt.dsl
> +++ b/src/acpi-dsdt.dsl
> @@ -599,6 +599,8 @@ DefinitionBlock (
>       * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
>       * must match piix4 emulation.
>       */
> +#if 0
> S3 is br0ken in qemu, do not advertise it
> Name (_S3, Package (0x04)
> {
>     0x01,  /* PM1a_CNT.SLP_TYP */
>
> @@ -606,6 +608,7 @@ DefinitionBlock (
>          Zero,  /* reserved */
>          Zero   /* reserved */
>      })
> +#endif
>      Name (_S4, Package (0x04)
>      {
>          Zero,  /* PM1a_CNT.SLP_TYP */
> diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
> index 5dc7bb4..9dcef05 100644
> --- a/src/acpi-dsdt.hex
> +++ b/src/acpi-dsdt.hex
> @@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
>  0x53,
>  0x44,
>  0x54,
> -0xd3,
> +0xc7,
>  0x10,
>  0x0,
>  0x0,
>  0x1,
> -0x2d,
> +0xa3,
>  0x42,
>  0x58,
>  0x50,
> @@ -3850,18 +3850,6 @@ static unsigned char AmlCode[] = {
>  0x8,
>  0x5f,
>  0x53,
> -0x33,
> -0x5f,
> -0x12,
> -0x6,
> -0x4,
> -0x1,
> -0x1,
> -0x0,
> -0x0,
> -0x8,
> -0x5f,
> -0x53,
>  0x34,
>  0x5f,
>  0x12,
> -- 
> 1.7.9.rc0.dirty

[Fred .]

SeaBIOS is used in more than just QEMU.
If QEMU does not support S3, then they should patch (or configure)
SeaBIOS to not advertise S3 support.

On Sun, Jan 15, 2012 at 10:00 AM, Gleb Natapov  wrote:
> On Fri, Jan 13, 2012 at 05:11:27PM -0200, Luiz Capitulino wrote:
> > S3 is broken in qemu in the following ways:
> >
> > 1. S3 is immediately followed by a resume (or it doesn't even
> >     take place)
>
> The only real problem. I am not aware of "it doesn't even take place"
> part. If there are such cases they are guest problems. Guest can refuse
> to do S3 for number of reasons. This has nothing to do with QEMU.
> > 2. The screen goes black after S3
>
> Guest problem.
> > 3. Some people reported a hang (I couldn't reproduce it myself)
>
> Guest problem.
> > Chances are we can get these bugs fixed for 1.1, until there let's
> > not advertise S3 to prevent things like qemu-ga from breaking qemu.
>
> I am OK with this, but make it "#if DSDT_S3", so that it can be enabled
> without changing the source.
> > Signed-off-by: Luiz Capitulino
> >
> > src/acpi-dsdt.dsl |    3 +++
> >  src/acpi-dsdt.hex |   16 ++--------------
> >  2 files changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> > index 7082b65..2bd8d95 100644
> > --- a/src/acpi-dsdt.dsl
> > +++ b/src/acpi-dsdt.dsl
> > @@ -599,6 +599,8 @@ DefinitionBlock (
> >       * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
> >       * must match piix4 emulation.
> >       */
> > +#if 0
> > S3 is br0ken in qemu, do not advertise it
> >
> > Name (_S3, Package (0x04)
> >      {
> >          0x01,  /* PM1a_CNT.SLP_TYP */
> > @@ -606,6 +608,7 @@ DefinitionBlock (
> >          Zero,  /* reserved */
> >          Zero   /* reserved */
> >      })
> > +#endif
> >      Name (_S4, Package (0x04)
> >      {
> >          Zero,  /* PM1a_CNT.SLP_TYP */
> > diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
> > index 5dc7bb4..9dcef05 100644
> > --- a/src/acpi-dsdt.hex
> > +++ b/src/acpi-dsdt.hex
> > @@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
> >  0x53,
> >  0x44,
> >  0x54,
> > -0xd3,
> > +0xc7,
> >  0x10,
> >  0x0,
> >  0x0,
> >  0x1,
> > -0x2d,
> > +0xa3,
> >  0x42,
> >  0x58,
> >  0x50,
> > @@ -3850,18 +3850,6 @@ static unsigned char AmlCode[] = {
> >  0x8,
> >  0x5f,
> >  0x53,
> > -0x33,
> > -0x5f,
> > -0x12,
> > -0x6,
> > -0x4,
> > -0x1,
> > -0x1,
> > -0x0,
> > -0x0,
> > -0x8,
> > -0x5f,
> > -0x53,
> >  0x34,
> >  0x5f,
> >  0x12,
> > --
> > 1.7.9.rc0.dirty
>
> SeaBIOS mailing list
> [e-mail]
> http://www.seabios.org/mailman/listinfo/seabios

[Luiz Capitulino]

On Sun, 15 Jan 2012 11:00:16 +0200
Gleb Natapov  wrote:
> On Fri, Jan 13, 2012 at 05:11:27PM -0200, Luiz Capitulino wrote:
> > S3 is broken in qemu in the following ways:
> > S3 is immediately followed by a resume (or it doesn't even
> > take place)
>
> The only real problem. I am not aware of "it doesn't even take place"
> part. If there are such cases they are guest problems. Guest can refuse
> to do S3 for number of reasons. This has nothing to do with QEMU.
> > The screen goes black after S3
>
> Guest problem.
> > Some people reported a hang (I couldn't reproduce it myself)
>
> Guest problem.
> > Chances are we can get these bugs fixed for 1.1, until there let's
> > not advertise S3 to prevent things like qemu-ga from breaking qemu.
>
> I am OK with this, but make it "#if DSDT_S3", so that it can be enabled
> without changing the source.

Gerd has submitted patches to fix item 1 in qemu. As I didn't know items
2 and 3 are guest problems, I'll hold this patch to see if Gerd's gets
merged (in which case disabling S3 won't be necessary.
> > Signed-off-by: Luiz Capitulino
> >
> > src/acpi-dsdt.dsl |    3 +++
> >  src/acpi-dsdt.hex |   16 ++--------------
> >  2 files changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> > index 7082b65..2bd8d95 100644
> > --- a/src/acpi-dsdt.dsl
> > +++ b/src/acpi-dsdt.dsl
> > @@ -599,6 +599,8 @@ DefinitionBlock (
> >       * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
> >       * must match piix4 emulation.
> >       */
> > +#if 0
> > S3 is br0ken in qemu, do not advertise it
> > Name (_S3, Package (0x04)
> > {
> >     0x01,  /* PM1a_CNT.SLP_TYP */
> >
> > @@ -606,6 +608,7 @@ DefinitionBlock (
> >          Zero,  /* reserved */
> >          Zero   /* reserved */
> >      })
> > +#endif
> >      Name (_S4, Package (0x04)
> >      {
> >          Zero,  /* PM1a_CNT.SLP_TYP */
> > diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
> > index 5dc7bb4..9dcef05 100644
> > --- a/src/acpi-dsdt.hex
> > +++ b/src/acpi-dsdt.hex
> > @@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
> >  0x53,
> >  0x44,
> >  0x54,
> > -0xd3,
> > +0xc7,
> >  0x10,
> >  0x0,
> >  0x0,
> >  0x1,
> > -0x2d,
> > +0xa3,
> >  0x42,
> >  0x58,
> >  0x50,
> > @@ -3850,18 +3850,6 @@ static unsigned char AmlCode[] = {
> >  0x8,
> >  0x5f,
> >  0x53,
> > -0x33,
> > -0x5f,
> > -0x12,
> > -0x6,
> > -0x4,
> > -0x1,
> > -0x1,
> > -0x0,
> > -0x0,
> > -0x8,
> > -0x5f,
> > -0x53,
> >  0x34,
> >  0x5f,
> >  0x12,
> > -- 
> > 1.7.9.rc0.dirty