diff mbox

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

Message ID 4F65016F6CB04E49BFFA15D4F7B798D9944E2524@orsmsx506.amr.corp.intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Cihula, Joseph March 24, 2009, 5:51 p.m. UTC
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> Sent: Tuesday, March 24, 2009 10:28 AM
>
> Bjorn Helgaas wrote:
> > On Tuesday 24 March 2009 01:05:43 am Jeremy Fitzhardinge wrote:
> >
> >> Tian, Kevin wrote:
> >>
> >>> OK, then it's safe to avoid that change. I had thought that dom0's 0-1M
> >>> is identity-mapped to machine 0-1M... :-)
> >>>
> >> No, only the ISA 640k-1M region.
> >>
> >
> > I'm speaking out of turn here because I don't work on Xen or
> > suspend/resume.  However, I do try to clean up random bits of
> > ACPI, and I have to say this patch looks like a pain in the
> > maintenance department.  Having tests for a specific hypervisor
> > is unpleasant.  We don't want to end up with tests for a collection
> > of hypervisors.
>
> I agree.  If we start to see other hypervisor-specific changes in this
> area, we'd need to rethink this approach.  But I'm not inclined to add a
> layer of infrastructure to just deal with Xen. (IOW, abstract only when
> there's something to abstract.)
>
> >   It looks like suspend becomes a weird hybrid of
> > ACPI and Xen, which makes it harder to reason about future suspend
> > changes.  And all this discussion about 640k-1M and dom0 identity
> > mapping and "there's no special effort to remap it" and whether
> > there are conflicts makes me nervous.  There's no way all those
> > assumptions can be remembered or verified five years down the road.
> >
>
> Well, I think Kevin was over-complicating things in his own mind.  The
> upshot is that the normal "running on bare metal" code can do its normal
> thing, and if we happen to be running under Xen we can make it a no-op.
> In other words, the acpi developers don't really need to worry about the
> "running under Xen case", for the most part.
>
> The two changes this patch makes are, unfortunately, unavoidable:
>
>    1. Turn the final "enter sleep" into a hypercall, so that Xen can do
>       all the low-level context save/restore.  The normal baremetal case
>       is still localized in acpica/hwsleep.c, but it (may) make an
>       excursion into arch code to see if it should do something else, and
>    2. Directly enter the sleep state, rather than save cpu context
>       (which Xen does)
>
> Though, come to think of it, perhaps there's no harm in letting the
> kernel do its own state-saving.  I'll check.
>
>     J

The Intel(R) TXT patch that I'm getting ready to post seems like it is logically very similar to Xen in its handling of shutdown (from the perspective of the kernel).  In the TXT case, the kernel performs the various HW and kernel preparation but the final platform entry into Sx is done by the tboot code (after some other work).  Below are the relevant changes from the TXT patch, where tboot_sleep() just translates the ACPI sleep param to a tboot-specific value and then calls tboot_shutdown(), and tboot_shutdown() simply calls into the tboot code to perform the actual platform sleep.


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Len Brown March 27, 2009, 9:57 p.m. UTC | #1
> > The two changes this patch makes are, unfortunately, unavoidable:
> >
> >    1. Turn the final "enter sleep" into a hypercall, so that Xen can do
> >       all the low-level context save/restore.  The normal baremetal case
> >       is still localized in acpica/hwsleep.c, but it (may) make an
> >       excursion into arch code to see if it should do something else, and
> >    2. Directly enter the sleep state, rather than save cpu context
> >       (which Xen does)
> >
> > Though, come to think of it, perhaps there's no harm in letting the
> > kernel do its own state-saving.  I'll check.

Certainly the less different than bare metal you can be, the better.

The last thing we need is to complicate something that in Linux
has only been sort of working for 


> 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...

thanks,
-Len Brown, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Fitzhardinge March 27, 2009, 11:20 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Len Brown March 28, 2009, 1:01 a.m. UTC | #3
> > 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.)

Everything in acpi/acpica/ is source code that we share with BSD
via the ACPICA project http://acpica.org/

ACPICA also supplies a couple of the headers outside that directory,
eg. acpixf.h, which is the API between the kernel and ACPICA.

We can, and do, change that API when it makes sense.
However, we want to think carefully before changing it,
for we either cause Linux to diverge, or we have to sell
the same change to several other operating systems.
So ideally we wouuld need to make no Linux-specific, or Xen-specific
changes in that directory.

One possibility is to have this called via
function pointer from ASM and scribble over the default
function pointer for the Xen case.  In that case, the Xen
version of the routine would live someplace other than acpi/acpica/ -
someplace with the word xen in the path.  If using _weak can effectively
do that at link time, then fine, if we can do it w/o changing the API --
a _weak annotation is an easy ACPICA/Linux divergencen to manage.

I don't know if Xen and TXT are exclusive, or if we'd ever need
to handle both cases at the same time.  I guess that will influence
if the TXT and Xen use the same approach or something different.

thanks,
Len Brown, Intel Open Source Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tian, Kevin March 28, 2009, 2:19 a.m. UTC | #4
>From: Len Brown [mailto:lenb@kernel.org] 
>Sent: 2009年3月28日 9:02
>> > 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.)
>
>Everything in acpi/acpica/ is source code that we share with BSD
>via the ACPICA project http://acpica.org/
>
>ACPICA also supplies a couple of the headers outside that directory,
>eg. acpixf.h, which is the API between the kernel and ACPICA.
>
>We can, and do, change that API when it makes sense.
>However, we want to think carefully before changing it,
>for we either cause Linux to diverge, or we have to sell
>the same change to several other operating systems.
>So ideally we wouuld need to make no Linux-specific, or Xen-specific
>changes in that directory.
>
>One possibility is to have this called via
>function pointer from ASM and scribble over the default
>function pointer for the Xen case.  In that case, the Xen
>version of the routine would live someplace other than acpi/acpica/ -
>someplace with the word xen in the path.  If using _weak can 
>effectively
>do that at link time, then fine, if we can do it w/o changing 
>the API --
>a _weak annotation is an easy ACPICA/Linux divergencen to manage.
>
>I don't know if Xen and TXT are exclusive, or if we'd ever need
>to handle both cases at the same time.  I guess that will influence
>if the TXT and Xen use the same approach or something different.
>

When only Xen exists, S3 flow is:
dom0 S3 -> Xen S3

When only TXT is enabled, it's:
dom0 S3 -> TXT S3

When both Xen and TXT exist, TXT is not exposed to dom0 and thus
the S3 flow is:
dom0 S3 -> Xen S3 -> TXT S3

I.e, dom0 only needs to care one case at given time. Transition to
TXT is only required if system software is the lowest level on top of
hardware.

Thanks
Kevin
Jeremy Fitzhardinge March 28, 2009, 3:19 a.m. UTC | #5
Len Brown wrote:
>>> 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.)
>>     
>
> Everything in acpi/acpica/ is source code that we share with BSD
> via the ACPICA project http://acpica.org/
>
> ACPICA also supplies a couple of the headers outside that directory,
> eg. acpixf.h, which is the API between the kernel and ACPICA.
>
> We can, and do, change that API when it makes sense.
> However, we want to think carefully before changing it,
> for we either cause Linux to diverge, or we have to sell
> the same change to several other operating systems.
> So ideally we wouuld need to make no Linux-specific, or Xen-specific
> changes in that directory.
>
> One possibility is to have this called via
> function pointer from ASM and scribble over the default
> function pointer for the Xen case.  In that case, the Xen
> version of the routine would live someplace other than acpi/acpica/ -
> someplace with the word xen in the path.

Yes, that would be easy enough to do; we could overwrite it only when 
actually running under Xen.

However, we don't need to replace the whole of acpi_enter_sleep_state(); 
there are two options: we could duplicate the early part of the function 
in the Xen version of it, or break just the differing part out via 
function pointer.  The former has the disadvantage of duplicating code, 
but it does allow the same function pointer to be used by the tboot version.

>   If using _weak can effectively
> do that at link time, then fine, if we can do it w/o changing the API --
> a _weak annotation is an easy ACPICA/Linux divergencen to manage.
>   

The weak approach is what my proposed patch does:

    * the default native-hardware version of the sleep-entry register
      writes is broken out into default_acpi_enter_sleep_state()
    * it introduces a weak arch_acpi_enter_sleep_state() which just
      calls the default case, leaving the normal function unchanged
    * in arch/x86/kernel/acpi/sleep.c, it adds an override
      arch_acpi_enter_sleep_state(), which checks to see if we're
      running under Xen; if not, then it simply calls
      default_acpi_enter_sleep_state() as usual; if it does, it calls
      xen_acpi_enter_sleep_state()
    * xen_acpi_enter_sleep-state() is defined in arch/x86/xen/acpi.c

(Actually it didn't break the Xen version out separately, but it easily 
could.)

On the whole, using a function pointer would be a bit cleaner, as it 
removes the need for the weak glue functions, at the cost of some 
(possible) code duplication.

> I don't know if Xen and TXT are exclusive, or if we'd ever need
> to handle both cases at the same time.  I guess that will influence
> if the TXT and Xen use the same approach or something different.
>   

As Kevin said, they're exclusive, so they could share the same function 
pointer.

    J
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael Wysocki March 28, 2009, 1:56 p.m. UTC | #6
On Saturday 28 March 2009, Jeremy Fitzhardinge wrote:
> Len Brown wrote:
> >>> 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.)
> >>     
> >
> > Everything in acpi/acpica/ is source code that we share with BSD
> > via the ACPICA project http://acpica.org/
> >
> > ACPICA also supplies a couple of the headers outside that directory,
> > eg. acpixf.h, which is the API between the kernel and ACPICA.
> >
> > We can, and do, change that API when it makes sense.
> > However, we want to think carefully before changing it,
> > for we either cause Linux to diverge, or we have to sell
> > the same change to several other operating systems.
> > So ideally we wouuld need to make no Linux-specific, or Xen-specific
> > changes in that directory.
> >
> > One possibility is to have this called via
> > function pointer from ASM and scribble over the default
> > function pointer for the Xen case.  In that case, the Xen
> > version of the routine would live someplace other than acpi/acpica/ -
> > someplace with the word xen in the path.
> 
> Yes, that would be easy enough to do; we could overwrite it only when 
> actually running under Xen.
> 
> However, we don't need to replace the whole of acpi_enter_sleep_state(); 
> there are two options: we could duplicate the early part of the function 
> in the Xen version of it, or break just the differing part out via 
> function pointer.  The former has the disadvantage of duplicating code, 
> but it does allow the same function pointer to be used by the tboot version.
> 
> >   If using _weak can effectively
> > do that at link time, then fine, if we can do it w/o changing the API --
> > a _weak annotation is an easy ACPICA/Linux divergencen to manage.
> >   
> 
> The weak approach is what my proposed patch does:
> 
>     * the default native-hardware version of the sleep-entry register
>       writes is broken out into default_acpi_enter_sleep_state()
>     * it introduces a weak arch_acpi_enter_sleep_state() which just
>       calls the default case, leaving the normal function unchanged
>     * in arch/x86/kernel/acpi/sleep.c, it adds an override
>       arch_acpi_enter_sleep_state(), which checks to see if we're
>       running under Xen; if not, then it simply calls
>       default_acpi_enter_sleep_state() as usual; if it does, it calls
>       xen_acpi_enter_sleep_state()
>     * xen_acpi_enter_sleep-state() is defined in arch/x86/xen/acpi.c
> 
> (Actually it didn't break the Xen version out separately, but it easily 
> could.)
> 
> On the whole, using a function pointer would be a bit cleaner, as it 
> removes the need for the weak glue functions, at the cost of some 
> (possible) code duplication.
> 
> > I don't know if Xen and TXT are exclusive, or if we'd ever need
> > to handle both cases at the same time.  I guess that will influence
> > if the TXT and Xen use the same approach or something different.
> >   
> 
> As Kevin said, they're exclusive, so they could share the same function 
> pointer.

FWIW, if you only care about suspen to RAM (S3). I'm still thinking that
do_suspend_lowlevel() is the place to work on.  After all
acpi_enter_sleep_state() is called from there.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff -r 855cb34ca992 arch/x86/kernel/reboot.c
--- a/arch/x86/kernel/reboot.c  Tue Mar 17 19:53:17 2009 -0400
+++ b/arch/x86/kernel/reboot.c  Tue Mar 24 09:37:22 2009 -0700
@@ -22,6 +22,8 @@ 
 #else
 # include <asm/iommu.h>
 #endif
