Re: [Xen-devel] Re: Paravirtualizing bits of acpi access

[Jeremy Fitzhardinge <jeremy@goop.org>]

Len Brown wrote:
>> diff -r 855cb34ca992 drivers/acpi/acpica/hwsleep.c
>> --- a/drivers/acpi/acpica/hwsleep.c     Tue Mar 17 19:53:17 2009 -0400
>> +++ b/drivers/acpi/acpica/hwsleep.c     Tue Mar 24 09:37:22 2009 -0700
>> @@ -45,6 +45,7 @@
>>  #include <acpi/acpi.h>
>>  #include "accommon.h"
>>  #include "actables.h"
>> +#include <asm/tboot.h>
>>
>>  #define _COMPONENT          ACPI_HARDWARE
>>  ACPI_MODULE_NAME("hwsleep")
>> @@ -332,6 +333,39 @@ acpi_status asmlinkage acpi_enter_sleep_
>>
>>         PM1Acontrol |= sleep_enable_reg_info->access_bit_mask;
>>         PM1Bcontrol |= sleep_enable_reg_info->access_bit_mask;
>> +
>> +#ifdef CONFIG_TXT
>> +#define TB_COPY_GAS(tbg, g)                 \
>> +       tbg.space_id = g.space_id;          \
>> +       tbg.bit_width = g.bit_width;        \
>> +       tbg.bit_offset = g.bit_offset;      \
>> +       tbg.access_width = g.access_width;  \
>> +       tbg.address = g.address;
>> +
>> +       if (tboot_in_measured_env()) {
>> +               TB_COPY_GAS(tboot_shared->acpi_sinfo.pm1a_cnt_blk,
>> +                           acpi_gbl_FADT.xpm1a_control_block);
>> +               TB_COPY_GAS(tboot_shared->acpi_sinfo.pm1b_cnt_blk,
>> +                           acpi_gbl_FADT.xpm1b_control_block);
>>     
>
> Who'd a thunk that suddently everybody would want to scribble
> on acpi_enter_sleep_state()?
>
> Note that acpica/hwsleep.c is a file from ACPICA that we share
> with BSD etc.  Yes, we manage local changes in Linux, but we
> try to reduce them to zero over time, else we create a big
> maintenace headache.
>
> perhaps tboot_in_measured_env() could compile in as 0
> for !CONFIG_TXT and you can get rid of the #ifdefs?
>
> Jeremy, I'm not excited about a proposed change to acpixf.h --
> this is the API to ACPICA...
>   
Do you have an issue with the mechanism (using weak function, etc), or 
just the placement of the prototypes in that header?  Would there be a 
better header to put them in?  Or would you prefer some other mechanism?

It certainly seems like Xen and tboot should be able to share the same 
hook, given that they're doing similar things for similar reasons.

(I don't really understand the structure of all the acpi stuff; I'm just 
wading in and making a mess of things until I can close the lid of 
laptop successfully.)

    J