Re: [PATCH 2 of 2] Add configuration options to selectively disable S3 and S4 ACPI power states

[Keir Fraser <keir.xen@gmail.com>]

On 18/11/2011 11:11, "Paul Durrant" <Paul.Durrant@citrix.com> wrote:

>> -----Original Message-----
>> From: Ian Campbell
>> Sent: 18 November 2011 10:41
>> To: Paul Durrant
>> Cc: xen-devel@lists.xensource.com
>> 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.

I think distributing the xenstore reads is fine. I will apply the final
patch series that you sent, that includes this change.

 -- Keir

>>>          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 :-)
> 
>>> +            bios->acpi_build_tables(s3_enabled, s4_enabled);
>>>          }
>>> 
>>>          acpi_enable_sci();
>> 
>> Ian.
>> 
> 
>   Paul
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel