[PATCH] acpi: update struct acpi_table_tpm2

[PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

Updated struct acpi_table_tpm2 to conform to the official TCG ACPI
spec.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 include/acpi/actbl3.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 440ca81..33df1dd 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -688,13 +688,14 @@ enum acpi_rasf_status {
  * TPM2 - Trusted Platform Module (TPM) 2.0 Hardware Interface Table
  *        Version 3
  *
- * Conforms to "TPM 2.0 Hardware Interface Table (TPM2)" 29 November 2011
+ * Conforms to "TCG ACPI Specification for Family 1.2 and 2.0" 19 December 2014
  *
  ******************************************************************************/
 
 struct acpi_table_tpm2 {
 	struct acpi_table_header header;	/* Common ACPI table header */
-	u32 flags;
+	u16 platform_class;
+	u16 reserved;
 	u64 control_address;
 	u32 start_method;
 };
-- 
2.1.4

RE: [PATCH] acpi: update struct acpi_table_tpm2

[Moore, Robert]

It looks like there is a change to the TCPA table also.


> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Monday, June 08, 2015 6:38 AM
> To: Moore, Robert
> Cc: Jarkko Sakkinen; Zheng, Lv; Wysocki, Rafael J; Len Brown; open
> list:ACPI COMPONENT AR...; open list:ACPI COMPONENT AR...; open list
> Subject: [PATCH] acpi: update struct acpi_table_tpm2
> 
> Updated struct acpi_table_tpm2 to conform to the official TCG ACPI spec.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  include/acpi/actbl3.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h index
> 440ca81..33df1dd 100644
> --- a/include/acpi/actbl3.h
> +++ b/include/acpi/actbl3.h
> @@ -688,13 +688,14 @@ enum acpi_rasf_status {
>   * TPM2 - Trusted Platform Module (TPM) 2.0 Hardware Interface Table
>   *        Version 3
>   *
> - * Conforms to "TPM 2.0 Hardware Interface Table (TPM2)" 29 November 2011
> + * Conforms to "TCG ACPI Specification for Family 1.2 and 2.0" 19
> + December 2014
>   *
> 
> **************************************************************************
> ****/
> 
>  struct acpi_table_tpm2 {
>  	struct acpi_table_header header;	/* Common ACPI table header */
> -	u32 flags;
> +	u16 platform_class;
> +	u16 reserved;
>  	u64 control_address;
>  	u32 start_method;
>  };
> --
> 2.1.4

Re: [PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

On Mon, Jun 08, 2015 at 08:52:02PM +0000, Moore, Robert wrote:
> It looks like there is a change to the TCPA table also.

Right. I'll update that too.

I strongly think that the struct acpi_tpm2_control should not be in
actbl3.h. It is not defined in the TCG ACPI specification. It is
defined in

http://www.trustedcomputinggroup.org/resources/pc_client_platform_tpm_profile_ptp_specification

FIFO control structures are internal for to the TPM subsystem and so
should be CRB control structures (and we have already inside tpm_crb.c).

The structure ended up there probably because it was combined with the
TPM2 table in that Microsoft specification.

/Jarkko

RE: [PATCH] acpi: update struct acpi_table_tpm2

[Moore, Robert]

ACPICA usually defines any "related" data structures, just for user convenience.

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Tuesday, June 09, 2015 2:18 AM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> AR...; open list:ACPI COMPONENT AR...; open list
> Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> 
> On Mon, Jun 08, 2015 at 08:52:02PM +0000, Moore, Robert wrote:
> > It looks like there is a change to the TCPA table also.
> 
> Right. I'll update that too.
> 
> I strongly think that the struct acpi_tpm2_control should not be in
> actbl3.h. It is not defined in the TCG ACPI specification. It is defined
> in
> 
> http://www.trustedcomputinggroup.org/resources/pc_client_platform_tpm_prof
> ile_ptp_specification
> 
> FIFO control structures are internal for to the TPM subsystem and so
> should be CRB control structures (and we have already inside tpm_crb.c).
> 
> The structure ended up there probably because it was combined with the
> TPM2 table in that Microsoft specification.
> 
> /Jarkko

Re: [PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> ACPICA usually defines any "related" data structures, just for user
> convenience.

If you want to keep it, it's fine for me but we still probably use the
internal structure for it in tpm_crb driver (as tpm_tis uses internal
structure for CRB).

Do other updates look fine? I'm looking into migrating to tpm_crb driver
to use actbl3.h.

/Jarkko

> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Tuesday, June 09, 2015 2:18 AM
> > To: Moore, Robert
> > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> > AR...; open list:ACPI COMPONENT AR...; open list
> > Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> > 
> > On Mon, Jun 08, 2015 at 08:52:02PM +0000, Moore, Robert wrote:
> > > It looks like there is a change to the TCPA table also.
> > 
> > Right. I'll update that too.
> > 
> > I strongly think that the struct acpi_tpm2_control should not be in
> > actbl3.h. It is not defined in the TCG ACPI specification. It is defined
> > in
> > 
> > http://www.trustedcomputinggroup.org/resources/pc_client_platform_tpm_prof
> > ile_ptp_specification
> > 
> > FIFO control structures are internal for to the TPM subsystem and so
> > should be CRB control structures (and we have already inside tpm_crb.c).
> > 
> > The structure ended up there probably because it was combined with the
> > TPM2 table in that Microsoft specification.
> > 
> > /Jarkko

Re: [PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

On Tue, Jun 09, 2015 at 06:19:19PM +0300, Jarkko Sakkinen wrote:
> On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> > ACPICA usually defines any "related" data structures, just for user
> > convenience.
> 
> If you want to keep it, it's fine for me but we still probably use the
> internal structure for it in tpm_crb driver (as tpm_tis uses internal
> structure for CRB).

Let me open this a little bit. Everytime we want to do some small change to
control area (lets say TCG adds some flag) we would have to cycle them
through you.

And the changes are not coupled with ACPI in any possible way. This is
only adds more maintenance burden to you and also to us. This structure
is the main control structure for the CRB driver and will be refined
many times in the future. There is no any kind of use to its fields
outside of the CRB driver.

/Jarkko

RE: [PATCH] acpi: update struct acpi_table_tpm2

[Moore, Robert]

Well, you don't have to use it. ACPICA has done this in several other places. Anyway, I'm hesitant to just remove it at this point.


> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Tuesday, June 09, 2015 8:43 AM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> AR...; open list:ACPI COMPONENT AR...; open list
> Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> 
> On Tue, Jun 09, 2015 at 06:19:19PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> > > ACPICA usually defines any "related" data structures, just for user
> > > convenience.
> >
> > If you want to keep it, it's fine for me but we still probably use the
> > internal structure for it in tpm_crb driver (as tpm_tis uses internal
> > structure for CRB).
> 
> Let me open this a little bit. Everytime we want to do some small change
> to control area (lets say TCG adds some flag) we would have to cycle them
> through you.
> 
> And the changes are not coupled with ACPI in any possible way. This is
> only adds more maintenance burden to you and also to us. This structure is
> the main control structure for the CRB driver and will be refined many
> times in the future. There is no any kind of use to its fields outside of
> the CRB driver.
> 
> /Jarkko

Re: [PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

On Tue, Jun 09, 2015 at 04:12:52PM +0000, Moore, Robert wrote:
> Well, you don't have to use it. ACPICA has done this in several other
> places. Anyway, I'm hesitant to just remove it at this point.

OK, understood. I'll send you an updated patch without the removal.

/Jarkko

> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Tuesday, June 09, 2015 8:43 AM
> > To: Moore, Robert
> > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> > AR...; open list:ACPI COMPONENT AR...; open list
> > Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> > 
> > On Tue, Jun 09, 2015 at 06:19:19PM +0300, Jarkko Sakkinen wrote:
> > > On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> > > > ACPICA usually defines any "related" data structures, just for user
> > > > convenience.
> > >
> > > If you want to keep it, it's fine for me but we still probably use the
> > > internal structure for it in tpm_crb driver (as tpm_tis uses internal
> > > structure for CRB).
> > 
> > Let me open this a little bit. Everytime we want to do some small change
> > to control area (lets say TCG adds some flag) we would have to cycle them
> > through you.
> > 
> > And the changes are not coupled with ACPI in any possible way. This is
> > only adds more maintenance burden to you and also to us. This structure is
> > the main control structure for the CRB driver and will be refined many
> > times in the future. There is no any kind of use to its fields outside of
> > the CRB driver.
> > 
> > /Jarkko

RE: [PATCH] acpi: update struct acpi_table_tpm2

[Moore, Robert]

Looking at the TCG ACPI Specification today, it looks like there are major changes to the TCPA table. There is a whole bunch of new stuff after the Log Area Start Address.

Perhaps I missed or didn't get part of your proposed change.


> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Tuesday, June 09, 2015 8:19 AM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> AR...; open list:ACPI COMPONENT AR...; open list
> Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> 
> On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> > ACPICA usually defines any "related" data structures, just for user
> > convenience.
> 
> If you want to keep it, it's fine for me but we still probably use the
> internal structure for it in tpm_crb driver (as tpm_tis uses internal
> structure for CRB).
> 
> Do other updates look fine? I'm looking into migrating to tpm_crb driver
> to use actbl3.h.
> 
> /Jarkko
> 
> >
> > > -----Original Message-----
> > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > Sent: Tuesday, June 09, 2015 2:18 AM
> > > To: Moore, Robert
> > > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI
> > > COMPONENT AR...; open list:ACPI COMPONENT AR...; open list
> > > Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> > >
> > > On Mon, Jun 08, 2015 at 08:52:02PM +0000, Moore, Robert wrote:
> > > > It looks like there is a change to the TCPA table also.
> > >
> > > Right. I'll update that too.
> > >
> > > I strongly think that the struct acpi_tpm2_control should not be in
> > > actbl3.h. It is not defined in the TCG ACPI specification. It is
> > > defined in
> > >
> > > http://www.trustedcomputinggroup.org/resources/pc_client_platform_tp
> > > m_prof
> > > ile_ptp_specification
> > >
> > > FIFO control structures are internal for to the TPM subsystem and so
> > > should be CRB control structures (and we have already inside
> tpm_crb.c).
> > >
> > > The structure ended up there probably because it was combined with
> > > the
> > > TPM2 table in that Microsoft specification.
> > >
> > > /Jarkko

Re: [PATCH] acpi: update struct acpi_table_tpm2

[Jarkko Sakkinen]

On Tue, Jun 09, 2015 at 04:16:14PM +0000, Moore, Robert wrote:
> Looking at the TCG ACPI Specification today, it looks like there are
> major changes to the TCPA table. There is a whole bunch of new stuff
> after the Log Area Start Address.
> 
> Perhaps I missed or didn't get part of your proposed change.

I was looking only at the client TCPA table so it was my bad. The client
table has only the change that I did in the patch. I'm myself focused
towards TPM 2.0 work.

If you look at very old specification version:

https://www.trustedcomputinggroup.org/files/temp/6453AF78-1D09-3519-AD74028427486A3B/Server%20TCG_ACPIGeneralSpecification.pdf

You see that there is also a server TCPA with a lot more fields that we
have. If the need hasn't rised for those fields since 2005, I would not
add them unless there is a real use case.

If you seriously want those fields to be part of the patch, I can do a
struct for server TCPA but I don't think there will be value for it.


/Jarkko

> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Tuesday, June 09, 2015 8:19 AM
> > To: Moore, Robert
> > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI COMPONENT
> > AR...; open list:ACPI COMPONENT AR...; open list
> > Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> > 
> > On Tue, Jun 09, 2015 at 02:21:12PM +0000, Moore, Robert wrote:
> > > ACPICA usually defines any "related" data structures, just for user
> > > convenience.
> > 
> > If you want to keep it, it's fine for me but we still probably use the
> > internal structure for it in tpm_crb driver (as tpm_tis uses internal
> > structure for CRB).
> > 
> > Do other updates look fine? I'm looking into migrating to tpm_crb driver
> > to use actbl3.h.
> > 
> > /Jarkko
> > 
> > >
> > > > -----Original Message-----
> > > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > > Sent: Tuesday, June 09, 2015 2:18 AM
> > > > To: Moore, Robert
> > > > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; open list:ACPI
> > > > COMPONENT AR...; open list:ACPI COMPONENT AR...; open list
> > > > Subject: Re: [PATCH] acpi: update struct acpi_table_tpm2
> > > >
> > > > On Mon, Jun 08, 2015 at 08:52:02PM +0000, Moore, Robert wrote:
> > > > > It looks like there is a change to the TCPA table also.
> > > >
> > > > Right. I'll update that too.
> > > >
> > > > I strongly think that the struct acpi_tpm2_control should not be in
> > > > actbl3.h. It is not defined in the TCG ACPI specification. It is
> > > > defined in
> > > >
> > > > http://www.trustedcomputinggroup.org/resources/pc_client_platform_tp
> > > > m_prof
> > > > ile_ptp_specification
> > > >
> > > > FIFO control structures are internal for to the TPM subsystem and so
> > > > should be CRB control structures (and we have already inside
> > tpm_crb.c).
> > > >
> > > > The structure ended up there probably because it was combined with
> > > > the
> > > > TPM2 table in that Microsoft specification.
> > > >
> > > > /Jarkko