RE: [Xen-devel] [PATCH 2 of 2] Add configuration options to selectively disable S3 and S4 ACPI power states

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-11-18 at 11:11 +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: 18 November 2011 10:41
> > To: Paul Durrant
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [Xen-devel] [PATCH 2 of 2] Add configuration options to
> > selectively disable S3 and S4 ACPI power states
> > 
> > On Fri, 2011-11-18 at 10:29 +0000, Paul Durrant wrote:
> > > diff -r d22ef0f60497 -r 66bdcb90560f
> > tools/firmware/hvmloader/hvmloader.c
> > > --- a/tools/firmware/hvmloader/hvmloader.c      Fri Nov 18
> > 10:28:52 2011 +0000
> > > +++ b/tools/firmware/hvmloader/hvmloader.c      Fri Nov 18
> > 10:28:53 2011 +0000
> > > @@ -516,11 +516,17 @@ int main(void)
> > >              .index = HVM_PARAM_ACPI_IOPORTS_LOCATION,
> > >              .value = 1,
> > >          };
> > > +        int s3_enabled, s4_enabled;
> > > +
> > > +        s3_enabled = !strncmp(xenstore_read("platform/acpi_s3",
> > "1"), "1", 1);
> > > +        s4_enabled = !strncmp(xenstore_read("platform/acpi_s4",
> > "1"),
> > > + "1", 1);
> > 
> > Is it not possible to do these in the underlying acpi_build_tables
> > and avoid the need to plumb them right the way through?
> > 
> 
> I guess that would be possible. It just seemed logical to group the
> acpi config xenstore reads close together. If we're happy to
> distribute use of the xenstore client code to all corners of the
> source then I can certainly do that.

FWIW I think that's ok.

if we wanted to keep them together we could push the acpi check itself
down too and just return without doing anything in that case. I don't
think it's worth it though...

> 
> > >          if ( bios->acpi_build_tables )
> > >          {
> > > -            printf("Loading ACPI ...\n");
> > > -            bios->acpi_build_tables();
> > > +            printf("Loading ACPI (S3=%s S4=%s) ...\n",
> > > +                   (s3_enabled) ? "ON" : "OFF",
> > > +                   (s4_enabled) ? "ON" : "OFF");
> > 
> > If possible this printf could also be pushed down so you can
> > continue to print the config info.
> > 
> 
> Well, if the xenstore reads are pushed down then I'd clearly need to do that 
> too :-)

That what I was trying (and failing) to say...

> 
> > > +            bios->acpi_build_tables(s3_enabled, s4_enabled);
> > >          }
> > >
> > >          acpi_enable_sci();
> > 
> > Ian.
> > 
> 
>   Paul



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel