tboot-devel — Developer support

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Konrad R. W. <kon...@or...>]

On Thu, Sep 08, 2011 at 09:38:48AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > >> +#define XENPF_enter_acpi_sleep    51
> > > > >> +struct xenpf_enter_acpi_sleep {
> > > > >> +	/* IN variables */
> > > > >> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> > > > >> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> > > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > > > 
> > > > Does ACPI define them as 32 or 16 bit?
> > > 
> > > The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which your other patch is truncating for this call.
> > 
> > Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> > and also address all the other comments (thanks for looking at it) you pointed
> > out.
> 
> So read up the ACPI spec and it says that the minimum is 2 bytes and does not
> say anything about the maximum. The list of what the bits do stops at 16-bits
> (the last two are reserved) so I think we are actually OK.

Perhaps a better way of doing this is in the hypercall (in the Linux kernel)
check if the other 16-bits have any value. And if so WARN (with an appropiate
message to email xen-devel) and also bail out on making the hypercall. That
way we can find out about this.

> 
> Albeit if the spec starts using more of them - then yes we will need to revist
> this Xen ABI and potentially add a new call.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen...@li...
> http://lists.xensource.com/xen-devel

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Jeremy F. <je...@go...>]

On 09/21/2011 12:29 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 07, 2011 at 10:29:11AM -0700, Jeremy Fitzhardinge wrote:
>> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
>>>> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
>>>> Sent: Wednesday, August 31, 2011 11:31 AM
>>>>
>>>> From: Yu Ke <ke...@in...>
>>>>
>>>> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>>>>
>>>> Signed-off-by: Yu Ke <ke...@in...>
>>>> Signed-off-by: Tian Kevin <kev...@in...>
>>>> Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
>>>> [v1: Added DEFINE_GUEST.. in appropiate headers]
>>>> [v2: Ripped out typedefs]
>>>> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
>>>> ---
>>>>  arch/ia64/include/asm/xen/interface.h |    1 +
>>>>  arch/x86/include/asm/xen/interface.h  |    1 +
>>>>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>>>>  include/xen/interface/xen.h           |    1 +
>>>>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
>>>> include/xen/interface/platform.h
>>>>
>>>> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
>>>> index e951e74..1d2427d 100644
>>>> --- a/arch/ia64/include/asm/xen/interface.h
>>>> +++ b/arch/ia64/include/asm/xen/interface.h
>>>> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>>>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
>>>> +DEFINE_GUEST_HANDLE(uint64_t);
>>>>
>>>>  typedef unsigned long xen_pfn_t;
>>>>  DEFINE_GUEST_HANDLE(xen_pfn_t);
>>>> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
>>>> index 5d4922a..a1f2db5 100644
>>>> --- a/arch/x86/include/asm/xen/interface.h
>>>> +++ b/arch/x86/include/asm/xen/interface.h
>>>> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>>>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
>>>> +DEFINE_GUEST_HANDLE(uint64_t);
>>>>  #endif
>>>>
>>>>  #ifndef HYPERVISOR_VIRT_START
>>>> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
>>>> new file mode 100644
>>>> index 0000000..c168468
>>>> --- /dev/null
>>>> +++ b/include/xen/interface/platform.h
>>> Why are you adding so many new hypercalls that aren't being used in this patchset?
>> May as well bring in all the platform-related hypercall definitions at
>> once rather than be piecemeal.
> Actually, I think making it piecemeal might be easier. As in
>  1). Not all of the hypercalls are going to be upstreamed.
>  2). It gives a nice history/idea of what is actually implemented when
>      seaching the code.
>  3). git annotate can give precise git commits for when an functionality was
>      added.
>
> But maybe I am just overthinking it.

If platform.h gets put on multiple branches which depend on it, it would
be easier to merge if it were the same copy in every branch rather than
trying to merge lots of different variants.

    J

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Konrad R. W. <kon...@or...>]

On Wed, Sep 07, 2011 at 10:29:11AM -0700, Jeremy Fitzhardinge wrote:
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke...@in...>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke...@in...>
> >> Signed-off-by: Tian Kevin <kev...@in...>
> >> Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> >> ---
> >>  arch/ia64/include/asm/xen/interface.h |    1 +
> >>  arch/x86/include/asm/xen/interface.h  |    1 +
> >>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
> >>  include/xen/interface/xen.h           |    1 +
> >>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
> >> include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
> >> +++ b/arch/ia64/include/asm/xen/interface.h
> >> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> >> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >>
> >>  typedef unsigned long xen_pfn_t;
> >>  DEFINE_GUEST_HANDLE(xen_pfn_t);
> >> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> >> index 5d4922a..a1f2db5 100644
> >> --- a/arch/x86/include/asm/xen/interface.h
> >> +++ b/arch/x86/include/asm/xen/interface.h
> >> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> >> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >>  #endif
> >>
> >>  #ifndef HYPERVISOR_VIRT_START
> >> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> >> new file mode 100644
> >> index 0000000..c168468
> >> --- /dev/null
> >> +++ b/include/xen/interface/platform.h
> > Why are you adding so many new hypercalls that aren't being used in this patchset?
> 
> May as well bring in all the platform-related hypercall definitions at
> once rather than be piecemeal.

Actually, I think making it piecemeal might be easier. As in
 1). Not all of the hypercalls are going to be upstreamed.
 2). It gives a nice history/idea of what is actually implemented when
     seaching the code.
 3). git annotate can give precise git commits for when an functionality was
     added.

But maybe I am just overthinking it.

[tboot-devel] Implementation of Flicker 5 on Windows

[Joe N. <joe...@ya...>]

Hello tboot list,
 
Does anyone have experience with implementing Flicker 5 on Windows?  I have a computer that works when Flicker runs on Ubuntu but hangs permanently with the Windows OS.  Has anyone else experienced similar issues?  
 
Thanks,
Joe N.

Re: [tboot-devel] PCR-17 incompletely specified in MLE Dev guide from Mar 2011

[Jonathan M. <jon...@cm...>]

Hi Joseph,

Thanks so much for your detailed reply.  I have indeed gotten things
to work.  For the aid of others, here are some of the useful bits.  I
may post some more complete code in a week or two.

> First, you need to adjust your calculation per the comment about 'SHA-256(SINIT) | EDX'.  Second, the SHA-256 of SINIT is not performed on the entire binary.  Appendix A.1 "alludes" to how the hash is calculated (since it is the same as what is used in the signature):

Indeed, the actual value of PCR 17 after the "first extend" turns out to be:

SHA1(0x00^20 | SHA1(SHA256(SINIT**) | EdxSenterFlags))

SINIT** denotes only part of the sinit module, as Joseph suggested.  More below.

> RSASig
> The PKCS #1.5 RSA Signature of the module. The RSA Signature signs an area that includes the some of the module header and the USER AREA data field (which represents the body of the module). Parts of the module header not included are: the RSA Signature, public key, and scratch field.

Not including the RSA Signature, Public Key, or Scratch field still
leaves a little wiggle room with respect to whether any metadata
(e.g., size information) pertaining to those areas is included.  Here
is what is working for me:

I'm using the definition of acm_hdr_t from acmod.h from tboot-1.6, and
I created a big binary version of the SINIT of interest in a C array
using:
'xxd -i /boot/i5_i7_DUAL_SINIT_18.BIN > i5_i7_DUAL_SINIT_18.h'

(As an aside, xxd may be the greatest program you've never heard of ;-).

Here is a function that recomputes the PCR-17 value from the "first
extend" which should (and does for my tests) match
SinitMleData.SinitHash:

int validate_pcr17_sinit_hash_version7(sinit_mle_data_t *smd) {
  SHA_CTX c;
  unsigned char pcr17[20], inner[20];

  EVP_MD_CTX mdctx;
  const EVP_MD *md;
  unsigned char md_value[EVP_MAX_MD_SIZE];
  int md_len;

  acm_hdr_t *s = (acm_hdr_t*)i5_i7_DUAL_SINIT_18_BIN;

  OpenSSL_add_all_digests();
  if(!(md = EVP_get_digestbyname("sha256"))) return -1;

  EVP_MD_CTX_init(&mdctx);
  EVP_DigestInit_ex(&mdctx, md, NULL);

#define HASHIN(foo) EVP_DigestUpdate(&mdctx, &foo, sizeof(foo)); \
    printf("  size %d\n", sizeof(foo));
  HASHIN(s->module_type);
  HASHIN(s->module_subtype);
  HASHIN(s->header_len);
  HASHIN(s->header_ver);
  HASHIN(s->chipset_id);
  HASHIN(s->flags);
  HASHIN(s->module_vendor);
  HASHIN(s->date);
  HASHIN(s->size);
  HASHIN(s->reserved1);
  HASHIN(s->code_control);
  HASHIN(s->error_entry_point);
  HASHIN(s->gdt_limit);
  HASHIN(s->gdt_base);
  HASHIN(s->seg_sel);
  HASHIN(s->entry_point);
  EVP_DigestUpdate(&mdctx, s->reserved2, sizeof(s->reserved2));
  printf("  size reserved2 %d\n", sizeof(s->reserved2));
  HASHIN(s->key_size);
  HASHIN(s->scratch_size);

  uint32_t user_area_len = i5_i7_DUAL_SINIT_18_BIN_len -
      ((uint32_t)s->user_area - (uint32_t)s);
  printf("  user_area_len %d\n", user_area_len);
  EVP_DigestUpdate(&mdctx, s->user_area, user_area_len);

  EVP_DigestFinal_ex(&mdctx, md_value, &md_len);
  dump_bytes1("sha256(): ", md_value, md_len);
  EVP_MD_CTX_cleanup(&mdctx);

  printf("  md_len %d\n", md_len);

  memset(pcr17, 0, sizeof(sha1_hash_t)); // PCR-17 reset to zero by
GETSEC[SENTER]

  // First compute "inner" SHA-1 (data sent in TPM_HASH_DATA sequence)
  SHA1_Init(&c);
  SHA1_Update(&c, md_value, md_len);
  SHA1_Update(&c, &smd->edx_senter_flags, sizeof(uint32_t));
  SHA1_Final(inner, &c);

  // Second compute Extend(inner)
  SHA1_Init(&c);
  SHA1_Update(&c, pcr17, sizeof(sha1_hash_t)); // First part of
payload existing 20 zeros
  SHA1_Update(&c, inner, sizeof(sha1_hash_t)); // Second part is
"inner" SHA-1 computed above
  SHA1_Final(pcr17, &c); // Compute final digest

  dump_bytes1("pcr17 (1st ext)", pcr17, sizeof(sha1_hash_t));

  if(!memcmp(pcr17, smd->sinit_hash, sizeof(sha1_hash_t))) {
      return 0; /* success */
  }
  return -1; /* failure */
}


So once again, thanks Joseph and JP for your responses.  JP, hopefully
this helps you to no longer take PCR-17 on faith.

Best,
-Jon

Re: [tboot-devel] PCR-17 incompletely specified in MLE Dev guide from Mar 2011

[Cihula, J. <jos...@in...>]

> From: Jonathan McCune [mailto:jon...@cm...]
> Sent: Friday, September 09, 2011 8:56 AM
> 
> Hello again,
> 
> Some progress to report!  I've decided to write in with an incomplete investigation because I
> suspect lots of other folks have similar questions / concerns.
> 
> First, thank you JP for sharing your method.

Indeed--a very good programmatic method for calculating PCR 17.

> Upon reading the spec more carefully, I realize that the nested SHA-1 expression for the contents
> of PCR-17 is indeed correct:
> 
> SHA-1 ( SinitMleData.SinitHash | SHA-1 ( SinitMleData.BiosAcm.ID | SinitMleData.MsegValid |
> SinitMleData.StmHash | SinitMleData.PolicyControl | SinitMleData.LcpPolicyHash |
> (OsSinitData.Capabilities, 0) ) )

Whew ;-)

> What was not initially clear to me is that SinitMleData.SinitHash is not actually the hash of the
> SINIT for a Version 7 SinitMleData.  It is the PCR-17 value following the "first extend".  As with

This is described in the first paragraph of that section:

PCR 17 is initialized using the TPM_HASH_START/TPM_HASH_END sequence.  The HASH_DATA provided in this sequence is the concatenation of the hash of the SINIT ACM that was used in the launch process and the 4 byte value of the SENTER parameters (in EDX and also in SinitMleData.EdxSenterFlags).  As part of this sequence, PCRs 17-23 are reset to 0.  The hash of SINIT is also stored in the SinitMleData.SinitHash field. If the SINIT To MLE Data Table (section C.4) version is 7 or greater, the hash of the SINIT ACM is performed using SHA-256, otherwise it uses SHA-1. If a SHA-256 hash was used, the SinitMleData.SinitHash field will contain the value of PCR 17 after the initial extend operation (see below for more details).

> a version 6 SinitMleData, there are two extends to PCR-17.  The nested SHA-1 expression in the
> spec is actually a verbose version of the "second extend". To me, the following is more clear:
> 
> /* first extend (XXX have not confirmed that these inputs are correct XXX) */
> Extend-PCR17 ( sha-256 hash ( SINIT ACM | EdxSenterFlags ) )

This isn't quite correct:  first, the TPM_HASH_START/DAT/END sequence isn't exactly the same as TPM_Extend, in that DATA can be more than 160 bits.  Second, what is sent as DATA is 'SHA-256(SINIT) | EDX' (i.e. 36 bytes; see above quote from MLE Guide).

> /* not an input to either extend, rather an intermediate value to aid in PCR reconstruction post-
> launch */ Read PCR17 into SinitMleData.SinitHash
> 
> /* second extend (have successfully confirmed that this is indeed correct) */
> Extend-PCR17(SHA-1 ( SinitMleData.BiosAcm.ID | SinitMleData.MsegValid | SinitMleData.StmHash |
> SinitMleData.PolicyControl | SinitMleData.LcpPolicyHash | (OsSinitData.Capabilities, 0) )
> 
> Once more: The "second extend" is equivalent to the SHA-1 expression listed in the spec.
> 
> Now, I have successfully recomputed the "second extend" taking on faith that
> SinitMleData.SinitHash does in fact represent the result of the "first extend" shown above.
> However, I don't want to take it on faith.  I want to recompute it. What follows is my current
> read of the spec, which is so far INCORRECT.  However, I still want to detail my thinking here, in
> case anybody is able to point out where it goes
> wrong:
> 
> The "first extend" listed above:
> Extend-PCR17 ( sha-256 hash(SINIT ACM | EdxSenterFlags ) ) ...breaks apart into:
> SHA-1(0x00^20 | SHA-1 ( SHA-256 ( SINIT ACM | EdxSenterFlags ) ) ) ... and that result should be
> equivalent to the value that I find in SinitMleData.SinitHash.
> 
> Right now the thing that I believe is most likely to be incorrect is the value that I am using for
> "SINIT ACM" in the input to sha-256.  I am simply using the entire contents of the file
> i5_i7_DUAL_SINIT_18.BIN.  I suspect I need to parse that file and strip off some metadata.

First, you need to adjust your calculation per the comment about 'SHA-256(SINIT) | EDX'.  Second, the SHA-256 of SINIT is not performed on the entire binary.  Appendix A.1 "alludes" to how the hash is calculated (since it is the same as what is used in the signature):

RSASig
The PKCS #1.5 RSA Signature of the module. The RSA Signature signs an area that includes the some of the module header and the USER AREA data field (which represents the body of the module). Parts of the module header not included are: the RSA Signature, public key, and scratch field.

> I will write in again when if / when I get this figured out.
> 
> In the mean time, thanks for any hints that anybody can provide!
> 
> Regards,
> -Jon
> 
> ------------------------------------------------------------------------------
> Why Cloud-Based Security and Archiving Make Sense Osterman Research conducted this study that
> outlines how and why cloud computing security and archiving is rapidly being adopted across the IT
> space for its ease of implementation, lower cost, and increased reliability. Learn more.
> http://www.accelacomm.com/jaw/sfnl/114/51425301/
> _______________________________________________
> tboot-devel mailing list
> tbo...@li...
> https://lists.sourceforge.net/lists/listinfo/tboot-devel

Re: [tboot-devel] PCR-17 incompletely specified in MLE Dev guide from Mar 2011

[Jonathan M. <jon...@cm...>]

Hello again,

Some progress to report!  I've decided to write in with an incomplete
investigation because I suspect lots of other folks have similar
questions / concerns.

First, thank you JP for sharing your method.

Upon reading the spec more carefully, I realize that the nested SHA-1
expression for the contents of PCR-17 is indeed correct:

SHA-1 ( SinitMleData.SinitHash | SHA-1 ( SinitMleData.BiosAcm.ID |
SinitMleData.MsegValid | SinitMleData.StmHash | SinitMleData.PolicyControl |
SinitMleData.LcpPolicyHash | (OsSinitData.Capabilities, 0) ) )

What was not initially clear to me is that SinitMleData.SinitHash is
not actually the hash of the SINIT for a Version 7 SinitMleData.  It
is the PCR-17 value following the "first extend".  As with a version 6
SinitMleData, there are two extends to PCR-17.  The nested SHA-1
expression in the spec is actually a verbose version of the "second
extend". To me, the following is more clear:

/* first extend (XXX have not confirmed that these inputs are correct XXX) */
Extend-PCR17 ( sha-256 hash ( SINIT ACM | EdxSenterFlags ) )

/* not an input to either extend, rather an intermediate value to aid
in PCR reconstruction post-launch */
Read PCR17 into SinitMleData.SinitHash

/* second extend (have successfully confirmed that this is indeed correct) */
Extend-PCR17(SHA-1 ( SinitMleData.BiosAcm.ID |
SinitMleData.MsegValid | SinitMleData.StmHash | SinitMleData.PolicyControl |
SinitMleData.LcpPolicyHash | (OsSinitData.Capabilities, 0) )

Once more: The "second extend" is equivalent to the SHA-1 expression
listed in the spec.

Now, I have successfully recomputed the "second extend" taking on
faith that SinitMleData.SinitHash does in fact represent the result of
the "first extend" shown above.  However, I don't want to take it on
faith.  I want to recompute it. What follows is my current read of the
spec, which is so far INCORRECT.  However, I still want to detail my
thinking here, in case anybody is able to point out where it goes
wrong:

The "first extend" listed above:
Extend-PCR17 ( sha-256 hash(SINIT ACM | EdxSenterFlags ) )
...breaks apart into:
SHA-1(0x00^20 | SHA-1 ( SHA-256 ( SINIT ACM | EdxSenterFlags ) ) )
... and that result should be equivalent to the value that I find in
SinitMleData.SinitHash.

Right now the thing that I believe is most likely to be incorrect is
the value that I am using for "SINIT ACM" in the input to sha-256.  I
am simply using the entire contents of the file
i5_i7_DUAL_SINIT_18.BIN.  I suspect I need to parse that file and
strip off some metadata.

I will write in again when if / when I get this figured out.

In the mean time, thanks for any hints that anybody can provide!

Regards,
-Jon

Re: [tboot-devel] PCR-17 incompletely specified in MLE Dev guide from Mar 2011

[JP B. <jp...@co...>]

Jonathan McCune <jonmccune <at> cmu.edu> writes:

> 
> Hi list,
> 
> I'm having a tough time recomputing the values that I find in PCR 17
> following SENTER.   I'm using i5_i7_DUAL_SINIT_18.BIN (I believe this
> to be the latest available), which produces a version 7
> sinit_to_mle_data.
> 
> Looking at Section 1.9.1 in the spec, it's quite unclear to me exactly
> what it's trying to tell me will be in PCR 17.
> 
> >From the spec:
> 
> If SinitMleData.Version = 7, PCR 17’s final value will be:
> 
> SHA-1 ( SinitMleData.SinitHash | SHA-1 ( SinitMleData.BiosAcm.ID |
> SinitMleData.MsegValid | SinitMleData.StmHash | SinitMleData.PolicyControl |
> SinitMleData.LcpPolicyHash | (OsSinitData.Capabilities, 0) ) )
> 
> There are at least three problems with this.  (1) There is no mention
> of a PCR Extend (contrast with the clear explanation for version 6
> sinit_to_mle_data which I have used successfully in the past), and (2)
> This expression is inconsistent with the paragraph at the start of
> Section 1.9.1 which mentions the use of SHA-256 to hash the "SINIT
> ACM".  I don't see SHA-256 in that expression anywhere. (3) It is not
> mentioned whether the 4-byte EdxSenterFlags is also input to SHA-256
> or not.
> 
> I'd rather not try to map out all the different possibilities and
> figure this out experimentally.
> 
> Things don't look much better for version 8 but I don't have the
> hardware to test that.
> 
> Thanks!
> -Jon
> 
> ------------------------------------------------------------------------------
> Doing More with Less: The Next Generation Virtual Desktop 
> What are the key obstacles that have prevented many mid-market businesses
> from deploying virtual desktops?   How do next-generation virtual desktops
> provide companies an easier-to-deploy, easier-to-manage and more affordable
> virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
> 


These are the steps I took to repro PCR-17.  Hope it helps.

###############################################################################
#PCR17 Calculation (SinitMleData_Version=7)
###############################################################################
SinitMleData_SinitHash=$( txt-stat | grep sinit_hash -A1 | tail -n1 | awk
'{gsub(/ /,"");print $1}')

SinitMleData_BiosAcmID==$( txt-stat | grep bios_acm_id -A1 | tail -n1 | awk
'{gsub(/ /,"");print $1}')

SinitMleData_MsegValid="0000000000000000"
SinitMleData_StmHash="0000000000000000000000000000000000000000"
SinitMleData_PolicyControl="00000000"

# will be non-zero if an LCP is defined
SinitMleData_LcpPolicyHash=$( txt-stat | grep lcp_policy_hash -A1 | tail -n1 |
awk '{gsub(/ /,"");print $1}')

OsSinitData_Capabilities="00000000"

#as defined in Sec 1.9.1 (pp.14) of "Intel TXT Software Dev Guide, MLE Dev
Guide, March 2011"
PCR17_PARTB1="$SinitMleData_BiosAcmID$SinitMleData_MsegValid$SinitMleData_StmHa
sh$SinitMleData_PolicyControl$SinitMleData_LcpPolicyHash$OsSinitData_Capabiliti
es"

PCR17_STEP1=$(echo -n "$PCR17_PARTB1" | ./binaryfi |  sha1sum |  awk '{ print 
$1 }')
echo "value extending PCR17 (i.e. sinit_hash) = $(fmt_bytestr $PCR17_STEP1)"

PCR17_PRE_VL=$(echo -n "$SinitMleData_SinitHash$PCR17_STEP1" | ./binaryfi |
sha1sum | awk '{ print $1 }' )

POLICY_HASH=$( txt-stat | grep " pol_hash:" |  awk -F 'pol_hash:' '{ gsub(/ 
/,""); print $2}')

# Using policy_control from `txt-stat | grep " policy_control:" | head -n 1`
# Remember endianess!
POLICY_CONTROL="01000000"

PCR17_VL_MEAS=$(echo -n "$POLICY_CONTROL$POLICY_HASH" | ./binaryfi | sha1sum |
awk '{ print $1 }')

echo "PENULTIMATE_PCR17= $(fmt_bytestr $PCR17_PRE_VL)"
echo "PCR17_VL_MEAS= $(fmt_bytestr $PCR17_VL_MEAS)"

# -o is old value, -n is new value
PCR17_FINAL=$(./mimic_extend.sh -o "$PCR17_PRE_VL" -n "$PCR17_VL_MEAS")


-JP

[tboot-devel] PCR-17 incompletely specified in MLE Dev guide from Mar 2011

[Jonathan M. <jon...@cm...>]

Hi list,

I'm having a tough time recomputing the values that I find in PCR 17
following SENTER.   I'm using i5_i7_DUAL_SINIT_18.BIN (I believe this
to be the latest available), which produces a version 7
sinit_to_mle_data.

Looking at Section 1.9.1 in the spec, it's quite unclear to me exactly
what it's trying to tell me will be in PCR 17.

>From the spec:

If SinitMleData.Version = 7, PCR 17’s final value will be:

SHA-1 ( SinitMleData.SinitHash | SHA-1 ( SinitMleData.BiosAcm.ID |
SinitMleData.MsegValid | SinitMleData.StmHash | SinitMleData.PolicyControl |
SinitMleData.LcpPolicyHash | (OsSinitData.Capabilities, 0) ) )

There are at least three problems with this.  (1) There is no mention
of a PCR Extend (contrast with the clear explanation for version 6
sinit_to_mle_data which I have used successfully in the past), and (2)
This expression is inconsistent with the paragraph at the start of
Section 1.9.1 which mentions the use of SHA-256 to hash the "SINIT
ACM".  I don't see SHA-256 in that expression anywhere. (3) It is not
mentioned whether the 4-byte EdxSenterFlags is also input to SHA-256
or not.

I'd rather not try to map out all the different possibilities and
figure this out experimentally.

Things don't look much better for version 8 but I don't have the
hardware to test that.

Thanks!
-Jon

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Konrad R. W. <kon...@or...>]

On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > >> +#define XENPF_enter_acpi_sleep    51
> > > >> +struct xenpf_enter_acpi_sleep {
> > > >> +	/* IN variables */
> > > >> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> > > >> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > > 
> > > Does ACPI define them as 32 or 16 bit?
> > 
> > The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which your other patch is truncating for this call.
> 
> Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> and also address all the other comments (thanks for looking at it) you pointed
> out.

So read up the ACPI spec and it says that the minimum is 2 bytes and does not
say anything about the maximum. The list of what the bits do stops at 16-bits
(the last two are reserved) so I think we are actually OK.

Albeit if the spec starts using more of them - then yes we will need to revist
this Xen ABI and potentially add a new call.

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Konrad R. W. <kon...@or...>]

> > >> +#define XENPF_enter_acpi_sleep    51
> > >> +struct xenpf_enter_acpi_sleep {
> > >> +	/* IN variables */
> > >> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> > >> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > 
> > Does ACPI define them as 32 or 16 bit?
> 
> The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which your other patch is truncating for this call.

Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
and also address all the other comments (thanks for looking at it) you pointed
out.

Re: [tboot-devel] [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

[Cihula, J. <jos...@in...>]

> From: Jeremy Fitzhardinge [mailto:je...@go...]
> Sent: Wednesday, September 07, 2011 10:27 AM
> 
> On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> The ACPI suspend path makes a call to tboot_sleep right before it
> >> writes the PM1A, PM1B values. We replace the direct call to tboot via an registration callback
> similar to __acpi_register_gsi.
> >>
> >> CC: Thomas Gleixner <tg...@li...>
> >> CC: "H. Peter Anvin" <hp...@zy...>
> >> CC: x8...@ke...
> >> CC: Len Brown <len...@in...>
> >> CC: Joseph Cihula <jos...@in...>
> >> CC: Shane Wang <sha...@in...>
> >> CC: xen...@li...
> >> CC: lin...@li...
> >> CC: tbo...@li...
> >> CC: lin...@vg...
> >> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> >> ---
> >>  arch/x86/include/asm/acpi.h   |    3 +++
> >>  arch/x86/kernel/acpi/boot.c   |    3 +++
> >>  arch/x86/kernel/tboot.c       |   13 +++++++++----
> >>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
> >>  include/linux/tboot.h         |    3 ++-
> >>  5 files changed, 27 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/acpi.h
> >> b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> >> 100644
> >> --- a/arch/x86/include/asm/acpi.h
> >> +++ b/arch/x86/include/asm/acpi.h
> >> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16);
> >> extern int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> >>  				  int trigger, int polarity);
> >>
> >> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> >> +				    u32 pm1b_ctrl, bool *skip_rest);
> >> +
> >>  static inline void disable_acpi(void)  {
> >>  	acpi_disabled = 1;
> >> diff --git a/arch/x86/kernel/acpi/boot.c
> >> b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
> >> 100644
> >> --- a/arch/x86/kernel/acpi/boot.c
> >> +++ b/arch/x86/kernel/acpi/boot.c
> >> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device
> >> *dev, u32 gsi,  int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> >>  			   int trigger, int polarity) = acpi_register_gsi_pic;
> >>
> >> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> >> +			     u32 pm1b_ctrl, bool *skip_rest) = NULL;
> >> +
> >>  /*
> >>   * success: return IRQ number (>=0)
> >>   * failure: return < 0
> >> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index
> >> 30ac65d..a18070c 100644
> >> --- a/arch/x86/kernel/tboot.c
> >> +++ b/arch/x86/kernel/tboot.c
> >> @@ -41,7 +41,7 @@
> >>  #include <asm/setup.h>
> >>  #include <asm/e820.h>
> >>  #include <asm/io.h>
> >> -
> >> +#include <linux/acpi.h>
> >>  #include "acpi/realmode/wakeup.h"
> >>
> >>  /* Global pointer to shared data; NULL means no measured launch. */
> >> @@ -270,7 +270,8 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
> >>  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
> >>
> >> -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> >> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> >> +		 bool *skip_rest)
> > Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?
> 
> No, gcc doesn't warn about unused parameters.

-Wunused-parameter

While the kernel may not be compiled with this flag, it wouldn't hurt to specify it anyway; but it's not a big issue.

Joe

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Jeremy F. <je...@go...>]

On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> From: Yu Ke <ke...@in...>
>>
>> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>>
>> Signed-off-by: Yu Ke <ke...@in...>
>> Signed-off-by: Tian Kevin <kev...@in...>
>> Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
>> [v1: Added DEFINE_GUEST.. in appropiate headers]
>> [v2: Ripped out typedefs]
>> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
>> ---
>>  arch/ia64/include/asm/xen/interface.h |    1 +
>>  arch/x86/include/asm/xen/interface.h  |    1 +
>>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>>  include/xen/interface/xen.h           |    1 +
>>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
>> include/xen/interface/platform.h
>>
>> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
>> index e951e74..1d2427d 100644
>> --- a/arch/ia64/include/asm/xen/interface.h
>> +++ b/arch/ia64/include/asm/xen/interface.h
>> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
>> +DEFINE_GUEST_HANDLE(uint64_t);
>>
>>  typedef unsigned long xen_pfn_t;
>>  DEFINE_GUEST_HANDLE(xen_pfn_t);
>> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
>> index 5d4922a..a1f2db5 100644
>> --- a/arch/x86/include/asm/xen/interface.h
>> +++ b/arch/x86/include/asm/xen/interface.h
>> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
>> +DEFINE_GUEST_HANDLE(uint64_t);
>>  #endif
>>
>>  #ifndef HYPERVISOR_VIRT_START
>> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
>> new file mode 100644
>> index 0000000..c168468
>> --- /dev/null
>> +++ b/include/xen/interface/platform.h
> Why are you adding so many new hypercalls that aren't being used in this patchset?

May as well bring in all the platform-related hypercall definitions at
once rather than be piecemeal.

>> @@ -0,0 +1,320 @@
>> +/**********************************************************************
>> +********
>> + * platform.h
>> + *
>> + * Hardware platform operations. Intended for use by domain-0 kernel.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a copy
>> + * of this software and associated documentation files (the
>> +"Software"), to
>> + * deal in the Software without restriction, including without
>> +limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense,
>> +and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> +Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> +included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> +SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> +OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Copyright (c) 2002-2006, K Fraser
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_PLATFORM_H__
>> +#define __XEN_PUBLIC_PLATFORM_H__
>> +
>> +#include "xen.h"
>> +
>> +#define XENPF_INTERFACE_VERSION 0x03000001
>> +
>> +/*
>> + * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
>> + * 1 January, 1970 if the current system time was <system_time>.
>> + */
>> +#define XENPF_settime             17
>> +struct xenpf_settime {
>> +	/* IN variables. */
>> +	uint32_t secs;
>> +	uint32_t nsecs;
>> +	uint64_t system_time;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
>> +
>> +/*
>> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
>> + * On x86, @type is an architecture-defined MTRR memory type.
>> + * On success, returns the MTRR that was used (@reg) and a handle that
>> +can
>> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
>> + * (x86-specific).
>> + */
>> +#define XENPF_add_memtype         31
>> +struct xenpf_add_memtype {
>> +	/* IN variables. */
>> +	unsigned long mfn;
>> +	uint64_t nr_mfns;
>> +	uint32_t type;
>> +	/* OUT variables. */
>> +	uint32_t handle;
>> +	uint32_t reg;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
>> +
>> +/*
>> + * Tear down an existing memory-range type. If @handle is remembered
>> +then it
>> + * should be passed in to accurately tear down the correct setting (in
>> +case
>> + * of overlapping memory regions with differing types). If it is not
>> +known
>> + * then @handle should be set to zero. In all cases @reg must be set.
>> + * (x86-specific).
>> + */
>> +#define XENPF_del_memtype         32
>> +struct xenpf_del_memtype {
>> +	/* IN variables. */
>> +	uint32_t handle;
>> +	uint32_t reg;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
>> +
>> +/* Read current type of an MTRR (x86-specific). */
>> +#define XENPF_read_memtype        33
>> +struct xenpf_read_memtype {
>> +	/* IN variables. */
>> +	uint32_t reg;
>> +	/* OUT variables. */
>> +	unsigned long mfn;
>> +	uint64_t nr_mfns;
>> +	uint32_t type;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
>> +
>> +#define XENPF_microcode_update    35
>> +struct xenpf_microcode_update {
>> +	/* IN variables. */
>> +	GUEST_HANDLE(void) data;          /* Pointer to microcode data */
>> +	uint32_t length;                  /* Length of microcode data. */
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
>> +
>> +#define XENPF_platform_quirk      39
>> +#define QUIRK_NOIRQBALANCING      1 /* Do not restrict IO-APIC RTE targets */
>> +#define QUIRK_IOAPIC_BAD_REGSEL   2 /* IO-APIC REGSEL forgets its value    */
>> +#define QUIRK_IOAPIC_GOOD_REGSEL  3 /* IO-APIC REGSEL behaves properly     */
>> +struct xenpf_platform_quirk {
>> +	/* IN variables. */
>> +	uint32_t quirk_id;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
>> +
>> +#define XENPF_firmware_info       50
>> +#define XEN_FW_DISK_INFO          1 /* from int 13 AH=08/41/48 */
>> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
>> +#define XEN_FW_VBEDDC_INFO        3 /* from int 10 AX=4f15 */
>> +struct xenpf_firmware_info {
>> +	/* IN variables. */
>> +	uint32_t type;
>> +	uint32_t index;
>> +	/* OUT variables. */
>> +	union {
>> +		struct {
>> +			/* Int13, Fn48: Check Extensions Present. */
>> +			uint8_t device;                   /* %dl: bios device number */
>> +			uint8_t version;                  /* %ah: major version      */
>> +			uint16_t interface_support;       /* %cx: support bitmap     */
>> +			/* Int13, Fn08: Legacy Get Device Parameters. */
>> +			uint16_t legacy_max_cylinder;     /* %cl[7:6]:%ch: max cyl # */
>> +			uint8_t legacy_max_head;          /* %dh: max head #         */
>> +			uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector #  */
>> +			/* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
>> +			/* NB. First uint16_t of buffer must be set to buffer size.      */
>> +			GUEST_HANDLE(void) edd_params;
>> +		} disk_info; /* XEN_FW_DISK_INFO */
>> +		struct {
>> +			uint8_t device;                   /* bios device number  */
>> +			uint32_t mbr_signature;           /* offset 0x1b8 in mbr */
>> +		} disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
>> +		struct {
>> +			/* Int10, AX=4F15: Get EDID info. */
>> +			uint8_t capabilities;
>> +			uint8_t edid_transfer_time;
>> +			/* must refer to 128-byte buffer */
>> +			GUEST_HANDLE(uchar) edid;
>> +		} vbeddc_info; /* XEN_FW_VBEDDC_INFO */
>> +	} u;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
>> +
>> +#define XENPF_enter_acpi_sleep    51
>> +struct xenpf_enter_acpi_sleep {
>> +	/* IN variables */
>> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
>> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> These are uint32_t in native Linux--why truncate in the API and not at use?

Does ACPI define them as 32 or 16 bit?

    J

Re: [tboot-devel] [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

[Jeremy F. <je...@go...>]

On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> The ACPI suspend path makes a call to tboot_sleep right before it writes the PM1A, PM1B values. We
>> replace the direct call to tboot via an registration callback similar to __acpi_register_gsi.
>>
>> CC: Thomas Gleixner <tg...@li...>
>> CC: "H. Peter Anvin" <hp...@zy...>
>> CC: x8...@ke...
>> CC: Len Brown <len...@in...>
>> CC: Joseph Cihula <jos...@in...>
>> CC: Shane Wang <sha...@in...>
>> CC: xen...@li...
>> CC: lin...@li...
>> CC: tbo...@li...
>> CC: lin...@vg...
>> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
>> ---
>>  arch/x86/include/asm/acpi.h   |    3 +++
>>  arch/x86/kernel/acpi/boot.c   |    3 +++
>>  arch/x86/kernel/tboot.c       |   13 +++++++++----
>>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
>>  include/linux/tboot.h         |    3 ++-
>>  5 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
>> 100644
>> --- a/arch/x86/include/asm/acpi.h
>> +++ b/arch/x86/include/asm/acpi.h
>> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16);  extern int
>> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>>  				  int trigger, int polarity);
>>
>> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
>> +				    u32 pm1b_ctrl, bool *skip_rest);
>> +
>>  static inline void disable_acpi(void)
>>  {
>>  	acpi_disabled = 1;
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
>> 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,  int
>> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>>  			   int trigger, int polarity) = acpi_register_gsi_pic;
>>
>> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
>> +			     u32 pm1b_ctrl, bool *skip_rest) = NULL;
>> +
>>  /*
>>   * success: return IRQ number (>=0)
>>   * failure: return < 0
>> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 30ac65d..a18070c 100644
>> --- a/arch/x86/kernel/tboot.c
>> +++ b/arch/x86/kernel/tboot.c
>> @@ -41,7 +41,7 @@
>>  #include <asm/setup.h>
>>  #include <asm/e820.h>
>>  #include <asm/io.h>
>> -
>> +#include <linux/acpi.h>
>>  #include "acpi/realmode/wakeup.h"
>>
>>  /* Global pointer to shared data; NULL means no measured launch. */ @@ -270,7 +270,8 @@ static
>> void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
>>  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
>>
>> -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
>> +		 bool *skip_rest)
> Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?

No, gcc doesn't warn about unused parameters.

    J

Re: [tboot-devel] [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Cihula, J. <jos...@in...>]

> From: Jeremy Fitzhardinge [mailto:je...@go...]
> Sent: Wednesday, September 07, 2011 10:29 AM
> 
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke...@in...>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to
> hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke...@in...>
> >> Signed-off-by: Tian Kevin <kev...@in...>
> >> Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> >> ---
> >>  arch/ia64/include/asm/xen/interface.h |    1 +
> >>  arch/x86/include/asm/xen/interface.h  |    1 +
> >>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
> >>  include/xen/interface/xen.h           |    1 +
> >>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode
> >> 100644 include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h
> >> b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
> >> +++ b/arch/ia64/include/asm/xen/interface.h
> >> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);
> >> DEFINE_GUEST_HANDLE(int); DEFINE_GUEST_HANDLE(long);
> >> DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >>
> >>  typedef unsigned long xen_pfn_t;
> >>  DEFINE_GUEST_HANDLE(xen_pfn_t);
> >> diff --git a/arch/x86/include/asm/xen/interface.h
> >> b/arch/x86/include/asm/xen/interface.h
> >> index 5d4922a..a1f2db5 100644
> >> --- a/arch/x86/include/asm/xen/interface.h
> >> +++ b/arch/x86/include/asm/xen/interface.h
> >> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);
> >> DEFINE_GUEST_HANDLE(int); DEFINE_GUEST_HANDLE(long);
> >> DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >>  #endif
> >>
> >>  #ifndef HYPERVISOR_VIRT_START
> >> diff --git a/include/xen/interface/platform.h
> >> b/include/xen/interface/platform.h
> >> new file mode 100644
> >> index 0000000..c168468
> >> --- /dev/null
> >> +++ b/include/xen/interface/platform.h
> > Why are you adding so many new hypercalls that aren't being used in this patchset?
> 
> May as well bring in all the platform-related hypercall definitions at once rather than be
> piecemeal.
> 
> >> @@ -0,0 +1,320 @@
> >> +/*******************************************************************
> >> +***
> >> +********
> >> + * platform.h
> >> + *
> >> + * Hardware platform operations. Intended for use by domain-0 kernel.
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> +obtaining a copy
> >> + * of this software and associated documentation files (the
> >> +"Software"), to
> >> + * deal in the Software without restriction, including without
> >> +limitation the
> >> + * rights to use, copy, modify, merge, publish, distribute,
> >> +sublicense, and/or
> >> + * sell copies of the Software, and to permit persons to whom the
> >> +Software is
> >> + * furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice shall be
> >> +included in
> >> + * all copies or substantial portions of the Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> +EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> +MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> >> +SHALL THE
> >> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >> +OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> +ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> +OTHER
> >> + * DEALINGS IN THE SOFTWARE.
> >> + *
> >> + * Copyright (c) 2002-2006, K Fraser  */
> >> +
> >> +#ifndef __XEN_PUBLIC_PLATFORM_H__
> >> +#define __XEN_PUBLIC_PLATFORM_H__
> >> +
> >> +#include "xen.h"
> >> +
> >> +#define XENPF_INTERFACE_VERSION 0x03000001
> >> +
> >> +/*
> >> + * Set clock such that it would read <secs,nsecs> after 00:00:00
> >> +UTC,
> >> + * 1 January, 1970 if the current system time was <system_time>.
> >> + */
> >> +#define XENPF_settime             17
> >> +struct xenpf_settime {
> >> +	/* IN variables. */
> >> +	uint32_t secs;
> >> +	uint32_t nsecs;
> >> +	uint64_t system_time;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> >> +
> >> +/*
> >> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> >> + * On x86, @type is an architecture-defined MTRR memory type.
> >> + * On success, returns the MTRR that was used (@reg) and a handle
> >> +that can
> >> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
> >> + * (x86-specific).
> >> + */
> >> +#define XENPF_add_memtype         31
> >> +struct xenpf_add_memtype {
> >> +	/* IN variables. */
> >> +	unsigned long mfn;
> >> +	uint64_t nr_mfns;
> >> +	uint32_t type;
> >> +	/* OUT variables. */
> >> +	uint32_t handle;
> >> +	uint32_t reg;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
> >> +
> >> +/*
> >> + * Tear down an existing memory-range type. If @handle is remembered
> >> +then it
> >> + * should be passed in to accurately tear down the correct setting
> >> +(in case
> >> + * of overlapping memory regions with differing types). If it is not
> >> +known
> >> + * then @handle should be set to zero. In all cases @reg must be set.
> >> + * (x86-specific).
> >> + */
> >> +#define XENPF_del_memtype         32
> >> +struct xenpf_del_memtype {
> >> +	/* IN variables. */
> >> +	uint32_t handle;
> >> +	uint32_t reg;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
> >> +
> >> +/* Read current type of an MTRR (x86-specific). */
> >> +#define XENPF_read_memtype        33
> >> +struct xenpf_read_memtype {
> >> +	/* IN variables. */
> >> +	uint32_t reg;
> >> +	/* OUT variables. */
> >> +	unsigned long mfn;
> >> +	uint64_t nr_mfns;
> >> +	uint32_t type;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
> >> +
> >> +#define XENPF_microcode_update    35
> >> +struct xenpf_microcode_update {
> >> +	/* IN variables. */
> >> +	GUEST_HANDLE(void) data;          /* Pointer to microcode data */
> >> +	uint32_t length;                  /* Length of microcode data. */
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
> >> +
> >> +#define XENPF_platform_quirk      39
> >> +#define QUIRK_NOIRQBALANCING      1 /* Do not restrict IO-APIC RTE targets */
> >> +#define QUIRK_IOAPIC_BAD_REGSEL   2 /* IO-APIC REGSEL forgets its value    */
> >> +#define QUIRK_IOAPIC_GOOD_REGSEL  3 /* IO-APIC REGSEL behaves properly     */
> >> +struct xenpf_platform_quirk {
> >> +	/* IN variables. */
> >> +	uint32_t quirk_id;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
> >> +
> >> +#define XENPF_firmware_info       50
> >> +#define XEN_FW_DISK_INFO          1 /* from int 13 AH=08/41/48 */
> >> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
> >> +#define XEN_FW_VBEDDC_INFO        3 /* from int 10 AX=4f15 */
> >> +struct xenpf_firmware_info {
> >> +	/* IN variables. */
> >> +	uint32_t type;
> >> +	uint32_t index;
> >> +	/* OUT variables. */
> >> +	union {
> >> +		struct {
> >> +			/* Int13, Fn48: Check Extensions Present. */
> >> +			uint8_t device;                   /* %dl: bios device number */
> >> +			uint8_t version;                  /* %ah: major version      */
> >> +			uint16_t interface_support;       /* %cx: support bitmap     */
> >> +			/* Int13, Fn08: Legacy Get Device Parameters. */
> >> +			uint16_t legacy_max_cylinder;     /* %cl[7:6]:%ch: max cyl # */
> >> +			uint8_t legacy_max_head;          /* %dh: max head #         */
> >> +			uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector #  */
> >> +			/* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
> >> +			/* NB. First uint16_t of buffer must be set to buffer size.      */
> >> +			GUEST_HANDLE(void) edd_params;
> >> +		} disk_info; /* XEN_FW_DISK_INFO */
> >> +		struct {
> >> +			uint8_t device;                   /* bios device number  */
> >> +			uint32_t mbr_signature;           /* offset 0x1b8 in mbr */
> >> +		} disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
> >> +		struct {
> >> +			/* Int10, AX=4F15: Get EDID info. */
> >> +			uint8_t capabilities;
> >> +			uint8_t edid_transfer_time;
> >> +			/* must refer to 128-byte buffer */
> >> +			GUEST_HANDLE(uchar) edid;
> >> +		} vbeddc_info; /* XEN_FW_VBEDDC_INFO */
> >> +	} u;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
> >> +
> >> +#define XENPF_enter_acpi_sleep    51
> >> +struct xenpf_enter_acpi_sleep {
> >> +	/* IN variables */
> >> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> >> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> > These are uint32_t in native Linux--why truncate in the API and not at use?
> 
> Does ACPI define them as 32 or 16 bit?

The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which your other patch is truncating for this call.

Joe

Re: [tboot-devel] [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Cihula, J. <jos...@in...>]

> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> From: Yu Ke <ke...@in...>
> 
> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
> 
> Signed-off-by: Yu Ke <ke...@in...>
> Signed-off-by: Tian Kevin <kev...@in...>
> Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
> [v1: Added DEFINE_GUEST.. in appropiate headers]
> [v2: Ripped out typedefs]
> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> ---
>  arch/ia64/include/asm/xen/interface.h |    1 +
>  arch/x86/include/asm/xen/interface.h  |    1 +
>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>  include/xen/interface/xen.h           |    1 +
>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
> include/xen/interface/platform.h
> 
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index e951e74..1d2427d 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
> 
>  typedef unsigned long xen_pfn_t;
>  DEFINE_GUEST_HANDLE(xen_pfn_t);
> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> index 5d4922a..a1f2db5 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
>  #endif
> 
>  #ifndef HYPERVISOR_VIRT_START
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> new file mode 100644
> index 0000000..c168468
> --- /dev/null
> +++ b/include/xen/interface/platform.h

Why are you adding so many new hypercalls that aren't being used in this patchset?

> @@ -0,0 +1,320 @@
> +/**********************************************************************
> +********
> + * platform.h
> + *
> + * Hardware platform operations. Intended for use by domain-0 kernel.
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a copy
> + * of this software and associated documentation files (the
> +"Software"), to
> + * deal in the Software without restriction, including without
> +limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense,
> +and/or
> + * sell copies of the Software, and to permit persons to whom the
> +Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> +SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2002-2006, K Fraser
> + */
> +
> +#ifndef __XEN_PUBLIC_PLATFORM_H__
> +#define __XEN_PUBLIC_PLATFORM_H__
> +
> +#include "xen.h"
> +
> +#define XENPF_INTERFACE_VERSION 0x03000001
> +
> +/*
> + * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
> + * 1 January, 1970 if the current system time was <system_time>.
> + */
> +#define XENPF_settime             17
> +struct xenpf_settime {
> +	/* IN variables. */
> +	uint32_t secs;
> +	uint32_t nsecs;
> +	uint64_t system_time;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> +
> +/*
> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> + * On x86, @type is an architecture-defined MTRR memory type.
> + * On success, returns the MTRR that was used (@reg) and a handle that
> +can
> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
> + * (x86-specific).
> + */
> +#define XENPF_add_memtype         31
> +struct xenpf_add_memtype {
> +	/* IN variables. */
> +	unsigned long mfn;
> +	uint64_t nr_mfns;
> +	uint32_t type;
> +	/* OUT variables. */
> +	uint32_t handle;
> +	uint32_t reg;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
> +
> +/*
> + * Tear down an existing memory-range type. If @handle is remembered
> +then it
> + * should be passed in to accurately tear down the correct setting (in
> +case
> + * of overlapping memory regions with differing types). If it is not
> +known
> + * then @handle should be set to zero. In all cases @reg must be set.
> + * (x86-specific).
> + */
> +#define XENPF_del_memtype         32
> +struct xenpf_del_memtype {
> +	/* IN variables. */
> +	uint32_t handle;
> +	uint32_t reg;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
> +
> +/* Read current type of an MTRR (x86-specific). */
> +#define XENPF_read_memtype        33
> +struct xenpf_read_memtype {
> +	/* IN variables. */
> +	uint32_t reg;
> +	/* OUT variables. */
> +	unsigned long mfn;
> +	uint64_t nr_mfns;
> +	uint32_t type;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
> +
> +#define XENPF_microcode_update    35
> +struct xenpf_microcode_update {
> +	/* IN variables. */
> +	GUEST_HANDLE(void) data;          /* Pointer to microcode data */
> +	uint32_t length;                  /* Length of microcode data. */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
> +
> +#define XENPF_platform_quirk      39
> +#define QUIRK_NOIRQBALANCING      1 /* Do not restrict IO-APIC RTE targets */
> +#define QUIRK_IOAPIC_BAD_REGSEL   2 /* IO-APIC REGSEL forgets its value    */
> +#define QUIRK_IOAPIC_GOOD_REGSEL  3 /* IO-APIC REGSEL behaves properly     */
> +struct xenpf_platform_quirk {
> +	/* IN variables. */
> +	uint32_t quirk_id;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
> +
> +#define XENPF_firmware_info       50
> +#define XEN_FW_DISK_INFO          1 /* from int 13 AH=08/41/48 */
> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
> +#define XEN_FW_VBEDDC_INFO        3 /* from int 10 AX=4f15 */
> +struct xenpf_firmware_info {
> +	/* IN variables. */
> +	uint32_t type;
> +	uint32_t index;
> +	/* OUT variables. */
> +	union {
> +		struct {
> +			/* Int13, Fn48: Check Extensions Present. */
> +			uint8_t device;                   /* %dl: bios device number */
> +			uint8_t version;                  /* %ah: major version      */
> +			uint16_t interface_support;       /* %cx: support bitmap     */
> +			/* Int13, Fn08: Legacy Get Device Parameters. */
> +			uint16_t legacy_max_cylinder;     /* %cl[7:6]:%ch: max cyl # */
> +			uint8_t legacy_max_head;          /* %dh: max head #         */
> +			uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector #  */
> +			/* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
> +			/* NB. First uint16_t of buffer must be set to buffer size.      */
> +			GUEST_HANDLE(void) edd_params;
> +		} disk_info; /* XEN_FW_DISK_INFO */
> +		struct {
> +			uint8_t device;                   /* bios device number  */
> +			uint32_t mbr_signature;           /* offset 0x1b8 in mbr */
> +		} disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
> +		struct {
> +			/* Int10, AX=4F15: Get EDID info. */
> +			uint8_t capabilities;
> +			uint8_t edid_transfer_time;
> +			/* must refer to 128-byte buffer */
> +			GUEST_HANDLE(uchar) edid;
> +		} vbeddc_info; /* XEN_FW_VBEDDC_INFO */
> +	} u;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
> +
> +#define XENPF_enter_acpi_sleep    51
> +struct xenpf_enter_acpi_sleep {
> +	/* IN variables */
> +	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> +	uint16_t pm1b_cnt_val;      /* PM1b control value. */

These are uint32_t in native Linux--why truncate in the API and not at use?

> +	uint32_t sleep_state;       /* Which state to enter (Sn). */
> +	uint32_t flags;             /* Must be zero. */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
> +
> +#define XENPF_change_freq         52
> +struct xenpf_change_freq {
> +	/* IN variables */
> +	uint32_t flags; /* Must be zero. */
> +	uint32_t cpu;   /* Physical cpu. */
> +	uint64_t freq;  /* New frequency (Hz). */ };
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_change_freq_t);
> +
> +/*
> + * Get idle times (nanoseconds since boot) for physical CPUs specified
> +in the
> + * @cpumap_bitmap with range [0..@cpumap_nr_cpus-1]. The @idletime
> +array is
> + * indexed by CPU number; only entries with the corresponding
> +@cpumap_bitmap
> + * bit set are written to. On return, @cpumap_bitmap is modified so
> +that any
> + * non-existent CPUs are cleared. Such CPUs have their @idletime array
> +entry
> + * cleared.
> + */
> +#define XENPF_getidletime         53
> +struct xenpf_getidletime {
> +	/* IN/OUT variables */
> +	/* IN: CPUs to interrogate; OUT: subset of IN which are present */
> +	GUEST_HANDLE(uchar) cpumap_bitmap;
> +	/* IN variables */
> +	/* Size of cpumap bitmap. */
> +	uint32_t cpumap_nr_cpus;
> +	/* Must be indexable for every cpu in cpumap_bitmap. */
> +	GUEST_HANDLE(uint64_t) idletime;
> +	/* OUT variables */
> +	/* System time when the idletime snapshots were taken. */
> +	uint64_t now;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_getidletime_t);
> +
> +#define XENPF_set_processor_pminfo      54
> +
> +/* ability bits */
> +#define XEN_PROCESSOR_PM_CX	1
> +#define XEN_PROCESSOR_PM_PX	2
> +#define XEN_PROCESSOR_PM_TX	4
> +
> +/* cmd type */
> +#define XEN_PM_CX   0
> +#define XEN_PM_PX   1
> +#define XEN_PM_TX   2
> +
> +/* Px sub info type */
> +#define XEN_PX_PCT   1
> +#define XEN_PX_PSS   2
> +#define XEN_PX_PPC   4
> +#define XEN_PX_PSD   8
> +
> +struct xen_power_register {
> +	uint32_t     space_id;
> +	uint32_t     bit_width;
> +	uint32_t     bit_offset;
> +	uint32_t     access_size;
> +	uint64_t     address;
> +};
> +
> +struct xen_processor_csd {
> +	uint32_t    domain;      /* domain number of one dependent group */
> +	uint32_t    coord_type;  /* coordination type */
> +	uint32_t    num;         /* number of processors in same domain */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_csd);
> +
> +struct xen_processor_cx {
> +	struct xen_power_register  reg; /* GAS for Cx trigger register */
> +	uint8_t     type;     /* cstate value, c0: 0, c1: 1, ... */
> +	uint32_t    latency;  /* worst latency (ms) to enter/exit this cstate */
> +	uint32_t    power;    /* average power consumption(mW) */
> +	uint32_t    dpcnt;    /* number of dependency entries */
> +	GUEST_HANDLE(xen_processor_csd) dp; /* NULL if no dependency */ };
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_cx);
> +
> +struct xen_processor_flags {
> +	uint32_t bm_control:1;
> +	uint32_t bm_check:1;
> +	uint32_t has_cst:1;
> +	uint32_t power_setup_done:1;
> +	uint32_t bm_rld_set:1;
> +};
> +
> +struct xen_processor_power {
> +	uint32_t count;  /* number of C state entries in array below */
> +	struct xen_processor_flags flags;  /* global flags of this processor */
> +	GUEST_HANDLE(xen_processor_cx) states; /* supported c states */ };
> +
> +struct xen_pct_register {
> +	uint8_t  descriptor;
> +	uint16_t length;
> +	uint8_t  space_id;
> +	uint8_t  bit_width;
> +	uint8_t  bit_offset;
> +	uint8_t  reserved;
> +	uint64_t address;
> +};
> +
> +struct xen_processor_px {
> +	uint64_t core_frequency; /* megahertz */
> +	uint64_t power;      /* milliWatts */
> +	uint64_t transition_latency; /* microseconds */
> +	uint64_t bus_master_latency; /* microseconds */
> +	uint64_t control;        /* control value */
> +	uint64_t status;     /* success indicator */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_px);
> +
> +struct xen_psd_package {
> +	uint64_t num_entries;
> +	uint64_t revision;
> +	uint64_t domain;
> +	uint64_t coord_type;
> +	uint64_t num_processors;
> +};
> +
> +struct xen_processor_performance {
> +	uint32_t flags;     /* flag for Px sub info type */
> +	uint32_t platform_limit;  /* Platform limitation on freq usage */
> +	struct xen_pct_register control_register;
> +	struct xen_pct_register status_register;
> +	uint32_t state_count;     /* total available performance states */
> +	GUEST_HANDLE(xen_processor_px) states;
> +	struct xen_psd_package domain_info;
> +	uint32_t shared_type;     /* coordination type of this processor */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_performance);
> +
> +struct xenpf_set_processor_pminfo {
> +	/* IN variables */
> +	uint32_t id;    /* ACPI CPU ID */
> +	uint32_t type;  /* {XEN_PM_CX, XEN_PM_PX} */
> +	union {
> +		struct xen_processor_power          power;/* Cx: _CST/_CSD */
> +		struct xen_processor_performance    perf; /* Px: _PPC/_PCT/_PSS/_PSD */
> +	};
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_set_processor_pminfo);
> +
> +struct xen_platform_op {
> +	uint32_t cmd;
> +	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> +	union {
> +		struct xenpf_settime           settime;
> +		struct xenpf_add_memtype       add_memtype;
> +		struct xenpf_del_memtype       del_memtype;
> +		struct xenpf_read_memtype      read_memtype;
> +		struct xenpf_microcode_update  microcode;
> +		struct xenpf_platform_quirk    platform_quirk;
> +		struct xenpf_firmware_info     firmware_info;
> +		struct xenpf_enter_acpi_sleep  enter_acpi_sleep;
> +		struct xenpf_change_freq       change_freq;
> +		struct xenpf_getidletime       getidletime;
> +		struct xenpf_set_processor_pminfo set_pminfo;
> +		uint8_t                        pad[128];
> +	} u;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_platform_op_t);
> +
> +#endif /* __XEN_PUBLIC_PLATFORM_H__ */
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h index 70213b4..d83cc08
> 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -453,6 +453,7 @@ struct start_info {
>  /* These flags are passed in the 'flags' field of start_info_t. */
>  #define SIF_PRIVILEGED    (1<<0)  /* Is the domain privileged? */
>  #define SIF_INITDOMAIN    (1<<1)  /* Is this the initial control domain? */
> +#define SIF_PM_MASK       (0xFF<<8) /* reserve 1 byte for xen-pm options */
> 
>  typedef uint64_t cpumap_t;
> 
> --
> 1.7.4.1

Re: [tboot-devel] [PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

[Cihula, J. <jos...@in...>]

> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> Provide the registration callback to call in the Xen's ACPI sleep functionality. This means that
> during S3/S5 we make a hypercall XENPF_enter_acpi_sleep with the proper PM1A/PM1B registers.
> 
> Based of Ke Yu's <ke...@in...> initial idea.
> [ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
> change c68699484a65 ]
> 
> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> ---
>  arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
>  arch/x86/xen/enlighten.c             |    3 +++
>  drivers/xen/Makefile                 |    2 +-
>  drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
>  include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
>  5 files changed, 63 insertions(+), 1 deletions(-)  create mode 100644 drivers/xen/acpi.c  create
> mode 100644 include/xen/acpi.h
> 
> diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
> index d240ea9..0c9894e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
> @@ -45,6 +45,7 @@
>  #include <xen/interface/xen.h>
>  #include <xen/interface/sched.h>
>  #include <xen/interface/physdev.h>
> +#include <xen/interface/platform.h>
> 
>  /*
>   * The hypercall asms have to meet several constraints:
> @@ -299,6 +300,13 @@ HYPERVISOR_set_timer_op(u64 timeout)  }
> 
>  static inline int
> +HYPERVISOR_dom0_op(struct xen_platform_op *platform_op) {
> +	platform_op->interface_version = XENPF_INTERFACE_VERSION;
> +	return _hypercall1(int, dom0_op, platform_op); }
> +
> +static inline int
>  HYPERVISOR_set_debugreg(int reg, unsigned long value)  {
>  	return _hypercall2(int, set_debugreg, reg, value); diff --git a/arch/x86/xen/enlighten.c
> b/arch/x86/xen/enlighten.c index 5525163..6962653 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -42,6 +42,7 @@
>  #include <xen/page.h>
>  #include <xen/hvm.h>
>  #include <xen/hvc-console.h>
> +#include <xen/acpi.h>
> 
>  #include <asm/paravirt.h>
>  #include <asm/apic.h>
> @@ -1250,6 +1251,8 @@ asmlinkage void __init xen_start_kernel(void)
>  	} else {
>  		/* Make sure ACS will be enabled */
>  		pci_request_acs();
> +
> +		xen_acpi_sleep_register();
>  	}
> 
> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index bbc1825..370552d 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -16,7 +16,7 @@ obj-$(CONFIG_XENFS)			+= xenfs/
>  obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
>  obj-$(CONFIG_XEN_PLATFORM_PCI)		+= xen-platform-pci.o
>  obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
> -obj-$(CONFIG_XEN_DOM0)			+= pci.o
> +obj-$(CONFIG_XEN_DOM0)			+= pci.o acpi.o
> 
>  xen-evtchn-y				:= evtchn.o
>  xen-gntdev-y				:= gntdev.o
> diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c new file mode 100644 index 0000000..c0f829f
> --- /dev/null
> +++ b/drivers/xen/acpi.c
> @@ -0,0 +1,25 @@

Copyright and license?

> +#include <xen/acpi.h>
> +#include <xen/interface/platform.h>
> +#include <asm/xen/hypercall.h>
> +#include <asm/xen/hypervisor.h>
> +
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> +				     u32 pm1a_cnt, u32 pm1b_cnt,
> +				     bool *skip_rest)
> +{
> +	struct xen_platform_op op = {
> +		.cmd = XENPF_enter_acpi_sleep,
> +		.interface_version = XENPF_INTERFACE_VERSION,
> +		.u = {
> +			.enter_acpi_sleep = {
> +				.pm1a_cnt_val = (u16)pm1a_cnt,
> +				.pm1b_cnt_val = (u16)pm1b_cnt,
> +				.sleep_state = sleep_state,
> +			},
> +		},
> +	};
> +	if (skip_rest)
> +		*skip_rest = true;
> +
> +	return HYPERVISOR_dom0_op(&op);
> +}
> diff --git a/include/xen/acpi.h b/include/xen/acpi.h new file mode 100644 index 0000000..e414f14
> --- /dev/null
> +++ b/include/xen/acpi.h
> @@ -0,0 +1,26 @@

Copyright and license?

> +#ifndef _XEN_ACPI_H
> +#define _XEN_ACPI_H
> +
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_XEN_DOM0
> +#include <asm/xen/hypervisor.h>
> +#include <xen/xen.h>
> +#include <linux/acpi.h>
> +
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> +				     u32 pm1a_cnt, u32 pm1b_cnd,
> +				     bool *skip_rest);
> +
> +static inline void xen_acpi_sleep_register(void) {
> +	if (xen_initial_domain())
> +		__acpi_override_sleep = xen_acpi_notify_hypervisor_state; } #else
> +static inline void xen_acpi_sleep_register(void) { } #endif
> +
> +#endif	/* _XEN_ACPI_H */
> --
> 1.7.4.1

Re: [tboot-devel] [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

[Cihula, J. <jos...@in...>]

> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> The ACPI suspend path makes a call to tboot_sleep right before it writes the PM1A, PM1B values. We
> replace the direct call to tboot via an registration callback similar to __acpi_register_gsi.
> 
> CC: Thomas Gleixner <tg...@li...>
> CC: "H. Peter Anvin" <hp...@zy...>
> CC: x8...@ke...
> CC: Len Brown <len...@in...>
> CC: Joseph Cihula <jos...@in...>
> CC: Shane Wang <sha...@in...>
> CC: xen...@li...
> CC: lin...@li...
> CC: tbo...@li...
> CC: lin...@vg...
> Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
> ---
>  arch/x86/include/asm/acpi.h   |    3 +++
>  arch/x86/kernel/acpi/boot.c   |    3 +++
>  arch/x86/kernel/tboot.c       |   13 +++++++++----
>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
>  include/linux/tboot.h         |    3 ++-
>  5 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16);  extern int
> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>  				  int trigger, int polarity);
> 
> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> +				    u32 pm1b_ctrl, bool *skip_rest);
> +
>  static inline void disable_acpi(void)
>  {
>  	acpi_disabled = 1;
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
> 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,  int
> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>  			   int trigger, int polarity) = acpi_register_gsi_pic;
> 
> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> +			     u32 pm1b_ctrl, bool *skip_rest) = NULL;
> +
>  /*
>   * success: return IRQ number (>=0)
>   * failure: return < 0
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 30ac65d..a18070c 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -41,7 +41,7 @@
>  #include <asm/setup.h>
>  #include <asm/e820.h>
>  #include <asm/io.h>
> -
> +#include <linux/acpi.h>
>  #include "acpi/realmode/wakeup.h"
> 
>  /* Global pointer to shared data; NULL means no measured launch. */ @@ -270,7 +270,8 @@ static
> void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
> 
> -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		 bool *skip_rest)

Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?

>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -279,7 +280,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  		/* S5: */ TB_SHUTDOWN_S5 };
> 
>  	if (!tboot_enabled())
> -		return;
> +		return AE_OK;
> 
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -290,10 +291,12 @@ void tboot_sleep(u8
> sleep_state, u32 pm1a_control, u32 pm1b_control)
>  	if (sleep_state >= ACPI_S_STATE_COUNT ||
>  	    acpi_shutdown_map[sleep_state] == -1) {
>  		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> -		return;
> +		return AE_ERROR;
>  	}
> 
>  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> +
> +	return AE_OK;
>  }
> 
>  static atomic_t ap_wfs_count;
> @@ -343,6 +346,8 @@ static __init int tboot_late_init(void)
> 
>  	atomic_set(&ap_wfs_count, 0);
>  	register_hotcpu_notifier(&tboot_cpu_notifier);
> +
> +	__acpi_override_sleep = tboot_sleep;
>  	return 0;
>  }
> 
> diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c index 2ac28bb..31d1198
> 100644
> --- a/drivers/acpi/acpica/hwsleep.c
> +++ b/drivers/acpi/acpica/hwsleep.c
> @@ -45,7 +45,6 @@
>  #include <acpi/acpi.h>
>  #include "accommon.h"
>  #include "actables.h"
> -#include <linux/tboot.h>
> 
>  #define _COMPONENT          ACPI_HARDWARE
>  ACPI_MODULE_NAME("hwsleep")
> @@ -343,8 +342,17 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state)
> 
>  	ACPI_FLUSH_CPU_CACHE();
> 
> -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> +	if (__acpi_override_sleep) {
> +		bool skip_rest = false;
> 
> +		status = __acpi_override_sleep(sleep_state, pm1a_control,
> +					       pm1b_control, &skip_rest);
> +
> +		if (ACPI_FAILURE(status))
> +			return_ACPI_STATUS(status);
> +		if (skip_rest)
> +			return_ACPI_STATUS(AE_OK);
> +	}
>  	/* Write #2: Write both SLP_TYP + SLP_EN */
> 
>  	status = acpi_hw_write_pm1_control(pm1a_control, pm1b_control); diff --git
> a/include/linux/tboot.h b/include/linux/tboot.h index 1dba6ee..19badbd 100644
> --- a/include/linux/tboot.h
> +++ b/include/linux/tboot.h
> @@ -143,7 +143,8 @@ static inline int tboot_enabled(void)
> 
>  extern void tboot_probe(void);
>  extern void tboot_shutdown(u32 shutdown_type); -extern void tboot_sleep(u8 sleep_state, u32
> pm1a_control, u32 pm1b_control);
> +extern int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		       bool *skip);
>  extern struct acpi_table_header *tboot_get_dmar_table(
>  				      struct acpi_table_header *dmar_tbl);  extern int
> tboot_force_iommu(void);
> --
> 1.7.4.1

Re: [tboot-devel] [Xen-devel] [RFC PATCH v1] ACPI S3 to work under Xen.

[Stefano S. <ste...@eu...>]

On Wed, 31 Aug 2011, Konrad Rzeszutek Wilk wrote:
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. The Linux
> kernel does the ACPI "stuff" and tells the hypervisor to do the low-level
> stuff (such as program the IOAPIC, setup vectors, etc). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> The major difficulties we hit was with 'acpi_suspend_lowlevel' - which tweaks
> a lot of lowlevel values and some of them are not properly handled by Xen.
> Liang Tang has figured which ones of them we trip over (read below) - and he
> suggested that perhaps we can provide a registration mechanism to abstract
> this away.
> 
> So the attached patches do exactly that - there are two entry points
> in the ACPI.
> 
>  1). For S3: acpi_suspend_lowlevel -> .. lots of code -> acpi_enter_sleep_state
>  2). For S1/S4/S5: acpi_enter_sleep_state
> 
> The first naive idea was of abstracting away in the 'acpi_enter_sleep_state'
> function the tboot_sleep code so that we can use it too. And low-behold - it
> worked splendidly for powering off (S5 I believe)
> 
> For S3 that did not work - during suspend the hypervisor tripped over when
> saving cr8. During resume it tripped over at restoring the cr3, cr8, idt,
> and gdt values.
> 
> What do you guys think? One thought is to use the paravirt interface to
> deal with cr3, cr8, idt, gdt for suspend/resume case.. But that is a lot
> of extra 'if' in the paravirt code  - which the callback registration would
> effectively do the same thing as the paravirt - except at a higher level.
> 
> Thoughts?
 

I think there are no doubts about the fact that this approach produces
cleaner code than using the pvop interface for dealing with cr3, cr8,
etc.
Also, having seen the original xen acpi implementation in 2.6.18, I want
to add that you did a _very_ good job with this series.

Re: [tboot-devel] [RFC PATCH v1] ACPI S3 to work under Xen.

[Tian, K. <kev...@in...>]

> From: Konrad Rzeszutek Wilk [mailto:kon...@or...]
> Sent: Thursday, September 01, 2011 2:31 AM
> 
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. The Linux
> kernel does the ACPI "stuff" and tells the hypervisor to do the low-level
> stuff (such as program the IOAPIC, setup vectors, etc). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> The major difficulties we hit was with 'acpi_suspend_lowlevel' - which tweaks
> a lot of lowlevel values and some of them are not properly handled by Xen.
> Liang Tang has figured which ones of them we trip over (read below) - and he
> suggested that perhaps we can provide a registration mechanism to abstract
> this away.

that's not a problem for Xen. On bare metal the processor state handled off
from BIOS after resume (e.g. real mode in many cases) doesn't match the
execution environment expected by Linux. So Linux has to save original context
and then restore it after resume.

However for Xen, Linux doesn't talk to BIOS directly. Instead it simply walks
through necessary ACPI methods, and then issues a hypercall to Xen which
then further completes the remaining suspend steps. So here ACPI S3 is
exactly same as any other hypercall path, where processor context will be
saved/restored by Xen automatically.

> 
> So the attached patches do exactly that - there are two entry points
> in the ACPI.
> 
>  1). For S3: acpi_suspend_lowlevel -> .. lots of code -> acpi_enter_sleep_state
>  2). For S1/S4/S5: acpi_enter_sleep_state
> 
> The first naive idea was of abstracting away in the 'acpi_enter_sleep_state'
> function the tboot_sleep code so that we can use it too. And low-behold - it
> worked splendidly for powering off (S5 I believe)
> 
> For S3 that did not work - during suspend the hypervisor tripped over when
> saving cr8. During resume it tripped over at restoring the cr3, cr8, idt,
> and gdt values.
> 
> What do you guys think? One thought is to use the paravirt interface to
> deal with cr3, cr8, idt, gdt for suspend/resume case.. But that is a lot
> of extra 'if' in the paravirt code  - which the callback registration would
> effectively do the same thing as the paravirt - except at a higher level.
> 

So tweaking at a higher level is a right thing to do.

Thanks
Kevin

[tboot-devel] [PATCH 4/7] xen: Utilize the restore_msi_irqs hook.

[Konrad R. W. <kon...@or...>]

to make a hypercall to restore the vectors in the MSI/MSI-X
configuration space.

Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
---
 arch/x86/pci/xen.c              |   12 ++++++++++++
 include/xen/interface/physdev.h |    7 +++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index f567965..f140999 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -241,6 +241,17 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 out:
 	return ret;
 }
+
+static void xen_initdom_restore_msi_irqs(struct pci_dev *dev, int irq)
+{
+	int ret = 0;
+	struct physdev_restore_msi restore;
+
+	restore.bus = dev->bus->number;
+	restore.devfn = dev->devfn;
+	ret = HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi, &restore);
+	WARN(ret && ret != -ENOSYS, "restore_msi -> %d\n", ret);
+}
 #endif
 #endif
 
@@ -458,6 +469,7 @@ static int __init pci_xen_initial_domain(void)
 #ifdef CONFIG_PCI_MSI
 	x86_msi.setup_msi_irqs = xen_initdom_setup_msi_irqs;
 	x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
+	x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
 #endif
 	xen_setup_acpi_sci();
 	__acpi_register_gsi = acpi_register_gsi_xen;
diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
index 534cac8..44aefa9 100644
--- a/include/xen/interface/physdev.h
+++ b/include/xen/interface/physdev.h
@@ -144,6 +144,13 @@ struct physdev_manage_pci {
 	uint8_t devfn;
 };
 
+#define PHYSDEVOP_restore_msi            19
+struct physdev_restore_msi {
+	/* IN */
+	uint8_t bus;
+	uint8_t devfn;
+};
+
 #define PHYSDEVOP_manage_pci_add_ext	20
 struct physdev_manage_pci_ext {
 	/* IN */
-- 
1.7.4.1

[tboot-devel] [PATCH 7/7] xen/acpi/sleep: Register to the acpi_suspend_lowlevel a callback.

[Konrad R. W. <kon...@or...>]

We piggyback on "x86/acpi: Provide registration for acpi_suspend_lowlevel."
to register a Xen version of the callback. The callback does not
do anything special - except it omits the x86_acpi_suspend_lowlevel.
It does that b/c during suspend it tries to save cr8 values (which
the hypervisor does not support), and then on resume path the
cr3, cr8, idt, and gdt are all resumed which clashes with what
the hypervisor has set up for the guest.

Signed-off-by: Liang Tang <lia...@or...>
Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
---
 include/xen/acpi.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/include/xen/acpi.h b/include/xen/acpi.h
index e414f14..0409919 100644
--- a/include/xen/acpi.h
+++ b/include/xen/acpi.h
@@ -12,10 +12,22 @@ int xen_acpi_notify_hypervisor_state(u8 sleep_state,
 				     u32 pm1a_cnt, u32 pm1b_cnd,
 				     bool *skip_rest);
 
+static inline int xen_acpi_suspend_lowlevel(void)
+{
+	/*
+	 * Xen will save and restore CPU context, so
+	 * we can skip that and just go straight to
+	 * the suspend.
+	 */
+	acpi_enter_sleep_state(ACPI_STATE_S3);
+	return 0;
+}
 static inline void xen_acpi_sleep_register(void)
 {
-	if (xen_initial_domain())
+	if (xen_initial_domain()) {
+		acpi_suspend_lowlevel = xen_acpi_suspend_lowlevel;
 		__acpi_override_sleep = xen_acpi_notify_hypervisor_state;
+	}
 }
 #else
 static inline void xen_acpi_sleep_register(void)
-- 
1.7.4.1

[tboot-devel] [PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

[Konrad R. W. <kon...@or...>]

Provide the registration callback to call in the Xen's
ACPI sleep functionality. This means that during S3/S5
we make a hypercall XENPF_enter_acpi_sleep with the
proper PM1A/PM1B registers.

Based of Ke Yu's <ke...@in...> initial idea.
[ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
change c68699484a65 ]

Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
---
 arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
 arch/x86/xen/enlighten.c             |    3 +++
 drivers/xen/Makefile                 |    2 +-
 drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
 include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/acpi.c
 create mode 100644 include/xen/acpi.h

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index d240ea9..0c9894e 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -45,6 +45,7 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/sched.h>
 #include <xen/interface/physdev.h>
+#include <xen/interface/platform.h>
 
 /*
  * The hypercall asms have to meet several constraints:
@@ -299,6 +300,13 @@ HYPERVISOR_set_timer_op(u64 timeout)
 }
 
 static inline int
+HYPERVISOR_dom0_op(struct xen_platform_op *platform_op)
+{
+	platform_op->interface_version = XENPF_INTERFACE_VERSION;
+	return _hypercall1(int, dom0_op, platform_op);
+}
+
+static inline int
 HYPERVISOR_set_debugreg(int reg, unsigned long value)
 {
 	return _hypercall2(int, set_debugreg, reg, value);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5525163..6962653 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -42,6 +42,7 @@
 #include <xen/page.h>
 #include <xen/hvm.h>
 #include <xen/hvc-console.h>
+#include <xen/acpi.h>
 
 #include <asm/paravirt.h>
 #include <asm/apic.h>
@@ -1250,6 +1251,8 @@ asmlinkage void __init xen_start_kernel(void)
 	} else {
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
+
+		xen_acpi_sleep_register();
 	}
 		
 
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index bbc1825..370552d 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_XENFS)			+= xenfs/
 obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
 obj-$(CONFIG_XEN_PLATFORM_PCI)		+= xen-platform-pci.o
 obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
-obj-$(CONFIG_XEN_DOM0)			+= pci.o
+obj-$(CONFIG_XEN_DOM0)			+= pci.o acpi.o
 
 xen-evtchn-y				:= evtchn.o
 xen-gntdev-y				:= gntdev.o
diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c
new file mode 100644
index 0000000..c0f829f
--- /dev/null
+++ b/drivers/xen/acpi.c
@@ -0,0 +1,25 @@
+#include <xen/acpi.h>
+#include <xen/interface/platform.h>
+#include <asm/xen/hypercall.h>
+#include <asm/xen/hypervisor.h>
+
+int xen_acpi_notify_hypervisor_state(u8 sleep_state,
+				     u32 pm1a_cnt, u32 pm1b_cnt,
+				     bool *skip_rest)
+{
+	struct xen_platform_op op = {
+		.cmd = XENPF_enter_acpi_sleep,
+		.interface_version = XENPF_INTERFACE_VERSION,
+		.u = {
+			.enter_acpi_sleep = {
+				.pm1a_cnt_val = (u16)pm1a_cnt,
+				.pm1b_cnt_val = (u16)pm1b_cnt,
+				.sleep_state = sleep_state,
+			},
+		},
+	};
+	if (skip_rest)
+		*skip_rest = true;
+
+	return HYPERVISOR_dom0_op(&op);
+}
diff --git a/include/xen/acpi.h b/include/xen/acpi.h
new file mode 100644
index 0000000..e414f14
--- /dev/null
+++ b/include/xen/acpi.h
@@ -0,0 +1,26 @@
+#ifndef _XEN_ACPI_H
+#define _XEN_ACPI_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_XEN_DOM0
+#include <asm/xen/hypervisor.h>
+#include <xen/xen.h>
+#include <linux/acpi.h>
+
+int xen_acpi_notify_hypervisor_state(u8 sleep_state,
+				     u32 pm1a_cnt, u32 pm1b_cnd,
+				     bool *skip_rest);
+
+static inline void xen_acpi_sleep_register(void)
+{
+	if (xen_initial_domain())
+		__acpi_override_sleep = xen_acpi_notify_hypervisor_state;
+}
+#else
+static inline void xen_acpi_sleep_register(void)
+{
+}
+#endif
+
+#endif	/* _XEN_ACPI_H */
-- 
1.7.4.1

[tboot-devel] [RFC PATCH v1] ACPI S3 to work under Xen.

[Konrad R. W. <kon...@or...>]

Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.

The relationship that Xen has with Linux kernel is symbiotic. The Linux
kernel does the ACPI "stuff" and tells the hypervisor to do the low-level
stuff (such as program the IOAPIC, setup vectors, etc). The realm of
ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
values - which the hypervisor has to do) and then restore them.

The major difficulties we hit was with 'acpi_suspend_lowlevel' - which tweaks
a lot of lowlevel values and some of them are not properly handled by Xen.
Liang Tang has figured which ones of them we trip over (read below) - and he
suggested that perhaps we can provide a registration mechanism to abstract
this away.

So the attached patches do exactly that - there are two entry points
in the ACPI.

 1). For S3: acpi_suspend_lowlevel -> .. lots of code -> acpi_enter_sleep_state
 2). For S1/S4/S5: acpi_enter_sleep_state

The first naive idea was of abstracting away in the 'acpi_enter_sleep_state'
function the tboot_sleep code so that we can use it too. And low-behold - it
worked splendidly for powering off (S5 I believe)

For S3 that did not work - during suspend the hypervisor tripped over when
saving cr8. During resume it tripped over at restoring the cr3, cr8, idt,
and gdt values.

What do you guys think? One thought is to use the paravirt interface to
deal with cr3, cr8, idt, gdt for suspend/resume case.. But that is a lot
of extra 'if' in the paravirt code  - which the callback registration would
effectively do the same thing as the paravirt - except at a higher level.

Thoughts?


Konrad Rzeszutek Wilk (5):
      x86: Expand the x86_msi_ops to have a restore MSIs.
      x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.
      xen: Utilize the restore_msi_irqs hook.
      xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep
      xen/acpi/sleep: Register to the acpi_suspend_lowlevel a callback.

Liang Tang (1):
      x86/acpi/sleep: Provide registration for acpi_suspend_lowlevel.

Yu Ke (1):
      xen/acpi: Domain0 acpi parser related platform hypercall

 arch/ia64/include/asm/xen/interface.h |    1 +
 arch/x86/include/asm/acpi.h           |    5 +-
 arch/x86/include/asm/pci.h            |    9 +
 arch/x86/include/asm/x86_init.h       |    1 +
 arch/x86/include/asm/xen/hypercall.h  |    8 +
 arch/x86/include/asm/xen/interface.h  |    1 +
 arch/x86/kernel/acpi/boot.c           |    5 +
 arch/x86/kernel/acpi/sleep.c          |    4 +-
 arch/x86/kernel/acpi/sleep.h          |    2 +
 arch/x86/kernel/tboot.c               |   13 +-
 arch/x86/kernel/x86_init.c            |    1 +
 arch/x86/pci/xen.c                    |   12 ++
 arch/x86/xen/enlighten.c              |    3 +
 drivers/acpi/acpica/hwsleep.c         |   12 +-
 drivers/acpi/sleep.c                  |    2 +
 drivers/pci/msi.c                     |   29 +++-
 drivers/xen/Makefile                  |    2 +-
 drivers/xen/acpi.c                    |   25 +++
 include/linux/tboot.h                 |    3 +-
 include/xen/acpi.h                    |   38 ++++
 include/xen/interface/physdev.h       |    7 +
 include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h           |    1 +
 23 files changed, 491 insertions(+), 13 deletions(-)

