diff mbox series

[v2,1/3] acpi: Extend TPM2 ACPI table with missing log fields

Message ID 20200330151536.871700-2-stefanb@linux.vnet.ibm.com (mailing list archive)
State Not Applicable, archived
Headers show
Series tpm2: Make TPM2 logs accessible for non-UEFI firmware | expand

Commit Message

Stefan Berger March 30, 2020, 3:15 p.m. UTC 
From: Stefan Berger <stefanb@linux.ibm.com>

Recent extensions of the TPM2 ACPI table added 3 more fields
including 12 bytes of start method specific parameters and Log Area
Minimum Length (u32) and Log Area Start Address (u64). So, we extend
the existing structure with these fields to allow non-UEFI systems
to access the TPM2's log.

The specification that has the new fields is the following:
  TCG ACPI Specification
  Family "1.2" and "2.0"
  Version 1.2, Revision 8

Adapt all existing table size calculations to use
offsetof(struct acpi_table_tpm2, start_method_specific)
[where start_method_specific is a newly added field]
rather than sizeof(struct acpi_table_tpm2) so that the addition
of the new fields does not affect current systems that may not
have them.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 drivers/char/tpm/tpm_crb.c | 13 ++++++++++---
 drivers/char/tpm/tpm_tis.c |  4 +++-
 include/acpi/actbl3.h      |  5 +++--
 3 files changed, 16 insertions(+), 6 deletions(-)

Comments

Jarkko Sakkinen March 30, 2020, 7:28 p.m. UTC | #1 
On Mon, Mar 30, 2020 at 11:15:34AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Recent extensions of the TPM2 ACPI table added 3 more fields
> including 12 bytes of start method specific parameters and Log Area
> Minimum Length (u32) and Log Area Start Address (u64). So, we extend
> the existing structure with these fields to allow non-UEFI systems
> to access the TPM2's log.
> 
> The specification that has the new fields is the following:
>   TCG ACPI Specification
>   Family "1.2" and "2.0"
>   Version 1.2, Revision 8
> 
> Adapt all existing table size calculations to use
> offsetof(struct acpi_table_tpm2, start_method_specific)
> [where start_method_specific is a newly added field]
> rather than sizeof(struct acpi_table_tpm2) so that the addition
> of the new fields does not affect current systems that may not
> have them.
> 

Cc: linux-acpi@vger.kernel.org

> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

You have this comment:

/* Platform-specific data follows */

Can you elaborate a bit which platform you are speaking of? It is now
enabled for everything.

/Jarkko
Stefan Berger March 30, 2020, 9:26 p.m. UTC | #2 
On 3/30/20 3:28 PM, Jarkko Sakkinen wrote:
> On Mon, Mar 30, 2020 at 11:15:34AM -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Recent extensions of the TPM2 ACPI table added 3 more fields
>> including 12 bytes of start method specific parameters and Log Area
>> Minimum Length (u32) and Log Area Start Address (u64). So, we extend
>> the existing structure with these fields to allow non-UEFI systems
>> to access the TPM2's log.
>>
>> The specification that has the new fields is the following:
>>    TCG ACPI Specification
>>    Family "1.2" and "2.0"
>>    Version 1.2, Revision 8
>>
>> Adapt all existing table size calculations to use
>> offsetof(struct acpi_table_tpm2, start_method_specific)
>> [where start_method_specific is a newly added field]
>> rather than sizeof(struct acpi_table_tpm2) so that the addition
>> of the new fields does not affect current systems that may not
>> have them.
>>
> Cc: linux-acpi@vger.kernel.org
>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> You have this comment:
>
> /* Platform-specific data follows */

You mean there 'was this comment'? I actually removed this comment 
because I didn't know what it meant or what it has to do with 'platform':

-
-       /* Platform-specific data follows */
+       u8  start_method_specific[12];
+       u32 log_area_minimum_length;
+       u64 log_area_start_address;

Specs: 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf

PDF Page 16 / Doc page 10 bottom.

start_method_specific is obviously related to the start_method field. 
The subsequent two fields are optional and show those 2 filelds we know 
from the TCPA ACPI table.