+
+#include <asm/tboot.h>

 #include <mach_ipi.h>

@@ -436,6 +438,8 @@  static void native_machine_emergency_res
        if (reboot_emergency)
                emergency_vmx_disable_all();

+       tboot_shutdown(TB_SHUTDOWN_REBOOT);
+
        /* Tell the BIOS if we want cold or warm reboot */
        *((unsigned short *)__va(0x472)) = reboot_mode;

@@ -501,11 +505,19 @@  static void native_machine_emergency_res

 void native_machine_shutdown(void)
 {
+#ifdef CONFIG_SMP
+       /* The boot cpu is always logical cpu 0 */
+       int reboot_cpu_id = 0;
+#endif
+
+       /* TXT requires VMX to be off for all shutdowns */
+       if (tboot_in_measured_env()) {
+               emergency_vmx_disable_all();
+               local_irq_enable();
+       }
+
        /* Stop the cpus and apics */
 #ifdef CONFIG_SMP
-
-       /* The boot cpu is always logical cpu 0 */
-       int reboot_cpu_id = 0;

 #ifdef CONFIG_X86_32
        /* See if there has been given a command line override */
@@ -562,6 +574,8 @@  static void native_machine_halt(void)
        /* stop other cpus and apics */
        machine_shutdown();

+       tboot_shutdown(TB_SHUTDOWN_HALT);
+
        /* stop this cpu */
        stop_this_cpu(NULL);
 }
@@ -573,6 +587,8 @@  static void native_machine_power_off(voi
                        machine_shutdown();
                pm_power_off();
        }
+       /* a fallback in case there is no PM info available */
+       tboot_shutdown(TB_SHUTDOWN_HALT);
 }

 struct machine_ops machine_ops = {
diff -r 855cb34ca992 arch/x86/kernel/smpboot.c
--- a/arch/x86/kernel/smpboot.c Tue Mar 17 19:53:17 2009 -0400
+++ b/arch/x86/kernel/smpboot.c Tue Mar 24 09:37:22 2009 -0700
@@ -63,6 +63,7 @@ 
 #include <asm/vmi.h>
 #include <asm/genapic.h>
 #include <asm/setup.h>
+#include <asm/tboot.h>
 #include <linux/mc146818rtc.h>

 #include <mach_apic.h>
@@ -1436,7 +1437,10 @@  void native_play_dead(void)
 void native_play_dead(void)
 {
        play_dead_common();
-       wbinvd_halt();
+       if (tboot_in_measured_env())
+               tboot_shutdown(TB_SHUTDOWN_WFS);
+       else
+               wbinvd_halt();
 }

 #else /* ... !CONFIG_HOTPLUG_CPU */
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);
+               TB_COPY_GAS(tboot_shared->acpi_sinfo.pm1a_evt_blk,
+                           acpi_gbl_FADT.xpm1a_event_block);
+               TB_COPY_GAS(tboot_shared->acpi_sinfo.pm1b_evt_blk,
+                           acpi_gbl_FADT.xpm1b_event_block);
+               tboot_shared->acpi_sinfo.pm1a_cnt_val = PM1Acontrol;
+               tboot_shared->acpi_sinfo.pm1b_cnt_val = PM1Bcontrol;
+               /* we need phys addr of waking vector, but can't use
+                  virt_to_phys() on &acpi_gbl_FACS because it is ioremap'ed,
+                  so calc from FACS phys addr */
+               tboot_shared->acpi_sinfo.wakeup_vector = acpi_gbl_FADT.facs +
+                       ((void *)&acpi_gbl_FACS->firmware_waking_vector -
+                        (void *)acpi_gbl_FACS);
+               tboot_shared->acpi_sinfo.vector_width = 32;
+               tboot_shared->acpi_sinfo.kernel_s3_resume_vector =
+                               acpi_wakeup_address;
+
+               tboot_sleep(sleep_state);
+       }
+#endif

        /* Write #2: SLP_TYP + SLP_EN */