Re: [SeaBIOS] [RFC] acpi: Do not advertise S3

[Gleb Natapov]

On Sun, Jan 15, 2012 at 12:35:07PM +0100, Fred . wrote:
> SeaBIOS is used in more than just QEMU.

The DSDT provided by Seabios is not used by anyone but QEMU (and may be
XEN).
> 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

[RFC] acpi: Do not advertise S3

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Sun, Jan 15, 2012 at 12:35:07PM +0100, Fred . wrote:
> > SeaBIOS is used in more than just QEMU.
>
> The DSDT provided by Seabios is not used by anyone but QEMU (and may be
> XEN).

How about renaming acpi-dsdt.dsl to qemu-acpi-dsdt.dsl to reflect this?

Sebastian

[RFC] acpi: Do not advertise S3

[Gleb Natapov]

On Sun, Jan 15, 2012 at 01:17:50PM +0100, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> > On Sun, Jan 15, 2012 at 12:35:07PM +0100, Fred . wrote:
> > > SeaBIOS is used in more than just QEMU.
> >
> > The DSDT provided by Seabios is not used by anyone but QEMU (and may be
> > XEN).
>
> How about renaming acpi-dsdt.dsl to qemu-acpi-dsdt.dsl to reflect this?

This is not the only qemu specific file. Generally SeaBIOS fulfill
two functions. First is legacy BIOS implementation, second is QEMU HW
initialization. Mostly the first one is used when Seabios runs on real HW,
HW initialization is done by coreboot in this kind of usage. And there
is no clean source file separation between QEMU and coreboot use case.
The POST code looks like if (CONFIG_COREBOOT) { do it coreboot way
(often do nothing)} else {init QEMU HW}.

That said, I am fine with renaming if this really helps someone.