>
> Can you elaborate a bit which platform you are speaking of? It is now
> enabled for everything.
>
> /Jarkko
Jarkko Sakkinen March 31, 2020, 11:55 a.m. UTC | #3 
On Mon, Mar 30, 2020 at 05:26:25PM -0400, Stefan Berger wrote:
> On 3/30/20 3:28 PM, Jarkko Sakkinen wrote:
> > On Mon, Mar 30, 2020 at 11:15:34AM -0400, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > > 
> > > Recent extensions of the TPM2 ACPI table added 3 more fields
> > > including 12 bytes of start method specific parameters and Log Area
> > > Minimum Length (u32) and Log Area Start Address (u64). So, we extend
> > > the existing structure with these fields to allow non-UEFI systems
> > > to access the TPM2's log.
> > > 
> > > The specification that has the new fields is the following:
> > >    TCG ACPI Specification
> > >    Family "1.2" and "2.0"
> > >    Version 1.2, Revision 8
> > > 
> > > Adapt all existing table size calculations to use
> > > offsetof(struct acpi_table_tpm2, start_method_specific)
> > > [where start_method_specific is a newly added field]
> > > rather than sizeof(struct acpi_table_tpm2) so that the addition
> > > of the new fields does not affect current systems that may not
> > > have them.
> > > 
> > Cc: linux-acpi@vger.kernel.org
> > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > You have this comment:
> > 
> > /* Platform-specific data follows */
> 
> You mean there 'was this comment'? I actually removed this comment because I
> didn't know what it meant or what it has to do with 'platform':
> 
> -
> -       /* Platform-specific data follows */
> +       u8  start_method_specific[12];
> +       u32 log_area_minimum_length;
> +       u64 log_area_start_address;
> 
> Specs: https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
> 
> PDF Page 16 / Doc page 10 bottom.
> 
> start_method_specific is obviously related to the start_method field. The
> subsequent two fields are optional and show those 2 filelds we know from the
> TCPA ACPI table.

You should CC the change to linux-acpi because it touches their files.
For this reason preferably this change should be commit of its own.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index a9dcf31eadd2..0565aa5482f9 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -669,7 +669,9 @@  static int crb_acpi_add(struct acpi_device *device)
 
 	status = acpi_get_table(ACPI_SIG_TPM2, 1,
 				(struct acpi_table_header **) &buf);
-	if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
+	if (ACPI_FAILURE(status) || buf->header.length <
+			offsetof(struct acpi_table_tpm2,
+				 start_method_specific)) {
 		dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
 		return -EINVAL;
 	}
@@ -684,14 +686,19 @@  static int crb_acpi_add(struct acpi_device *device)
 		return -ENOMEM;
 
 	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
-		if (buf->header.length < (sizeof(*buf) + sizeof(*crb_smc))) {
+		if (buf->header.length <
+			(offsetof(struct acpi_table_tpm2,
+				  start_method_specific) +
+			 sizeof(*crb_smc))) {
 			dev_err(dev,
 				FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
 				buf->header.length,
 				ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
 			return -EINVAL;
 		}
-		crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf, sizeof(*buf));
+		crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf,
+			   offsetof(struct acpi_table_tpm2,
+				    start_method_specific));
 		priv->smc_func_id = crb_smc->smc_func_id;
 	}
 
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index e7df342a317d..a80f36860bac 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -111,7 +111,9 @@  static int check_acpi_tpm2(struct device *dev)
 	 */
 	st =
 	    acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
-	if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
+	if (ACPI_FAILURE(st) || tbl->header.length <
+			offsetof(struct acpi_table_tpm2,
+				 start_method_specific)) {
 		dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
 		return -EINVAL;
 	}
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 2bf3baf819bb..ae205063bfa9 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -411,8 +411,9 @@  struct acpi_table_tpm2 {
 	u16 reserved;
 	u64 control_address;
 	u32 start_method;
-
-	/* Platform-specific data follows */
+	u8  start_method_specific[12];
+	u32 log_area_minimum_length;
+	u64 log_area_start_address;
 };
 
 /* Values for start_method above */