[tboot-devel] [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

[Konrad R. W. <kon...@or...>]

From: Yu Ke <ke...@in...>

This patches implements the xen_platform_op hypercall, to pass the parsed
ACPI info to hypervisor.

Signed-off-by: Yu Ke <ke...@in...>
Signed-off-by: Tian Kevin <kev...@in...>
Signed-off-by: Jeremy Fitzhardinge <jer...@ci...>
[v1: Added DEFINE_GUEST.. in appropiate headers]
[v2: Ripped out typedefs]
Signed-off-by: Konrad Rzeszutek Wilk <kon...@or...>
---
 arch/ia64/include/asm/xen/interface.h |    1 +
 arch/x86/include/asm/xen/interface.h  |    1 +
 include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h           |    1 +
 4 files changed, 323 insertions(+), 0 deletions(-)
 create mode 100644 include/xen/interface/platform.h

diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
index e951e74..1d2427d 100644
--- a/arch/ia64/include/asm/xen/interface.h
+++ b/arch/ia64/include/asm/xen/interface.h
@@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
 DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
+DEFINE_GUEST_HANDLE(uint64_t);
 
 typedef unsigned long xen_pfn_t;
 DEFINE_GUEST_HANDLE(xen_pfn_t);
diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
index 5d4922a..a1f2db5 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
 DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
+DEFINE_GUEST_HANDLE(uint64_t);
 #endif
 
 #ifndef HYPERVISOR_VIRT_START
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
new file mode 100644
index 0000000..c168468
--- /dev/null
+++ b/include/xen/interface/platform.h
@@ -0,0 +1,320 @@
+/******************************************************************************
+ * platform.h
+ *
+ * Hardware platform operations. Intended for use by domain-0 kernel.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2002-2006, K Fraser
+ */
+
+#ifndef __XEN_PUBLIC_PLATFORM_H__
+#define __XEN_PUBLIC_PLATFORM_H__
+
+#include "xen.h"
+
+#define XENPF_INTERFACE_VERSION 0x03000001
+
+/*
+ * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
+ * 1 January, 1970 if the current system time was <system_time>.
+ */
+#define XENPF_settime             17
+struct xenpf_settime {
+	/* IN variables. */
+	uint32_t secs;
+	uint32_t nsecs;
+	uint64_t system_time;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
+
+/*
+ * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
+ * On x86, @type is an architecture-defined MTRR memory type.
+ * On success, returns the MTRR that was used (@reg) and a handle that can
+ * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
+ * (x86-specific).
+ */
+#define XENPF_add_memtype         31
+struct xenpf_add_memtype {
+	/* IN variables. */
+	unsigned long mfn;
+	uint64_t nr_mfns;
+	uint32_t type;
+	/* OUT variables. */
+	uint32_t handle;
+	uint32_t reg;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
+
+/*
+ * Tear down an existing memory-range type. If @handle is remembered then it
+ * should be passed in to accurately tear down the correct setting (in case
+ * of overlapping memory regions with differing types). If it is not known
+ * then @handle should be set to zero. In all cases @reg must be set.
+ * (x86-specific).
+ */
+#define XENPF_del_memtype         32
+struct xenpf_del_memtype {
+	/* IN variables. */
+	uint32_t handle;
+	uint32_t reg;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
+
+/* Read current type of an MTRR (x86-specific). */
+#define XENPF_read_memtype        33
+struct xenpf_read_memtype {
+	/* IN variables. */
+	uint32_t reg;
+	/* OUT variables. */
+	unsigned long mfn;
+	uint64_t nr_mfns;
+	uint32_t type;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
+
+#define XENPF_microcode_update    35
+struct xenpf_microcode_update {
+	/* IN variables. */
+	GUEST_HANDLE(void) data;          /* Pointer to microcode data */
+	uint32_t length;                  /* Length of microcode data. */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
+
+#define XENPF_platform_quirk      39
+#define QUIRK_NOIRQBALANCING      1 /* Do not restrict IO-APIC RTE targets */
+#define QUIRK_IOAPIC_BAD_REGSEL   2 /* IO-APIC REGSEL forgets its value    */
+#define QUIRK_IOAPIC_GOOD_REGSEL  3 /* IO-APIC REGSEL behaves properly     */
+struct xenpf_platform_quirk {
+	/* IN variables. */
+	uint32_t quirk_id;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
+
+#define XENPF_firmware_info       50
+#define XEN_FW_DISK_INFO          1 /* from int 13 AH=08/41/48 */
+#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
+#define XEN_FW_VBEDDC_INFO        3 /* from int 10 AX=4f15 */
+struct xenpf_firmware_info {
+	/* IN variables. */
+	uint32_t type;
+	uint32_t index;
+	/* OUT variables. */
+	union {
+		struct {
+			/* Int13, Fn48: Check Extensions Present. */
+			uint8_t device;                   /* %dl: bios device number */
+			uint8_t version;                  /* %ah: major version      */
+			uint16_t interface_support;       /* %cx: support bitmap     */
+			/* Int13, Fn08: Legacy Get Device Parameters. */
+			uint16_t legacy_max_cylinder;     /* %cl[7:6]:%ch: max cyl # */
+			uint8_t legacy_max_head;          /* %dh: max head #         */
+			uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector #  */
+			/* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
+			/* NB. First uint16_t of buffer must be set to buffer size.      */
+			GUEST_HANDLE(void) edd_params;
+		} disk_info; /* XEN_FW_DISK_INFO */
+		struct {
+			uint8_t device;                   /* bios device number  */
+			uint32_t mbr_signature;           /* offset 0x1b8 in mbr */
+		} disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
+		struct {
+			/* Int10, AX=4F15: Get EDID info. */
+			uint8_t capabilities;
+			uint8_t edid_transfer_time;
+			/* must refer to 128-byte buffer */
+			GUEST_HANDLE(uchar) edid;
+		} vbeddc_info; /* XEN_FW_VBEDDC_INFO */
+	} u;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
+
+#define XENPF_enter_acpi_sleep    51
+struct xenpf_enter_acpi_sleep {
+	/* IN variables */
+	uint16_t pm1a_cnt_val;      /* PM1a control value. */
+	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint32_t sleep_state;       /* Which state to enter (Sn). */
+	uint32_t flags;             /* Must be zero. */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
+
+#define XENPF_change_freq         52
+struct xenpf_change_freq {
+	/* IN variables */
+	uint32_t flags; /* Must be zero. */
+	uint32_t cpu;   /* Physical cpu. */
+	uint64_t freq;  /* New frequency (Hz). */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_change_freq_t);
+
+/*
+ * Get idle times (nanoseconds since boot) for physical CPUs specified in the
+ * @cpumap_bitmap with range [0..@cpumap_nr_cpus-1]. The @idletime array is
+ * indexed by CPU number; only entries with the corresponding @cpumap_bitmap
+ * bit set are written to. On return, @cpumap_bitmap is modified so that any
+ * non-existent CPUs are cleared. Such CPUs have their @idletime array entry
+ * cleared.
+ */
+#define XENPF_getidletime         53
+struct xenpf_getidletime {
+	/* IN/OUT variables */
+	/* IN: CPUs to interrogate; OUT: subset of IN which are present */
+	GUEST_HANDLE(uchar) cpumap_bitmap;
+	/* IN variables */
+	/* Size of cpumap bitmap. */
+	uint32_t cpumap_nr_cpus;
+	/* Must be indexable for every cpu in cpumap_bitmap. */
+	GUEST_HANDLE(uint64_t) idletime;
+	/* OUT variables */
+	/* System time when the idletime snapshots were taken. */
+	uint64_t now;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_getidletime_t);
+
+#define XENPF_set_processor_pminfo      54
+
+/* ability bits */
+#define XEN_PROCESSOR_PM_CX	1
+#define XEN_PROCESSOR_PM_PX	2
+#define XEN_PROCESSOR_PM_TX	4
+
+/* cmd type */
+#define XEN_PM_CX   0
+#define XEN_PM_PX   1
+#define XEN_PM_TX   2
+
+/* Px sub info type */
+#define XEN_PX_PCT   1
+#define XEN_PX_PSS   2
+#define XEN_PX_PPC   4
+#define XEN_PX_PSD   8
+
+struct xen_power_register {
+	uint32_t     space_id;
+	uint32_t     bit_width;
+	uint32_t     bit_offset;
+	uint32_t     access_size;
+	uint64_t     address;
+};
+
+struct xen_processor_csd {
+	uint32_t    domain;      /* domain number of one dependent group */
+	uint32_t    coord_type;  /* coordination type */
+	uint32_t    num;         /* number of processors in same domain */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_processor_csd);
+
+struct xen_processor_cx {
+	struct xen_power_register  reg; /* GAS for Cx trigger register */
+	uint8_t     type;     /* cstate value, c0: 0, c1: 1, ... */
+	uint32_t    latency;  /* worst latency (ms) to enter/exit this cstate */
+	uint32_t    power;    /* average power consumption(mW) */
+	uint32_t    dpcnt;    /* number of dependency entries */
+	GUEST_HANDLE(xen_processor_csd) dp; /* NULL if no dependency */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_processor_cx);
+
+struct xen_processor_flags {
+	uint32_t bm_control:1;
+	uint32_t bm_check:1;
+	uint32_t has_cst:1;
+	uint32_t power_setup_done:1;
+	uint32_t bm_rld_set:1;
+};
+
+struct xen_processor_power {
+	uint32_t count;  /* number of C state entries in array below */
+	struct xen_processor_flags flags;  /* global flags of this processor */
+	GUEST_HANDLE(xen_processor_cx) states; /* supported c states */
+};
+
+struct xen_pct_register {
+	uint8_t  descriptor;
+	uint16_t length;
+	uint8_t  space_id;
+	uint8_t  bit_width;
+	uint8_t  bit_offset;
+	uint8_t  reserved;
+	uint64_t address;
+};
+
+struct xen_processor_px {
+	uint64_t core_frequency; /* megahertz */
+	uint64_t power;      /* milliWatts */
+	uint64_t transition_latency; /* microseconds */
+	uint64_t bus_master_latency; /* microseconds */
+	uint64_t control;        /* control value */
+	uint64_t status;     /* success indicator */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_processor_px);
+
+struct xen_psd_package {
+	uint64_t num_entries;
+	uint64_t revision;
+	uint64_t domain;
+	uint64_t coord_type;
+	uint64_t num_processors;
+};
+
+struct xen_processor_performance {
+	uint32_t flags;     /* flag for Px sub info type */
+	uint32_t platform_limit;  /* Platform limitation on freq usage */
+	struct xen_pct_register control_register;
+	struct xen_pct_register status_register;
+	uint32_t state_count;     /* total available performance states */
+	GUEST_HANDLE(xen_processor_px) states;
+	struct xen_psd_package domain_info;
+	uint32_t shared_type;     /* coordination type of this processor */
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_processor_performance);
+
+struct xenpf_set_processor_pminfo {
+	/* IN variables */
+	uint32_t id;    /* ACPI CPU ID */
+	uint32_t type;  /* {XEN_PM_CX, XEN_PM_PX} */
+	union {
+		struct xen_processor_power          power;/* Cx: _CST/_CSD */
+		struct xen_processor_performance    perf; /* Px: _PPC/_PCT/_PSS/_PSD */
+	};
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_set_processor_pminfo);
+
+struct xen_platform_op {
+	uint32_t cmd;
+	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
+	union {
+		struct xenpf_settime           settime;
+		struct xenpf_add_memtype       add_memtype;
+		struct xenpf_del_memtype       del_memtype;
+		struct xenpf_read_memtype      read_memtype;
+		struct xenpf_microcode_update  microcode;
+		struct xenpf_platform_quirk    platform_quirk;
+		struct xenpf_firmware_info     firmware_info;
+		struct xenpf_enter_acpi_sleep  enter_acpi_sleep;
+		struct xenpf_change_freq       change_freq;
+		struct xenpf_getidletime       getidletime;
+		struct xenpf_set_processor_pminfo set_pminfo;
+		uint8_t                        pad[128];
+	} u;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_platform_op_t);
+
+#endif /* __XEN_PUBLIC_PLATFORM_H__ */
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 70213b4..d83cc08 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -453,6 +453,7 @@ struct start_info {
 /* These flags are passed in the 'flags' field of start_info_t. */
 #define SIF_PRIVILEGED    (1<<0)  /* Is the domain privileged? */
 #define SIF_INITDOMAIN    (1<<1)  /* Is this the initial control domain? */
+#define SIF_PM_MASK       (0xFF<<8) /* reserve 1 byte for xen-pm options */
 
 typedef uint64_t cpumap_t;
 
-- 
1.7.4.1