[PATCH v2 0/3] tpm2: Make TPM2 logs accessible for non-UEFI firmware

[PATCH v2 0/3] tpm2: Make TPM2 logs accessible for non-UEFI firmware

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

This series of patches extends the existing TPM2 ACPI table with additional
fields found in the TPM2 TCG ACPI specification (reference is in the patch)
that allow access to the log's address and its size. We then modify the
code that so far only enables access to a TPM 1.2's log for a TPM2 as well.
This then enables access to the TPM2's log on non-UEFI system that for example
run SeaBIOS.

   Stefan

v1->v2:
  - Repost of the series

Stefan Berger (3):
  acpi: Extend TPM2 ACPI table with missing log fields
  tpm: Rearrange ACPI log code to easier extend for TPM2 case
  tpm: Add support for ACPI logs from TPM2 ACPI table

 drivers/char/tpm/eventlog/acpi.c | 54 ++++++++++++++++++++++++----------------
 drivers/char/tpm/tpm_crb.c       | 13 +++++++---
 drivers/char/tpm/tpm_tis.c       |  4 ++-
 include/acpi/actbl3.h            |  5 ++--
 4 files changed, 49 insertions(+), 27 deletions(-)

-- 
2.14.5

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

[Stefan Berger]

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(-)

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 */
-- 
2.14.5

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

[Jarkko Sakkinen]

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

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

[Stefan Berger]

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

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

[Jarkko Sakkinen]

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

[PATCH v2 2/3] tpm: Rearrange ACPI log code to easier extend for TPM2 case

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Rearrange the ACPI table based log code to allow for easier extension
for the TPM2 case.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 drivers/char/tpm/eventlog/acpi.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 63ada5e53f13..1ef2267c9378 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -50,9 +50,6 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 	u64 len, start;
 	struct tpm_bios_log *log;
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return -ENODEV;
-
 	log = &chip->log;
 
 	/* Unfortuntely ACPI does not associate the event log with a specific
@@ -61,23 +58,27 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 	if (!chip->acpi_dev_handle)
 		return -ENODEV;
 
-	/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
-	status = acpi_get_table(ACPI_SIG_TCPA, 1,
-				(struct acpi_table_header **)&buff);
-
-	if (ACPI_FAILURE(status))
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		return -ENODEV;
-
-	switch(buff->platform_class) {
-	case BIOS_SERVER:
-		len = buff->server.log_max_len;
-		start = buff->server.log_start_addr;
-		break;
-	case BIOS_CLIENT:
-	default:
-		len = buff->client.log_max_len;
-		start = buff->client.log_start_addr;
-		break;
+	} else {
+		/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
+		status = acpi_get_table(ACPI_SIG_TCPA, 1,
+					(struct acpi_table_header **)&buff);
+
+		if (ACPI_FAILURE(status))
+			return -ENODEV;
+
+		switch (buff->platform_class) {
+		case BIOS_SERVER:
+			len = buff->server.log_max_len;
+			start = buff->server.log_start_addr;
+			break;
+		case BIOS_CLIENT:
+		default:
+			len = buff->client.log_max_len;
+			start = buff->client.log_start_addr;
+			break;
+		}
 	}
 	if (!len) {
 		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
-- 
2.14.5

Re: [PATCH v2 2/3] tpm: Rearrange ACPI log code to easier extend for TPM2 case

[Jarkko Sakkinen]

On Mon, Mar 30, 2020 at 11:15:35AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Rearrange the ACPI table based log code to allow for easier extension
> for the TPM2 case.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

I think this change does not do anything useful.

/Jarkko

Re: [PATCH v2 2/3] tpm: Rearrange ACPI log code to easier extend for TPM2 case

[Stefan Berger]

On 3/30/20 3:32 PM, Jarkko Sakkinen wrote:
> On Mon, Mar 30, 2020 at 11:15:35AM -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Rearrange the ACPI table based log code to allow for easier extension
>> for the TPM2 case.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> I think this change does not do anything useful.

I know. So it's easy to see what's happening and harmless. But I'll 
squash it.


>
> /Jarkko

Re: [PATCH v2 2/3] tpm: Rearrange ACPI log code to easier extend for TPM2 case

[Jarkko Sakkinen]

On Mon, Mar 30, 2020 at 05:12:24PM -0400, Stefan Berger wrote:
> On 3/30/20 3:32 PM, Jarkko Sakkinen wrote:
> > On Mon, Mar 30, 2020 at 11:15:35AM -0400, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > > 
> > > Rearrange the ACPI table based log code to allow for easier extension
> > > for the TPM2 case.
> > > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > I think this change does not do anything useful.
> 
> I know. So it's easy to see what's happening and harmless. But I'll squash
> it.

Yeah, well files aren't getting moved or anything and overally
the change is still somewhat scoped.

/Jarkko

[PATCH v2 3/3] tpm: Add support for ACPI logs from TPM2 ACPI table

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

In case of a TPM2 search for a TPM2 ACPI table and if found use it to
find start and length of the log area.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 drivers/char/tpm/eventlog/acpi.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 1ef2267c9378..2447f21e4a23 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -49,6 +49,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 	void __iomem *virt;
 	u64 len, start;
 	struct tpm_bios_log *log;
+	struct acpi_table_tpm2 *tbl;
+	int format;
 
 	log = &chip->log;
 
@@ -59,7 +61,15 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		return -ENODEV;
+		status = acpi_get_table("TPM2", 1,
+					(struct acpi_table_header **)&tbl);
+		if (ACPI_FAILURE(status))
+			return -ENODEV;
+		if (tbl->header.length < sizeof(*tbl))
+			return -ENODEV;
+		len = tbl->log_area_minimum_length;
+		start = tbl->log_area_start_address;
+		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
 	} else {
 		/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
 		status = acpi_get_table(ACPI_SIG_TCPA, 1,
@@ -79,6 +89,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 			start = buff->client.log_start_addr;
 			break;
 		}
+		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
 	}
 	if (!len) {
 		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
@@ -99,7 +110,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 	memcpy_fromio(log->bios_event_log, virt, len);
 
 	acpi_os_unmap_iomem(virt, len);
-	return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+	return format;
 
 err:
 	kfree(log->bios_event_log);
-- 
2.14.5

Re: [PATCH v2 3/3] tpm: Add support for ACPI logs from TPM2 ACPI table

[Jarkko Sakkinen]

On Mon, Mar 30, 2020 at 11:15:36AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> In case of a TPM2 search for a TPM2 ACPI table and if found use it to
> find start and length of the log area.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Please use "tpm/eventlog".

There's a typo in the short summary: I guess you meant event log when
you speak of "ACPI log".

Also, I'm not getting the long description e.g. "TPM2 search" is an
alien term.

> ---
>  drivers/char/tpm/eventlog/acpi.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 1ef2267c9378..2447f21e4a23 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -49,6 +49,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  	void __iomem *virt;
>  	u64 len, start;
>  	struct tpm_bios_log *log;
> +	struct acpi_table_tpm2 *tbl;
> +	int format;
>  
>  	log = &chip->log;
>  
> @@ -59,7 +61,15 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  		return -ENODEV;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> -		return -ENODEV;
> +		status = acpi_get_table("TPM2", 1,
> +					(struct acpi_table_header **)&tbl);
> +		if (ACPI_FAILURE(status))
> +			return -ENODEV;
> +		if (tbl->header.length < sizeof(*tbl))
> +			return -ENODEV;
> +		len = tbl->log_area_minimum_length;
> +		start = tbl->log_area_start_address;
> +		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
>  	} else {
>  		/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
>  		status = acpi_get_table(ACPI_SIG_TCPA, 1,
> @@ -79,6 +89,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  			start = buff->client.log_start_addr;
>  			break;
>  		}
> +		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
>  	}
>  	if (!len) {
>  		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
> @@ -99,7 +110,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  	memcpy_fromio(log->bios_event_log, virt, len);
>  
>  	acpi_os_unmap_iomem(virt, len);
> -	return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> +	return format;
>  
>  err:
>  	kfree(log->bios_event_log);
> -- 
> 2.14.5
> 

The code change looks fine but need a review after squashed with 2/3.

/Jarkko