[PATCH 1/3 v2] tpm: Change response length of tpm2_get_capability()

[PATCH 1/3 v2] tpm: Change response length of tpm2_get_capability()

[Ilias Apalodimas]

For implementing the EFI_TCG2_PROTOCOL we need the count field returned by
the TPM when reading capabilities via tpm2_get_capability().

Adjust the implementation of the 'tpm2 get_capability' command accordingly.

Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes since v1:
- Unconditionally get the extra 4 bytes on the response and account for them
  in do_tpm_get_capability()
 cmd/tpm-v2.c | 4 ++--
 lib/tpm-v2.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index e6742656f578..5fa4788a72de 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -191,10 +191,10 @@ static int do_tpm_get_capability(struct cmd_tbl *cmdtp, int flag, int argc,
 	for (i = 0; i < count; i++) {
 		printf("Property 0x");
 		for (j = 0; j < 4; j++)
-			printf("%02x", data[(i * 8) + j]);
+			printf("%02x", data[(i * 8) + j + sizeof(u32)]);
 		printf(": 0x");
 		for (j = 4; j < 8; j++)
-			printf("%02x", data[(i * 8) + j]);
+			printf("%02x", data[(i * 8) + j + sizeof(u32)]);
 		printf("\n");
 	}
 
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index a4c352e3ef75..91759068cf03 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -184,10 +184,10 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
 	/*
 	 * In the response buffer, the properties are located after the:
 	 * tag (u16), response size (u32), response code (u32),
-	 * YES/NO flag (u8), TPM_CAP (u32) and TPMU_CAPABILITIES (u32).
+	 * YES/NO flag (u8), TPM_CAP (u32).
 	 */
 	properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
-			 sizeof(u8) + sizeof(u32) + sizeof(u32);
+			 sizeof(u8) + sizeof(u32);
 	memcpy(buf, &response[properties_off], response_len - properties_off);
 
 	return 0;
-- 
2.29.2

[PATCH 2/3 v2] tpm: Add some headers from the spec

[Ilias Apalodimas]

A following patch introduces EFI_TCG2_PROTOCOL.
Add the required TPMv2 headers to support that and remove the (now)
redundant definitions from tpm2_tis_sandbox

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/tpm-v2.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index f6c045d35480..b62f2c5b0fb8 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -11,6 +11,73 @@
 
 #define TPM2_DIGEST_LEN		32
 
+#define TPM2_MAX_PCRS 32
+#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
+#define TPM2_MAX_CAP_BUFFER 1024
+#define TPM2_MAX_TPM_PROPERTIES   ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* TPM2_CAP */ - \
+				   sizeof(u32)) / sizeof(struct tpms_tagged_property))
+
+/*
+ *  We deviate from this draft of the specification by increasing the value of TPM2_NUM_PCR_BANKS
+ *  from 3 to 16 to ensure compatibility with TPM2 implementations that have enabled a larger than
+ *  typical number of PCR banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included
+ *  in a future revision of the specification.
+ */
+#define TPM2_NUM_PCR_BANKS 16
+
+/* Definition of (UINT32) TPM2_CAP Constants */
+#define TPM2_CAP_PCRS 0x00000005U
+#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
+
+/* Definition of (UINT32) TPM2_PT Constants */
+#define PT_GROUP                   (u32)(0x00000100)
+#define PT_FIXED                   (u32)(PT_GROUP * 1)
+#define TPM2_PT_MANUFACTURER        (u32)(PT_FIXED + 5)
+#define TPM2_PT_PCR_COUNT           (u32)(PT_FIXED + 18)
+#define TPM2_PT_MAX_COMMAND_SIZE    (u32)(PT_FIXED + 30)
+#define TPM2_PT_MAX_RESPONSE_SIZE   (u32)(PT_FIXED + 31)
+
+/* TPMS_TAGGED_PROPERTY Structure */
+struct tpms_tagged_property {
+	u32 property;
+	u32 value;
+} __packed;
+
+/* TPMS_PCR_SELECTION Structure */
+struct tpms_pcr_selection {
+	u16 hash;
+	u8 size_of_select;
+	u8 pcr_select[TPM2_PCR_SELECT_MAX];
+} __packed;
+
+/* TPML_PCR_SELECTION Structure */
+struct tpml_pcr_selection {
+	u32 count;
+	struct tpms_pcr_selection selection[TPM2_NUM_PCR_BANKS];
+} __packed;
+
+/* TPML_TAGGED_TPM_PROPERTY Structure */
+struct tpml_tagged_tpm_property {
+	u32 count;
+	struct tpms_tagged_property tpm_property[TPM2_MAX_TPM_PROPERTIES];
+} __packed;
+
+/* TPMU_CAPABILITIES Union */
+union tpmu_capabilities {
+	/*
+	 * Non exhaustive. Only added the structs needed for our
+	 * current code
+	 */
+	struct tpml_pcr_selection assigned_pcr;
+	struct tpml_tagged_tpm_property tpm_properties;
+} __packed;
+
+/* TPMS_CAPABILITY_DATA Structure */
+struct tpms_capability_data {
+	u32 capability;
+	union tpmu_capabilities data;
+} __packed;
+
 /**
  * TPM2 Structure Tags for command/response buffers.
  *
@@ -123,11 +190,13 @@ enum tpm2_return_codes {
  * TPM2 algorithms.
  */
 enum tpm2_algorithms {
+	TPM2_ALG_SHA1		= 0x04,
 	TPM2_ALG_XOR		= 0x0A,
 	TPM2_ALG_SHA256		= 0x0B,
 	TPM2_ALG_SHA384		= 0x0C,
 	TPM2_ALG_SHA512		= 0x0D,
 	TPM2_ALG_NULL		= 0x10,
+	TPM2_ALG_SM3_256	= 0x12,
 };
 
 /* NV index attributes */
-- 
2.29.2

[PATCH 2/3 v2] tpm: Add some headers from the spec

[Heinrich Schuchardt]

On 11/5/20 10:58 PM, Ilias Apalodimas wrote:
> A following patch introduces EFI_TCG2_PROTOCOL.
> Add the required TPMv2 headers to support that and remove the (now)
> redundant definitions from tpm2_tis_sandbox
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  include/tpm-v2.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index f6c045d35480..b62f2c5b0fb8 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -11,6 +11,73 @@
>
>  #define TPM2_DIGEST_LEN		32

Miquel, the constant name and its usage seems not to reflect the TCG spec:

The spec has the following alternative digest length constants:

#define TPM2_SHA_DIGEST_SIZE       20
#define TPM2_SHA1_DIGEST_SIZE      20
#define TPM2_SHA256_DIGEST_SIZE    32
#define TPM2_SHA384_DIGEST_SIZE    48
#define TPM2_SHA512_DIGEST_SIZE    64
#define TPM2_SM3_256_DIGEST_SIZE   32

Can't we use the same names as in the spec? Why did you assume in your
code that SHA256 is the only digest being used?

>
> +#define TPM2_MAX_PCRS 32
> +#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
> +#define TPM2_MAX_CAP_BUFFER 1024
> +#define TPM2_MAX_TPM_PROPERTIES   ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* TPM2_CAP */ - \
> +				   sizeof(u32)) / sizeof(struct tpms_tagged_property))
> +
> +/*
> + *  We deviate from this draft of the specification by increasing the value of TPM2_NUM_PCR_BANKS
> + *  from 3 to 16 to ensure compatibility with TPM2 implementations that have enabled a larger than
> + *  typical number of PCR banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included
> + *  in a future revision of the specification.

Ilias, please, restrict your lines to 80 characters where possible.

Do you have a reference for the usage of that larger number on current
hardware? We should make the deviation from the standard easily verifiable.

> + */
> +#define TPM2_NUM_PCR_BANKS 16
> +
> +/* Definition of (UINT32) TPM2_CAP Constants */
> +#define TPM2_CAP_PCRS 0x00000005U
> +#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
> +
> +/* Definition of (UINT32) TPM2_PT Constants */
> +#define PT_GROUP                   (u32)(0x00000100)
> +#define PT_FIXED                   (u32)(PT_GROUP * 1)
> +#define TPM2_PT_MANUFACTURER        (u32)(PT_FIXED + 5)
> +#define TPM2_PT_PCR_COUNT           (u32)(PT_FIXED + 18)
> +#define TPM2_PT_MAX_COMMAND_SIZE    (u32)(PT_FIXED + 30)
> +#define TPM2_PT_MAX_RESPONSE_SIZE   (u32)(PT_FIXED + 31)

All these definitions are all copied from the "TCG TSS2.0 Overview and
Common Structures Specification". I am missing a reference to the
copyright notice of the spec. I think the best thing to do would be
placing the TCG copyrighted code into a separate include that is
included in tpm_v2.h. Please, check with Tom if the license contradicts
GPL. Especially the following sentence seems problematic:

"THE COPYRIGHT LICENSES SET FORTH ABOVE DO NOT REPRESENT ANY FORM OF
LICENSE OR WAIVER, EXPRESS OR IMPLIED, BY ESTOPPEL OR OTHERWISE, WITH
RESPECT TO PATENT RIGHTS HELD BY TCG MEMBERS (OR OTHER THIRD PARTIES)
THAT MAY BE NECESSARY TO IMPLEMENT THIS SPECIFICATION OR OTHERWISE."

Cf. https://fedoraproject.org/wiki/Licensing/TCGL

> +
> +/* TPMS_TAGGED_PROPERTY Structure */
> +struct tpms_tagged_property {
> +	u32 property;
> +	u32 value;
> +} __packed;
> +
> +/* TPMS_PCR_SELECTION Structure */
> +struct tpms_pcr_selection {
> +	u16 hash;
(This is a TPM2_ALG_ID.)
> +	u8 size_of_select;
> +	u8 pcr_select[TPM2_PCR_SELECT_MAX];
> +} __packed;
> +
> +/* TPML_PCR_SELECTION Structure */
> +struct tpml_pcr_selection {
> +	u32 count;
> +	struct tpms_pcr_selection selection[TPM2_NUM_PCR_BANKS];
> +} __packed;
> +
> +/* TPML_TAGGED_TPM_PROPERTY Structure */
> +struct tpml_tagged_tpm_property {
> +	u32 count;
> +	struct tpms_tagged_property tpm_property[TPM2_MAX_TPM_PROPERTIES];
> +} __packed;
> +
> +/* TPMU_CAPABILITIES Union */
> +union tpmu_capabilities {
> +	/*
> +	 * Non exhaustive. Only added the structs needed for our
> +	 * current code
> +	 */
> +	struct tpml_pcr_selection assigned_pcr;
> +	struct tpml_tagged_tpm_property tpm_properties;
> +} __packed;
> +
> +/* TPMS_CAPABILITY_DATA Structure */
> +struct tpms_capability_data {
> +	u32 capability;
> +	union tpmu_capabilities data;
> +} __packed;
> +
>  /**
>   * TPM2 Structure Tags for command/response buffers.
>   *
> @@ -123,11 +190,13 @@ enum tpm2_return_codes {
>   * TPM2 algorithms.
>   */
>  enum tpm2_algorithms {
> +	TPM2_ALG_SHA1		= 0x04,
>  	TPM2_ALG_XOR		= 0x0A,
>  	TPM2_ALG_SHA256		= 0x0B,
>  	TPM2_ALG_SHA384		= 0x0C,
>  	TPM2_ALG_SHA512		= 0x0D,
>  	TPM2_ALG_NULL		= 0x10,
> +	TPM2_ALG_SM3_256	= 0x12,
>  };

In the spec these values are not an enum (i.e. 32 bit values if you do
not use short enums) but explicitly u16/TPM2_ALG_ID. But probably that
does not make a difference.

Best regards

Heinrich

>
>  /* NV index attributes */
>

[PATCH 2/3 v2] tpm: Add some headers from the spec

[Ilias Apalodimas]

On Sat, Nov 07, 2020 at 09:33:35PM +0100, Heinrich Schuchardt wrote:
> On 11/5/20 10:58 PM, Ilias Apalodimas wrote:
> > A following patch introduces EFI_TCG2_PROTOCOL.
> > Add the required TPMv2 headers to support that and remove the (now)
> > redundant definitions from tpm2_tis_sandbox
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  include/tpm-v2.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index f6c045d35480..b62f2c5b0fb8 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -11,6 +11,73 @@
> >
> >  #define TPM2_DIGEST_LEN		32
> 
> Miquel, the constant name and its usage seems not to reflect the TCG spec:
> 
> The spec has the following alternative digest length constants:
> 
> #define TPM2_SHA_DIGEST_SIZE       20
> #define TPM2_SHA1_DIGEST_SIZE      20
> #define TPM2_SHA256_DIGEST_SIZE    32
> #define TPM2_SHA384_DIGEST_SIZE    48
> #define TPM2_SHA512_DIGEST_SIZE    64
> #define TPM2_SM3_256_DIGEST_SIZE   32
> 
> Can't we use the same names as in the spec? Why did you assume in your
> code that SHA256 is the only digest being used?
> 
> >
> > +#define TPM2_MAX_PCRS 32
> > +#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
> > +#define TPM2_MAX_CAP_BUFFER 1024
> > +#define TPM2_MAX_TPM_PROPERTIES   ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* TPM2_CAP */ - \
> > +				   sizeof(u32)) / sizeof(struct tpms_tagged_property))
> > +
> > +/*
> > + *  We deviate from this draft of the specification by increasing the value of TPM2_NUM_PCR_BANKS
> > + *  from 3 to 16 to ensure compatibility with TPM2 implementations that have enabled a larger than
> > + *  typical number of PCR banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included
> > + *  in a future revision of the specification.
> 
> Ilias, please, restrict your lines to 80 characters where possible.
> 

Sure

> Do you have a reference for the usage of that larger number on current
> hardware? We should make the deviation from the standard easily verifiable.
> 

Yes this was copied from this:
https://tpm2-tss.readthedocs.io/en/latest/

> > + */
> > +#define TPM2_NUM_PCR_BANKS 16
> > +
> > +/* Definition of (UINT32) TPM2_CAP Constants */
> > +#define TPM2_CAP_PCRS 0x00000005U
> > +#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
> > +
> > +/* Definition of (UINT32) TPM2_PT Constants */
> > +#define PT_GROUP                   (u32)(0x00000100)
> > +#define PT_FIXED                   (u32)(PT_GROUP * 1)
> > +#define TPM2_PT_MANUFACTURER        (u32)(PT_FIXED + 5)
> > +#define TPM2_PT_PCR_COUNT           (u32)(PT_FIXED + 18)
> > +#define TPM2_PT_MAX_COMMAND_SIZE    (u32)(PT_FIXED + 30)
> > +#define TPM2_PT_MAX_RESPONSE_SIZE   (u32)(PT_FIXED + 31)
> 
> All these definitions are all copied from the "TCG TSS2.0 Overview and
> Common Structures Specification". I am missing a reference to the
> copyright notice of the spec. I think the best thing to do would be
> placing the TCG copyrighted code into a separate include that is
> included in tpm_v2.h. Please, check with Tom if the license contradicts
> GPL. Especially the following sentence seems problematic:
> 
> "THE COPYRIGHT LICENSES SET FORTH ABOVE DO NOT REPRESENT ANY FORM OF
> LICENSE OR WAIVER, EXPRESS OR IMPLIED, BY ESTOPPEL OR OTHERWISE, WITH
> RESPECT TO PATENT RIGHTS HELD BY TCG MEMBERS (OR OTHER THIRD PARTIES)
> THAT MAY BE NECESSARY TO IMPLEMENT THIS SPECIFICATION OR OTHERWISE."
> 
> Cf. https://fedoraproject.org/wiki/Licensing/TCGL
> 

Ok will do

> > +
> > +/* TPMS_TAGGED_PROPERTY Structure */
> > +struct tpms_tagged_property {
> > +	u32 property;
> > +	u32 value;
> > +} __packed;
> > +
> > +/* TPMS_PCR_SELECTION Structure */
> > +struct tpms_pcr_selection {
> > +	u16 hash;
> (This is a TPM2_ALG_ID.)
> > +	u8 size_of_select;
> > +	u8 pcr_select[TPM2_PCR_SELECT_MAX];
> > +} __packed;
> > +
> > +/* TPML_PCR_SELECTION Structure */
> > +struct tpml_pcr_selection {
> > +	u32 count;
> > +	struct tpms_pcr_selection selection[TPM2_NUM_PCR_BANKS];
> > +} __packed;
> > +
> > +/* TPML_TAGGED_TPM_PROPERTY Structure */
> > +struct tpml_tagged_tpm_property {
> > +	u32 count;
> > +	struct tpms_tagged_property tpm_property[TPM2_MAX_TPM_PROPERTIES];
> > +} __packed;
> > +
> > +/* TPMU_CAPABILITIES Union */
> > +union tpmu_capabilities {
> > +	/*
> > +	 * Non exhaustive. Only added the structs needed for our
> > +	 * current code
> > +	 */
> > +	struct tpml_pcr_selection assigned_pcr;
> > +	struct tpml_tagged_tpm_property tpm_properties;
> > +} __packed;
> > +
> > +/* TPMS_CAPABILITY_DATA Structure */
> > +struct tpms_capability_data {
> > +	u32 capability;
> > +	union tpmu_capabilities data;
> > +} __packed;
> > +
> >  /**
> >   * TPM2 Structure Tags for command/response buffers.
> >   *
> > @@ -123,11 +190,13 @@ enum tpm2_return_codes {
> >   * TPM2 algorithms.
> >   */
> >  enum tpm2_algorithms {
> > +	TPM2_ALG_SHA1		= 0x04,
> >  	TPM2_ALG_XOR		= 0x0A,
> >  	TPM2_ALG_SHA256		= 0x0B,
> >  	TPM2_ALG_SHA384		= 0x0C,
> >  	TPM2_ALG_SHA512		= 0x0D,
> >  	TPM2_ALG_NULL		= 0x10,
> > +	TPM2_ALG_SM3_256	= 0x12,
> >  };
> 
> In the spec these values are not an enum (i.e. 32 bit values if you do
> not use short enums) but explicitly u16/TPM2_ALG_ID. But probably that
> does not make a difference.

I just extended what was already there. 
I agree we are better off changing it, not on this series though.

Regards
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >
> >  /* NV index attributes */
> >
> 

[PATCH 2/3 v2] tpm: Add some headers from the spec

[Ilias Apalodimas]

Hi Heinrich, 

[...]
> 
> > > + */
> > > +#define TPM2_NUM_PCR_BANKS 16
> > > +
> > > +/* Definition of (UINT32) TPM2_CAP Constants */
> > > +#define TPM2_CAP_PCRS 0x00000005U
> > > +#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
> > > +
> > > +/* Definition of (UINT32) TPM2_PT Constants */
> > > +#define PT_GROUP                   (u32)(0x00000100)
> > > +#define PT_FIXED                   (u32)(PT_GROUP * 1)
> > > +#define TPM2_PT_MANUFACTURER        (u32)(PT_FIXED + 5)
> > > +#define TPM2_PT_PCR_COUNT           (u32)(PT_FIXED + 18)
> > > +#define TPM2_PT_MAX_COMMAND_SIZE    (u32)(PT_FIXED + 30)
> > > +#define TPM2_PT_MAX_RESPONSE_SIZE   (u32)(PT_FIXED + 31)
> > 
> > All these definitions are all copied from the "TCG TSS2.0 Overview and
> > Common Structures Specification". I am missing a reference to the
> > copyright notice of the spec. I think the best thing to do would be
> > placing the TCG copyrighted code into a separate include that is
> > included in tpm_v2.h. Please, check with Tom if the license contradicts
> > GPL. Especially the following sentence seems problematic:
> > 
> > "THE COPYRIGHT LICENSES SET FORTH ABOVE DO NOT REPRESENT ANY FORM OF
> > LICENSE OR WAIVER, EXPRESS OR IMPLIED, BY ESTOPPEL OR OTHERWISE, WITH
> > RESPECT TO PATENT RIGHTS HELD BY TCG MEMBERS (OR OTHER THIRD PARTIES)
> > THAT MAY BE NECESSARY TO IMPLEMENT THIS SPECIFICATION OR OTHERWISE."
> > 
> > Cf. https://fedoraproject.org/wiki/Licensing/TCGL
> > 
> 
> Ok will do

So I talked to Tom and he suggested we have a look at linux, or any other project
that uses those. I can't find any copyright claims in include/linux/tpm.h, apart
from a pointer to the spec.
I don't think splitting the changes to a new file is a good idea. Most of the 
existing definitions of the file are part of the same document. Maybe just 
updating the copyright properly is the right thing to do?

[...]

Regards
/Ilias

[PATCH 3/3 v2] efi: Add basic EFI_TCG2_PROTOCOL support

[Ilias Apalodimas]

Since U-boot EFI implementation is getting richer it makes sense to
add support for EFI_TCG2_PROTOCOL taking advantage of any hardware TPM
available on the device.

This is the initial implementation of the protocol which only adds
support for GetCapability(). It's limited in the newer and safer
TPMv2 devices.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
changes since v1: 
- change return variable of platform_get_tpm2_device() when used
- since more headers were included in patch #2 use them in offset 
  calculations for all tpm commands
- change the size of the response buffer regardless of what 
  tpm2_get_capability() is doing
 include/efi_loader.h       |   2 +
 include/efi_tcg2.h         |  91 +++++++
 lib/efi_loader/Kconfig     |   8 +
 lib/efi_loader/Makefile    |   1 +
 lib/efi_loader/efi_setup.c |   7 +
 lib/efi_loader/efi_tcg2.c  | 493 +++++++++++++++++++++++++++++++++++++
 6 files changed, 602 insertions(+)
 create mode 100644 include/efi_tcg2.h
 create mode 100644 lib/efi_loader/efi_tcg2.c

diff --git a/include/efi_loader.h b/include/efi_loader.h
index f550ced56876..e5015d865ec9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -405,6 +405,8 @@ efi_status_t efi_console_register(void);
 efi_status_t efi_disk_register(void);
 /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
 efi_status_t efi_rng_register(void);
+/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
+efi_status_t efi_tcg2_register(void);
 /* Create handles and protocols for the partitions of a block device */
 int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
 			       const char *if_typename, int diskid,
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
new file mode 100644
index 000000000000..9e7b85db058d
--- /dev/null
+++ b/include/efi_tcg2.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_TCG2_PROTOCOL_H_
+#define _EFI_TCG2_PROTOCOL_H_
+
+#include <tpm-v2.h>
+
+#define EFI_TCG2_PROTOCOL_GUID \
+	EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \
+		 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
+
+/* TPMV2 only */
+#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
+
+/* SHA1, SHA256, SHA384, SHA512, TPM_ALG_SM3_256 */
+#define MAX_HASH_COUNT 5
+/* Algorithm Registry */
+#define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
+#define EFI_TCG2_BOOT_HASH_ALG_SHA256  0x00000002
+#define EFI_TCG2_BOOT_HASH_ALG_SHA384  0x00000004
+#define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x00000008
+#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
+
+typedef u32 efi_tcg_event_log_bitmap;
+typedef u32 efi_tcg_event_log_format;
+typedef u32 efi_tcg_event_algorithm_bitmap;
+
+struct efi_tcg2_version {
+	u8 major;
+	u8 minor;
+};
+
+struct efi_tcg2_event_header {
+	u32 header_size;
+	u16 header_version;
+	u32 pcr_index;
+	u32 event_type;
+} __packed;
+
+struct efi_tcg2_event {
+	u32 size;
+	struct efi_tcg2_event_header header;
+	u8 event[];
+} __packed;
+
+struct efi_tcg2_boot_service_capability {
+	u8 size;
+	struct efi_tcg2_version structure_version;
+	struct efi_tcg2_version protocol_version;
+	efi_tcg_event_algorithm_bitmap hash_algorithm_bitmap;
+	efi_tcg_event_log_bitmap supported_event_logs;
+	bool tpm_present_flag;
+	u16 max_command_size;
+	u16 max_response_size;
+	u32 manufacturer_id;
+	u32 number_of_pcr_banks;
+	efi_tcg_event_algorithm_bitmap active_pcr_banks;
+};
+
+#define boot_service_capability_min \
+	sizeof(struct efi_tcg2_boot_service_capability) - \
+	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
+
+struct efi_tcg2_protocol {
+	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
+					       struct efi_tcg2_boot_service_capability *capability);
+	efi_status_t (EFIAPI * get_eventlog)(struct efi_tcg2_protocol *this,
+					     efi_tcg_event_log_format log_format,
+					     u64 *event_log_location, u64 *event_log_last_entry,
+					     bool *event_log_truncated);
+	efi_status_t (EFIAPI * hash_log_extend_event)(struct efi_tcg2_protocol *this,
+						      u64 flags, u64 data_to_hash,
+						      u64 data_to_hash_len,
+						      struct efi_tcg2_event *efi_tcg_event);
+	efi_status_t (EFIAPI * submit_command)(struct efi_tcg2_protocol *this,
+					       u32 input_parameter_block_size,
+					       u8 *input_parameter_block,
+					       u32 output_parameter_block_size,
+					       u8 *output_parameter_block);
+	efi_status_t (EFIAPI * get_active_pcr_banks)(struct efi_tcg2_protocol *this,
+						     u32 *active_pcr_banks);
+	efi_status_t (EFIAPI * set_active_pcr_banks)(struct efi_tcg2_protocol *this,
+						     u32 active_pcr_banks);
+	efi_status_t (EFIAPI * get_result_of_set_active_pcr_banks)(struct efi_tcg2_protocol *this,
+								   u32 *operation_present,
+								   u32 *response);
+};
+#endif
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 075481428cdf..5d5074a4bc41 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -184,6 +184,14 @@ config EFI_RNG_PROTOCOL
 	  Provide a EFI_RNG_PROTOCOL implementation using the hardware random
 	  number generator of the platform.
 
+config EFI_TCG2_PROTOCOL
+	bool "EFI_TCG2_PROTOCOL support"
+	depends on TPM_V2
+	default n
+	help
+	  Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware
+	  of the platform.
+
 config EFI_LOAD_FILE2_INITRD
 	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
 	default n
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 8892fb01e125..cd4b252a417c 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_NET) += efi_net.o
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
 obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
+obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
 obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
 obj-y += efi_signature.o
 
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 45226c5c1a53..e206b60bb82c 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -156,6 +156,13 @@ efi_status_t efi_init_obj_list(void)
 		if (ret != EFI_SUCCESS)
 			goto out;
 	}
+
+	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+		ret = efi_tcg2_register();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	/* Initialize variable services */
 	ret = efi_init_variables();
 	if (ret != EFI_SUCCESS)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
new file mode 100644
index 000000000000..b735f3f2a83d
--- /dev/null
+++ b/lib/efi_loader/efi_tcg2.c
@@ -0,0 +1,493 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+#include <common.h>
+#include <dm.h>
+#include <efi_loader.h>
+#include <efi_tcg2.h>
+#include <log.h>
+#include <tpm-v2.h>
+#include <linux/unaligned/access_ok.h>
+#include <linux/unaligned/generic.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
+ * Since the current tpm2_get_capability() response buffers starts at
+ * 'union tpmu_capabilities data' of 'struct tpms_capability_data', calculate the
+ * response size and offset once for all consumers
+ */
+#define TPM2_RESPONSE_BUFFER_SIZE (sizeof(struct tpms_capability_data) - \
+				   offsetof(struct tpms_capability_data, data))
+#define properties_offset (offsetof(struct tpml_tagged_tpm_property, tpm_property) + \
+			   offsetof(struct tpms_tagged_property, value))
+
+const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
+
+/**
+ * platform_get_tpm_device() - retrieve TPM device
+ *
+ * This function retrieves the udevice implementing a TPM
+ *
+ * This function may be overridden if special initialization is needed.
+ *
+ * @dev:	udevice
+ * Return:	status code
+ */
+__weak efi_status_t platform_get_tpm2_device(struct udevice **dev)
+{
+	int ret;
+	struct udevice *devp;
+
+	ret = uclass_get_device(UCLASS_TPM, 0, &devp);
+	if (ret)
+		return EFI_DEVICE_ERROR;
+
+	*dev = devp;
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * tpm2_get_max_command_size() - TPM2 command to get the supported max command size
+ *
+ * @dev:		TPM device
+ * @max_command_size:	output buffer for the size
+ *
+ * Return: 0 on success -1 on error
+ */
+static int tpm2_get_max_command_size(struct udevice *dev, u16 *max_command_size)
+{
+	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
+	u32 ret;
+
+	memset(response, 0, sizeof(response));
+	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+				  TPM2_PT_MAX_COMMAND_SIZE, response, 1);
+	if (ret)
+		return -1;
+
+	*max_command_size = (uint16_t)get_unaligned_be32(response + properties_offset);
+
+	return 0;
+}
+
+/**
+ * tpm2_get_max_response_size() - TPM2 command to get the supported max response size
+ *
+ * @dev:		TPM device
+ * @max_response_size:	output buffer for the size
+ *
+ * Return: 0 on success -1 on error
+ */
+static int tpm2_get_max_response_size(struct udevice *dev, u16 *max_response_size)
+{
+	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
+	u32 ret;
+
+	memset(response, 0, sizeof(response));
+	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+				  TPM2_PT_MAX_RESPONSE_SIZE, response, 1);
+	if (ret)
+		return -1;
+
+	*max_response_size = (uint16_t)get_unaligned_be32(response + properties_offset);
+
+	return 0;
+}
+
+/**
+ * tpm2_get_manufacturer_id() - Issue a TPM2 command to get the manufacturer ID
+ *
+ * @dev:		TPM device
+ * @manufacturer_id:	output buffer for the id
+ *
+ * Return: 0 on success -1 on error
+ */
+static int tpm2_get_manufacturer_id(struct udevice *dev, u32 *manufacturer_id)
+{
+	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
+	u32 ret;
+
+	memset(response, 0, sizeof(response));
+	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+				  TPM2_PT_MANUFACTURER, response, 1);
+	if (ret)
+		return -1;
+
+	*manufacturer_id = get_unaligned_be32(response + properties_offset);
+
+	return 0;
+}
+
+/**
+ * tpm2_get_num_pcr() - Issue a TPM2 command to get the number of PCRs
+ *
+ * @dev:		TPM device
+ * @manufacturer_id:	output buffer for the number
+ *
+ * Return: 0 on success -1 on error
+ */
+static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
+{
+	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
+	u32 ret;
+
+	memset(response, 0, sizeof(response));
+	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+				  TPM2_PT_PCR_COUNT, response, 1);
+	if (ret)
+		return -1;
+
+	*num_pcr = get_unaligned_be32(response + properties_offset);
+
+	return 0;
+}
+
+/**
+ * is_active_pcr() - Check if a supported algorithm is active
+ *
+ * @dev:		TPM device
+ * @selection:		struct of PCR information
+ *
+ * Return: true if PCR is active
+ */
+bool is_active_pcr(struct tpms_pcr_selection *selection)
+{
+	int i;
+	/*
+	 * check the pcr_select. If at least one of the PCRs supports the algorithm
+	 * add it on the active ones
+	 */
+	for (i = 0; i < selection->size_of_select; i++) {
+		if (selection->pcr_select[i])
+			return true;
+	}
+
+	return false;
+}
+
+/**
+ * tpm2_get_pcr_info() - Issue a TPM2 command to get the supported, active PCRs and number of banks
+ *
+ * @dev:		TPM device
+ * @supported_pcr:	bitmask with the algorithms supported
+ * @active_pcr:		bitmask with the active algorithms
+ * @pcr_banks:		number of PCR banks
+ *
+ * Return: 0 on success -1 on error
+ */
+static int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
+			     u32 *pcr_banks)
+{
+	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
+	struct tpml_pcr_selection pcrs;
+	u32 ret, num_pcr;
+	int i, tpm_ret;
+
+	memset(response, 0, sizeof(response));
+	ret = tpm2_get_capability(dev, TPM2_CAP_PCRS, 0, response, 1);
+	if (ret)
+		return -1;
+
+	pcrs.count = get_unaligned_be32(response);
+	if (pcrs.count > MAX_HASH_COUNT || pcrs.count < 1)
+		return -1;
+
+	tpm_ret = tpm2_get_num_pcr(dev, &num_pcr);
+	if (tpm_ret)
+		return -1;
+	for (i = 0; i < pcrs.count; i++) {
+		/*
+		 * Definition of TPMS_PCR_SELECTION Structure
+		 * hash: u16
+		 * size_of_select: u8
+		 * pcr_select: u8 array
+		 *
+		 * The offsets depend on the number of the device PCRs
+		 * so we have to calculate them based on that
+		 */
+		u32 hash_offset = offsetof(struct tpml_pcr_selection, selection) +
+			i * offsetof(struct tpms_pcr_selection, pcr_select) +
+			i * ((num_pcr + 7) / 8);
+		u32 size_select_offset =
+			hash_offset + offsetof(struct tpms_pcr_selection, size_of_select);
+		u32 pcr_select_offset =
+			hash_offset + offsetof(struct tpms_pcr_selection, pcr_select);
+
+		pcrs.selection[i].hash = get_unaligned_be16(response + hash_offset);
+		pcrs.selection[i].size_of_select =
+			__get_unaligned_be(response + size_select_offset);
+		/* copy the array of pcr_select */
+		memcpy(pcrs.selection[i].pcr_select, response + pcr_select_offset,
+		       pcrs.selection[i].size_of_select);
+	}
+
+	for (i = 0; i < pcrs.count; i++) {
+		switch (pcrs.selection[i].hash) {
+		case TPM2_ALG_SHA1:
+			 *supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
+			if (is_active_pcr(&pcrs.selection[i]))
+				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
+			break;
+		case TPM2_ALG_SHA256:
+			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
+			if (is_active_pcr(&pcrs.selection[i]))
+				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
+			break;
+		case TPM2_ALG_SHA384:
+			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
+			if (is_active_pcr(&pcrs.selection[i]))
+				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
+			break;
+		case TPM2_ALG_SHA512:
+			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
+			if (is_active_pcr(&pcrs.selection[i]))
+				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
+			break;
+		case TPM2_ALG_SM3_256:
+			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
+			if (is_active_pcr(&pcrs.selection[i]))
+				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
+			break;
+		default:
+			EFI_PRINT("Unknown algorithm %x\n", pcrs.selection[i].hash);
+			break;
+		}
+	}
+
+	*pcr_banks = pcrs.count;
+
+	return 0;
+}
+
+/**
+ * get_capability() - provide protocol capability information and state information
+ *
+ * @this:		TCG2 protocol instance
+ * @capability:		caller allocated memory with size field to the size of the
+ *			structure allocated
+
+ * Return:	status code
+ */
+static efi_status_t EFIAPI
+get_capability(struct efi_tcg2_protocol *this,
+	       struct efi_tcg2_boot_service_capability *capability)
+{
+	struct udevice *dev;
+	efi_status_t efi_ret;
+	int ret;
+
+	EFI_ENTRY("%p, %p", this, capability);
+
+	if (!this || !capability)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	if (capability->size < boot_service_capability_min) {
+		capability->size = boot_service_capability_min;
+		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+	}
+
+	if (capability->size < sizeof(*capability)) {
+		capability->size = sizeof(*capability);
+		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+	}
+
+	capability->structure_version.major = 1;
+	capability->structure_version.minor = 1;
+	capability->protocol_version.major = 1;
+	capability->protocol_version.minor = 1;
+
+	efi_ret = platform_get_tpm2_device(&dev);
+	if (efi_ret != EFI_SUCCESS) {
+		capability->supported_event_logs = 0;
+		capability->hash_algorithm_bitmap = 0;
+		capability->tpm_present_flag = false;
+		capability->max_command_size = 0;
+		capability->max_response_size = 0;
+		capability->manufacturer_id = 0;
+		capability->number_of_pcr_banks = 0;
+		capability->active_pcr_banks = 0;
+
+		return EFI_EXIT(EFI_SUCCESS);
+	}
+
+	/* We only allow a TPMv2 device to register the EFI protocol */
+	capability->supported_event_logs = TCG2_EVENT_LOG_FORMAT_TCG_2;
+
+	capability->tpm_present_flag = true;
+
+	/* Supported and active PCRs */
+	capability->hash_algorithm_bitmap = 0;
+	capability->active_pcr_banks = 0;
+	ret = tpm2_get_pcr_info(dev, &capability->hash_algorithm_bitmap,
+				&capability->active_pcr_banks,
+				&capability->number_of_pcr_banks);
+	if (ret)
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+
+	/* Max command size */
+	ret = tpm2_get_max_command_size(dev, &capability->max_command_size);
+	if (ret)
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+
+	/* Max response size */
+	ret = tpm2_get_max_response_size(dev, &capability->max_response_size);
+	if (ret)
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+
+	/* Manufacturer ID */
+	ret = tpm2_get_manufacturer_id(dev, &capability->manufacturer_id);
+	if (ret)
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+
+	return EFI_EXIT(EFI_SUCCESS);
+}
+
+/**
+ * get_eventlog() - retrieve the the address of a given event log and its last entry.
+ *
+ * @this:			TCG2 protocol instance
+ * @log_format:			type of event log format
+ * @event_log_location:		pointer to the memory address of the event log
+ * @event_log_last_entry:	pointer to the address of the start of the last entry in the
+ *				event log in memory, if log contains more than 1 entry
+ * @event_log_truncated:	set to true, if the Event Log is missing at least one entry
+ *
+ * Return:	status code
+ */
+static efi_status_t EFIAPI
+get_eventlog(struct efi_tcg2_protocol *this,
+	     efi_tcg_event_log_format log_format, u64 *event_log_location,
+	     u64 *event_log_last_entry, bool *event_log_truncated)
+{
+	return EFI_UNSUPPORTED;
+}
+
+/**
+ * hash_log_extend_event()- extend and optionally log events
+ *
+ * @this:			TCG2 protocol instance
+ * @flags:			bitmap providing additional information on the operation
+ * @data_to_hash:		physical address of the start of the data buffer to be hashed
+ * @data_to_hash_len:		the length in bytes of the buffer referenced by data_to_hash
+ * @efi_tcg_event:		pointer to data buffer containing information about the event
+ *
+ * Return:	status code
+ */
+static efi_status_t EFIAPI
+hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
+		      u64 data_to_hash, u64 data_to_hash_len,
+		      struct efi_tcg2_event *efi_tcg_event)
+{
+	return EFI_UNSUPPORTED;
+}
+
+/**
+ * submit_command() - Send command to the TPM
+ *
+ * @this:			TCG2 protocol instance
+ * @input_param_block_size:	size of the TPM input parameter block
+ * @input_param_block:		pointer to the TPM input parameter block
+ * @output_param_block_size:	size of the TPM output parameter block
+ * @output_param_block:		pointer to the TPM output parameter block
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI
+submit_command(struct efi_tcg2_protocol *this, u32 input_param_block_size,
+	       u8 *input_param_block, u32 output_param_block_size,
+	       u8 *output_param_block)
+{
+	return EFI_UNSUPPORTED;
+}
+
+/**
+ * get_active_pcr_banks() - returns the currently active PCR banks
+ *
+ * @this:			TCG2 protocol instance
+ * @active_pcr_banks:		pointer for receiving the bitmap of currently active PCR banks
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI
+get_active_pcr_banks(struct efi_tcg2_protocol *this, u32 *active_pcr_banks)
+{
+	return EFI_UNSUPPORTED;
+}
+
+/**
+ * set_active_pcr_banks() - sets the currently active PCR banks
+ *
+ * @this:			TCG2 protocol instance
+ * @active_pcr_banks:		bitmap of the requested active PCR banks
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI
+set_active_pcr_banks(struct efi_tcg2_protocol *this, u32 active_pcr_banks)
+{
+	return EFI_UNSUPPORTED;
+}
+
+/**
+ * get_result_of_set_active_pcr_banks() - retrieves the result of a previous set_active_pcr_banks()
+ *
+ * @this:			TCG2 protocol instance
+ * @operation_present:		non-zero value to indicate a set_active_pcr_banks operation was
+ *				invoked during last boot
+ * @response:			result value could be returned
+ *
+ * Return:	status code
+ */
+efi_status_t EFIAPI
+get_result_of_set_active_pcr_banks(struct efi_tcg2_protocol *this,
+				   u32 *operation_present, u32 *response)
+{
+	return EFI_UNSUPPORTED;
+}
+
+static const struct efi_tcg2_protocol efi_tcg2_protocol = {
+	.get_capability = get_capability,
+	.get_eventlog = get_eventlog,
+	.hash_log_extend_event = hash_log_extend_event,
+	.submit_command = submit_command,
+	.get_active_pcr_banks = get_active_pcr_banks,
+	.set_active_pcr_banks = set_active_pcr_banks,
+	.get_result_of_set_active_pcr_banks = get_result_of_set_active_pcr_banks,
+};
+
+/**
+ * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
+ *
+ * If a TPM2 device is available, the TPM TCG2 Protocol is registered
+ *
+ * Return:	An error status is only returned if adding the protocol fails.
+ */
+efi_status_t efi_tcg2_register(void)
+{
+	efi_status_t ret;
+	struct udevice *dev;
+	enum tpm_version tpm_ver;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		return EFI_SUCCESS;
+
+	tpm_ver = tpm_get_version(dev);
+	if (tpm_ver != TPM_V2) {
+		log_warning("Only TPMv2 supported for EFI_TCG2_PROTOCOL");
+		return EFI_SUCCESS;
+	}
+
+	ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
+			       (void *)&efi_tcg2_protocol);
+	if (ret != EFI_SUCCESS)
+		log_err("Cannot install EFI_TCG2_PROTOCOL");
+
+	return ret;
+}
-- 
2.29.2

[PATCH 3/3 v2] efi: Add basic EFI_TCG2_PROTOCOL support

[Heinrich Schuchardt]

On 11/5/20 10:58 PM, Ilias Apalodimas wrote:
> Since U-boot EFI implementation is getting richer it makes sense to
> add support for EFI_TCG2_PROTOCOL taking advantage of any hardware TPM
> available on the device.
>
> This is the initial implementation of the protocol which only adds
> support for GetCapability(). It's limited in the newer and safer
> TPMv2 devices.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> changes since v1:
> - change return variable of platform_get_tpm2_device() when used
> - since more headers were included in patch #2 use them in offset
>   calculations for all tpm commands
> - change the size of the response buffer regardless of what
>   tpm2_get_capability() is doing
>  include/efi_loader.h       |   2 +
>  include/efi_tcg2.h         |  91 +++++++
>  lib/efi_loader/Kconfig     |   8 +
>  lib/efi_loader/Makefile    |   1 +
>  lib/efi_loader/efi_setup.c |   7 +
>  lib/efi_loader/efi_tcg2.c  | 493 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 602 insertions(+)
>  create mode 100644 include/efi_tcg2.h
>  create mode 100644 lib/efi_loader/efi_tcg2.c
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f550ced56876..e5015d865ec9 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -405,6 +405,8 @@ efi_status_t efi_console_register(void);
>  efi_status_t efi_disk_register(void);
>  /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
>  efi_status_t efi_rng_register(void);
> +/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
> +efi_status_t efi_tcg2_register(void);
>  /* Create handles and protocols for the partitions of a block device */
>  int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
>  			       const char *if_typename, int diskid,
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> new file mode 100644
> index 000000000000..9e7b85db058d
> --- /dev/null
> +++ b/include/efi_tcg2.h
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited

Please, add at least one line describing what this include is about.

> + */
> +
> +#if !defined _EFI_TCG2_PROTOCOL_H_
> +#define _EFI_TCG2_PROTOCOL_H_
> +
> +#include <tpm-v2.h>
> +
> +#define EFI_TCG2_PROTOCOL_GUID \
> +	EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \
> +		 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
> +
> +/* TPMV2 only */
> +#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
> +
> +/* SHA1, SHA256, SHA384, SHA512, TPM_ALG_SM3_256 */
> +#define MAX_HASH_COUNT 5
> +/* Algorithm Registry */
> +#define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
> +#define EFI_TCG2_BOOT_HASH_ALG_SHA256  0x00000002
> +#define EFI_TCG2_BOOT_HASH_ALG_SHA384  0x00000004
> +#define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x00000008
> +#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
> +
> +typedef u32 efi_tcg_event_log_bitmap;
> +typedef u32 efi_tcg_event_log_format;
> +typedef u32 efi_tcg_event_algorithm_bitmap;
> +
> +struct efi_tcg2_version {
> +	u8 major;
> +	u8 minor;
> +};
> +
> +struct efi_tcg2_event_header {
> +	u32 header_size;
> +	u16 header_version;
> +	u32 pcr_index;
> +	u32 event_type;
> +} __packed;
> +
> +struct efi_tcg2_event {
> +	u32 size;
> +	struct efi_tcg2_event_header header;
> +	u8 event[];
> +} __packed;
> +
> +struct efi_tcg2_boot_service_capability {
> +	u8 size;
> +	struct efi_tcg2_version structure_version;
> +	struct efi_tcg2_version protocol_version;
> +	efi_tcg_event_algorithm_bitmap hash_algorithm_bitmap;
> +	efi_tcg_event_log_bitmap supported_event_logs;
> +	bool tpm_present_flag;

The UEFI spec defines BOOLEAN as "1-byte value".
The C99 standard does not require that _Bool has size 1 byte.
Wouldn't it be wiser to use u8?

Other structures in this patch are packed. Has this structure to be
packed too? The EFI ProtocolSpecification has this sentence:
"All structures defined in this specification are packed, except where
explicitly otherwise defined."

> +	u16 max_command_size;
> +	u16 max_response_size;
> +	u32 manufacturer_id;
> +	u32 number_of_pcr_banks;
> +	efi_tcg_event_algorithm_bitmap active_pcr_banks;
> +};
> +
> +#define boot_service_capability_min \
> +	sizeof(struct efi_tcg2_boot_service_capability) - \
> +	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
> +
> +struct efi_tcg2_protocol {
> +	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
> +					       struct efi_tcg2_boot_service_capability *capability);
> +	efi_status_t (EFIAPI * get_eventlog)(struct efi_tcg2_protocol *this,
> +					     efi_tcg_event_log_format log_format,
> +					     u64 *event_log_location, u64 *event_log_last_entry,
> +					     bool *event_log_truncated);
> +	efi_status_t (EFIAPI * hash_log_extend_event)(struct efi_tcg2_protocol *this,
> +						      u64 flags, u64 data_to_hash,
> +						      u64 data_to_hash_len,
> +						      struct efi_tcg2_event *efi_tcg_event);
> +	efi_status_t (EFIAPI * submit_command)(struct efi_tcg2_protocol *this,
> +					       u32 input_parameter_block_size,
> +					       u8 *input_parameter_block,
> +					       u32 output_parameter_block_size,
> +					       u8 *output_parameter_block);
> +	efi_status_t (EFIAPI * get_active_pcr_banks)(struct efi_tcg2_protocol *this,
> +						     u32 *active_pcr_banks);
> +	efi_status_t (EFIAPI * set_active_pcr_banks)(struct efi_tcg2_protocol *this,
> +						     u32 active_pcr_banks);
> +	efi_status_t (EFIAPI * get_result_of_set_active_pcr_banks)(struct efi_tcg2_protocol *this,
> +								   u32 *operation_present,
> +								   u32 *response);
> +};
> +#endif
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 075481428cdf..5d5074a4bc41 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -184,6 +184,14 @@ config EFI_RNG_PROTOCOL
>  	  Provide a EFI_RNG_PROTOCOL implementation using the hardware random
>  	  number generator of the platform.
>
> +config EFI_TCG2_PROTOCOL
> +	bool "EFI_TCG2_PROTOCOL support"
> +	depends on TPM_V2
> +	default n

default n is superfluous.

> +	help
> +	  Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware
> +	  of the platform.
> +
>  config EFI_LOAD_FILE2_INITRD
>  	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
>  	default n
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 8892fb01e125..cd4b252a417c 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_NET) += efi_net.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> +obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
>  obj-y += efi_signature.o
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 45226c5c1a53..e206b60bb82c 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -156,6 +156,13 @@ efi_status_t efi_init_obj_list(void)
>  		if (ret != EFI_SUCCESS)
>  			goto out;
>  	}
> +
> +	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> +		ret = efi_tcg2_register();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>  	/* Initialize variable services */
>  	ret = efi_init_variables();
>  	if (ret != EFI_SUCCESS)
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> new file mode 100644
> index 000000000000..b735f3f2a83d
> --- /dev/null
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -0,0 +1,493 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited

Please, add at least one line describing what this module is about.

> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +#include <common.h>
> +#include <dm.h>
> +#include <efi_loader.h>
> +#include <efi_tcg2.h>
> +#include <log.h>
> +#include <tpm-v2.h>
> +#include <linux/unaligned/access_ok.h>
> +#include <linux/unaligned/generic.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
> + * Since the current tpm2_get_capability() response buffers starts at
> + * 'union tpmu_capabilities data' of 'struct tpms_capability_data', calculate the
> + * response size and offset once for all consumers

Please, restrict your code to 80 characters per line.

> + */
> +#define TPM2_RESPONSE_BUFFER_SIZE (sizeof(struct tpms_capability_data) - \
> +				   offsetof(struct tpms_capability_data, data))
> +#define properties_offset (offsetof(struct tpml_tagged_tpm_property, tpm_property) + \
> +			   offsetof(struct tpms_tagged_property, value))
> +
> +const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
> +
> +/**
> + * platform_get_tpm_device() - retrieve TPM device
> + *
> + * This function retrieves the udevice implementing a TPM
> + *
> + * This function may be overridden if special initialization is needed.
> + *
> + * @dev:	udevice
> + * Return:	status code
> + */
> +__weak efi_status_t platform_get_tpm2_device(struct udevice **dev)
> +{
> +	int ret;
> +	struct udevice *devp;
> +
> +	ret = uclass_get_device(UCLASS_TPM, 0, &devp);
> +	if (ret)
> +		return EFI_DEVICE_ERROR;
> +
> +	*dev = devp;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * tpm2_get_max_command_size() - TPM2 command to get the supported max command size
> + *
> + * @dev:		TPM device
> + * @max_command_size:	output buffer for the size
> + *
> + * Return: 0 on success -1 on error
> + */
> +static int tpm2_get_max_command_size(struct udevice *dev, u16 *max_command_size)
> +{
> +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> +	u32 ret;
> +
> +	memset(response, 0, sizeof(response));

Why is memset() required here?
Do you want avoid the TPM chip to be able to see your bytes from the stack?
Wouldn't it preferable that tpm2_get_capability() takes care of zeroing
out buffers?

> +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> +				  TPM2_PT_MAX_COMMAND_SIZE, response, 1);

Here you pass TPM2_PT_MAX_COMMAND_SIZE but the buffer is of size
TPM2_RESPONSE_BUFFER_SIZE. How should tpm2_get_capability() guess the
size of the response?

> +	if (ret)
> +		return -1;
> +
> +	*max_command_size = (uint16_t)get_unaligned_be32(response + properties_offset);
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm2_get_max_response_size() - TPM2 command to get the supported max response size
> + *
> + * @dev:		TPM device
> + * @max_response_size:	output buffer for the size
> + *
> + * Return: 0 on success -1 on error
> + */
> +static int tpm2_get_max_response_size(struct udevice *dev, u16 *max_response_size)
> +{
> +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> +	u32 ret;
> +
> +	memset(response, 0, sizeof(response));

Same here. Moving memset to tpm2_get_capability already would save code
size.

> +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> +				  TPM2_PT_MAX_RESPONSE_SIZE, response, 1);
> +	if (ret)
> +		return -1;
> +
> +	*max_response_size = (uint16_t)get_unaligned_be32(response + properties_offset);
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm2_get_manufacturer_id() - Issue a TPM2 command to get the manufacturer ID
> + *
> + * @dev:		TPM device
> + * @manufacturer_id:	output buffer for the id
> + *
> + * Return: 0 on success -1 on error
> + */
> +static int tpm2_get_manufacturer_id(struct udevice *dev, u32 *manufacturer_id)
> +{
> +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> +	u32 ret;
> +
> +	memset(response, 0, sizeof(response));

Again

> +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> +				  TPM2_PT_MANUFACTURER, response, 1);
> +	if (ret)
> +		return -1;
> +
> +	*manufacturer_id = get_unaligned_be32(response + properties_offset);
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm2_get_num_pcr() - Issue a TPM2 command to get the number of PCRs
> + *
> + * @dev:		TPM device
> + * @manufacturer_id:	output buffer for the number
> + *
> + * Return: 0 on success -1 on error
> + */
> +static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
> +{
> +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> +	u32 ret;
> +
> +	memset(response, 0, sizeof(response));

Again

> +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> +				  TPM2_PT_PCR_COUNT, response, 1);
> +	if (ret)
> +		return -1;
> +
> +	*num_pcr = get_unaligned_be32(response + properties_offset);
> +
> +	return 0;
> +}
> +
> +/**
> + * is_active_pcr() - Check if a supported algorithm is active
> + *
> + * @dev:		TPM device
> + * @selection:		struct of PCR information
> + *
> + * Return: true if PCR is active
> + */
> +bool is_active_pcr(struct tpms_pcr_selection *selection)
> +{
> +	int i;
> +	/*
> +	 * check the pcr_select. If at least one of the PCRs supports the algorithm
> +	 * add it on the active ones
> +	 */
> +	for (i = 0; i < selection->size_of_select; i++) {
> +		if (selection->pcr_select[i])
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * tpm2_get_pcr_info() - Issue a TPM2 command to get the supported, active PCRs and number of banks
> + *
> + * @dev:		TPM device
> + * @supported_pcr:	bitmask with the algorithms supported
> + * @active_pcr:		bitmask with the active algorithms
> + * @pcr_banks:		number of PCR banks
> + *
> + * Return: 0 on success -1 on error

%s/ -1/, -1/

> + */
> +static int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
> +			     u32 *pcr_banks)
> +{
> +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> +	struct tpml_pcr_selection pcrs;
> +	u32 ret, num_pcr;
> +	int i, tpm_ret;
> +
> +	memset(response, 0, sizeof(response));

Again

> +	ret = tpm2_get_capability(dev, TPM2_CAP_PCRS, 0, response, 1);
> +	if (ret)
> +		return -1;
> +
> +	pcrs.count = get_unaligned_be32(response);
> +	if (pcrs.count > MAX_HASH_COUNT || pcrs.count < 1)
> +		return -1;
> +
> +	tpm_ret = tpm2_get_num_pcr(dev, &num_pcr);
> +	if (tpm_ret)
> +		return -1;
> +	for (i = 0; i < pcrs.count; i++) {
> +		/*
> +		 * Definition of TPMS_PCR_SELECTION Structure
> +		 * hash: u16
> +		 * size_of_select: u8
> +		 * pcr_select: u8 array
> +		 *
> +		 * The offsets depend on the number of the device PCRs
> +		 * so we have to calculate them based on that
> +		 */
> +		u32 hash_offset = offsetof(struct tpml_pcr_selection, selection) +
> +			i * offsetof(struct tpms_pcr_selection, pcr_select) +
> +			i * ((num_pcr + 7) / 8);
> +		u32 size_select_offset =
> +			hash_offset + offsetof(struct tpms_pcr_selection, size_of_select);
> +		u32 pcr_select_offset =
> +			hash_offset + offsetof(struct tpms_pcr_selection, pcr_select);
> +
> +		pcrs.selection[i].hash = get_unaligned_be16(response + hash_offset);
> +		pcrs.selection[i].size_of_select =
> +			__get_unaligned_be(response + size_select_offset);
> +		/* copy the array of pcr_select */
> +		memcpy(pcrs.selection[i].pcr_select, response + pcr_select_offset,
> +		       pcrs.selection[i].size_of_select);
> +	}
> +
> +	for (i = 0; i < pcrs.count; i++) {
> +		switch (pcrs.selection[i].hash) {
> +		case TPM2_ALG_SHA1:
> +			 *supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
> +			if (is_active_pcr(&pcrs.selection[i]))
> +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
> +			break;
> +		case TPM2_ALG_SHA256:
> +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
> +			if (is_active_pcr(&pcrs.selection[i]))
> +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
> +			break;
> +		case TPM2_ALG_SHA384:
> +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
> +			if (is_active_pcr(&pcrs.selection[i]))
> +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
> +			break;
> +		case TPM2_ALG_SHA512:
> +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
> +			if (is_active_pcr(&pcrs.selection[i]))
> +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
> +			break;
> +		case TPM2_ALG_SM3_256:
> +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
> +			if (is_active_pcr(&pcrs.selection[i]))
> +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
> +			break;
> +		default:
> +			EFI_PRINT("Unknown algorithm %x\n", pcrs.selection[i].hash);
> +			break;
> +		}
> +	}
> +
> +	*pcr_banks = pcrs.count;
> +
> +	return 0;
> +}
> +
> +/**
> + * get_capability() - provide protocol capability information and state information
> + *
> + * @this:		TCG2 protocol instance
> + * @capability:		caller allocated memory with size field to the size of the
> + *			structure allocated
> +
> + * Return:	status code
> + */
> +static efi_status_t EFIAPI
> +get_capability(struct efi_tcg2_protocol *this,
> +	       struct efi_tcg2_boot_service_capability *capability)
> +{
> +	struct udevice *dev;
> +	efi_status_t efi_ret;
> +	int ret;
> +
> +	EFI_ENTRY("%p, %p", this, capability);
> +
> +	if (!this || !capability)
> +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +	if (capability->size < boot_service_capability_min) {
> +		capability->size = boot_service_capability_min;
> +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +	}
> +
> +	if (capability->size < sizeof(*capability)) {
> +		capability->size = sizeof(*capability);
> +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +	}
> +
> +	capability->structure_version.major = 1;
> +	capability->structure_version.minor = 1;
> +	capability->protocol_version.major = 1;
> +	capability->protocol_version.minor = 1;
> +
> +	efi_ret = platform_get_tpm2_device(&dev);
> +	if (efi_ret != EFI_SUCCESS) {
> +		capability->supported_event_logs = 0;
> +		capability->hash_algorithm_bitmap = 0;
> +		capability->tpm_present_flag = false;
> +		capability->max_command_size = 0;
> +		capability->max_response_size = 0;
> +		capability->manufacturer_id = 0;
> +		capability->number_of_pcr_banks = 0;
> +		capability->active_pcr_banks = 0;
> +
> +		return EFI_EXIT(EFI_SUCCESS);

Instead of hoping that the compiler reduces the code size I would prefer
to use a common exit point where you call EFI_EXIT().

Best regards

Heinrich

> +	}
> +
> +	/* We only allow a TPMv2 device to register the EFI protocol */
> +	capability->supported_event_logs = TCG2_EVENT_LOG_FORMAT_TCG_2;
> +
> +	capability->tpm_present_flag = true;
> +
> +	/* Supported and active PCRs */
> +	capability->hash_algorithm_bitmap = 0;
> +	capability->active_pcr_banks = 0;
> +	ret = tpm2_get_pcr_info(dev, &capability->hash_algorithm_bitmap,
> +				&capability->active_pcr_banks,
> +				&capability->number_of_pcr_banks);
> +	if (ret)
> +		return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> +	/* Max command size */
> +	ret = tpm2_get_max_command_size(dev, &capability->max_command_size);
> +	if (ret)
> +		return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> +	/* Max response size */
> +	ret = tpm2_get_max_response_size(dev, &capability->max_response_size);
> +	if (ret)
> +		return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> +	/* Manufacturer ID */
> +	ret = tpm2_get_manufacturer_id(dev, &capability->manufacturer_id);
> +	if (ret)
> +		return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> +	return EFI_EXIT(EFI_SUCCESS);
> +}
> +
> +/**
> + * get_eventlog() - retrieve the the address of a given event log and its last entry.
> + *
> + * @this:			TCG2 protocol instance
> + * @log_format:			type of event log format
> + * @event_log_location:		pointer to the memory address of the event log
> + * @event_log_last_entry:	pointer to the address of the start of the last entry in the
> + *				event log in memory, if log contains more than 1 entry
> + * @event_log_truncated:	set to true, if the Event Log is missing at least one entry
> + *
> + * Return:	status code
> + */
> +static efi_status_t EFIAPI
> +get_eventlog(struct efi_tcg2_protocol *this,
> +	     efi_tcg_event_log_format log_format, u64 *event_log_location,
> +	     u64 *event_log_last_entry, bool *event_log_truncated)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * hash_log_extend_event()- extend and optionally log events
> + *
> + * @this:			TCG2 protocol instance
> + * @flags:			bitmap providing additional information on the operation
> + * @data_to_hash:		physical address of the start of the data buffer to be hashed
> + * @data_to_hash_len:		the length in bytes of the buffer referenced by data_to_hash
> + * @efi_tcg_event:		pointer to data buffer containing information about the event
> + *
> + * Return:	status code
> + */
> +static efi_status_t EFIAPI
> +hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
> +		      u64 data_to_hash, u64 data_to_hash_len,
> +		      struct efi_tcg2_event *efi_tcg_event)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * submit_command() - Send command to the TPM
> + *
> + * @this:			TCG2 protocol instance
> + * @input_param_block_size:	size of the TPM input parameter block
> + * @input_param_block:		pointer to the TPM input parameter block
> + * @output_param_block_size:	size of the TPM output parameter block
> + * @output_param_block:		pointer to the TPM output parameter block
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI
> +submit_command(struct efi_tcg2_protocol *this, u32 input_param_block_size,
> +	       u8 *input_param_block, u32 output_param_block_size,
> +	       u8 *output_param_block)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * get_active_pcr_banks() - returns the currently active PCR banks
> + *
> + * @this:			TCG2 protocol instance
> + * @active_pcr_banks:		pointer for receiving the bitmap of currently active PCR banks
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI
> +get_active_pcr_banks(struct efi_tcg2_protocol *this, u32 *active_pcr_banks)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * set_active_pcr_banks() - sets the currently active PCR banks
> + *
> + * @this:			TCG2 protocol instance
> + * @active_pcr_banks:		bitmap of the requested active PCR banks
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI
> +set_active_pcr_banks(struct efi_tcg2_protocol *this, u32 active_pcr_banks)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * get_result_of_set_active_pcr_banks() - retrieves the result of a previous set_active_pcr_banks()
> + *
> + * @this:			TCG2 protocol instance
> + * @operation_present:		non-zero value to indicate a set_active_pcr_banks operation was
> + *				invoked during last boot
> + * @response:			result value could be returned
> + *
> + * Return:	status code
> + */
> +efi_status_t EFIAPI
> +get_result_of_set_active_pcr_banks(struct efi_tcg2_protocol *this,
> +				   u32 *operation_present, u32 *response)
> +{
> +	return EFI_UNSUPPORTED;
> +}
> +
> +static const struct efi_tcg2_protocol efi_tcg2_protocol = {
> +	.get_capability = get_capability,
> +	.get_eventlog = get_eventlog,
> +	.hash_log_extend_event = hash_log_extend_event,
> +	.submit_command = submit_command,
> +	.get_active_pcr_banks = get_active_pcr_banks,
> +	.set_active_pcr_banks = set_active_pcr_banks,
> +	.get_result_of_set_active_pcr_banks = get_result_of_set_active_pcr_banks,
> +};
> +
> +/**
> + * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
> + *
> + * If a TPM2 device is available, the TPM TCG2 Protocol is registered
> + *
> + * Return:	An error status is only returned if adding the protocol fails.
> + */
> +efi_status_t efi_tcg2_register(void)
> +{
> +	efi_status_t ret;
> +	struct udevice *dev;
> +	enum tpm_version tpm_ver;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		return EFI_SUCCESS;
> +
> +	tpm_ver = tpm_get_version(dev);
> +	if (tpm_ver != TPM_V2) {
> +		log_warning("Only TPMv2 supported for EFI_TCG2_PROTOCOL");
> +		return EFI_SUCCESS;
> +	}
> +
> +	ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
> +			       (void *)&efi_tcg2_protocol);
> +	if (ret != EFI_SUCCESS)
> +		log_err("Cannot install EFI_TCG2_PROTOCOL");
> +
> +	return ret;
> +}
>

[PATCH 3/3 v2] efi: Add basic EFI_TCG2_PROTOCOL support

[Ilias Apalodimas]

Hi Heinrich, 

[...]
> > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> > new file mode 100644
> > index 000000000000..9e7b85db058d
> > --- /dev/null
> > +++ b/include/efi_tcg2.h
> > @@ -0,0 +1,91 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (c) 2020, Linaro Limited
> 
> Please, add at least one line describing what this include is about.
> 

Ok 

> > + */
> > +
> > +#if !defined _EFI_TCG2_PROTOCOL_H_
> > +#define _EFI_TCG2_PROTOCOL_H_
> > +
> > +#include <tpm-v2.h>
> > +
> > +#define EFI_TCG2_PROTOCOL_GUID \
> > +	EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \
> > +		 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
> > +
> > +/* TPMV2 only */
> > +#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
> > +
> > +/* SHA1, SHA256, SHA384, SHA512, TPM_ALG_SM3_256 */
> > +#define MAX_HASH_COUNT 5
> > +/* Algorithm Registry */
> > +#define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
> > +#define EFI_TCG2_BOOT_HASH_ALG_SHA256  0x00000002
> > +#define EFI_TCG2_BOOT_HASH_ALG_SHA384  0x00000004
> > +#define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x00000008
> > +#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
> > +
> > +typedef u32 efi_tcg_event_log_bitmap;
> > +typedef u32 efi_tcg_event_log_format;
> > +typedef u32 efi_tcg_event_algorithm_bitmap;
> > +
> > +struct efi_tcg2_version {
> > +	u8 major;
> > +	u8 minor;
> > +};
> > +
> > +struct efi_tcg2_event_header {
> > +	u32 header_size;
> > +	u16 header_version;
> > +	u32 pcr_index;
> > +	u32 event_type;
> > +} __packed;
> > +
> > +struct efi_tcg2_event {
> > +	u32 size;
> > +	struct efi_tcg2_event_header header;
> > +	u8 event[];
> > +} __packed;
> > +
> > +struct efi_tcg2_boot_service_capability {
> > +	u8 size;
> > +	struct efi_tcg2_version structure_version;
> > +	struct efi_tcg2_version protocol_version;
> > +	efi_tcg_event_algorithm_bitmap hash_algorithm_bitmap;
> > +	efi_tcg_event_log_bitmap supported_event_logs;
> > +	bool tpm_present_flag;
> 
> The UEFI spec defines BOOLEAN as "1-byte value".
> The C99 standard does not require that _Bool has size 1 byte.
> Wouldn't it be wiser to use u8?

Yes, good catch

> 
> Other structures in this patch are packed. Has this structure to be
> packed too? The EFI ProtocolSpecification has this sentence:
> "All structures defined in this specification are packed, except where
> explicitly otherwise defined."
> 

Nop this is expliticly required to be not packed 
https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
Section 6.4 

[...]
> >
> > +config EFI_TCG2_PROTOCOL
> > +	bool "EFI_TCG2_PROTOCOL support"
> > +	depends on TPM_V2
> > +	default n
> 
> default n is superfluous.
> 

Ok

> > +	help
> > +	  Provide a EFI_TCG2_PROTOCOL implementation using the TPM hardware
> > +	  of the platform.
> > +
> >  config EFI_LOAD_FILE2_INITRD
> >  	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> >  	default n
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 8892fb01e125..cd4b252a417c 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -53,6 +53,7 @@ obj-$(CONFIG_NET) += efi_net.o
> >  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> > +obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
> >  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> >  obj-y += efi_signature.o
> >
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 45226c5c1a53..e206b60bb82c 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -156,6 +156,13 @@ efi_status_t efi_init_obj_list(void)
> >  		if (ret != EFI_SUCCESS)
> >  			goto out;
> >  	}
> > +
> > +	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> > +		ret = efi_tcg2_register();
> > +		if (ret != EFI_SUCCESS)
> > +			goto out;
> > +	}
> > +
> >  	/* Initialize variable services */
> >  	ret = efi_init_variables();
> >  	if (ret != EFI_SUCCESS)
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > new file mode 100644
> > index 000000000000..b735f3f2a83d
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -0,0 +1,493 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2020, Linaro Limited
> 
> Please, add at least one line describing what this module is about.
> 

Ok

> > + */
> > +
> > +#define LOG_CATEGORY LOGC_EFI
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <efi_loader.h>
> > +#include <efi_tcg2.h>
> > +#include <log.h>
> > +#include <tpm-v2.h>
> > +#include <linux/unaligned/access_ok.h>
> > +#include <linux/unaligned/generic.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +/*
> > + * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
> > + * Since the current tpm2_get_capability() response buffers starts at
> > + * 'union tpmu_capabilities data' of 'struct tpms_capability_data', calculate the
> > + * response size and offset once for all consumers
> 
> Please, restrict your code to 80 characters per line.
> 

Ok

> > + */
> > +#define TPM2_RESPONSE_BUFFER_SIZE (sizeof(struct tpms_capability_data) - \
> > +				   offsetof(struct tpms_capability_data, data))
> > +#define properties_offset (offsetof(struct tpml_tagged_tpm_property, tpm_property) + \
> > +			   offsetof(struct tpms_tagged_property, value))
> > +
> > +const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
> > +
> > +/**
> > + * platform_get_tpm_device() - retrieve TPM device
> > + *
> > + * This function retrieves the udevice implementing a TPM
> > + *
> > + * This function may be overridden if special initialization is needed.
> > + *
> > + * @dev:	udevice
> > + * Return:	status code
> > + */
> > +__weak efi_status_t platform_get_tpm2_device(struct udevice **dev)
> > +{
> > +	int ret;
> > +	struct udevice *devp;
> > +
> > +	ret = uclass_get_device(UCLASS_TPM, 0, &devp);
> > +	if (ret)
> > +		return EFI_DEVICE_ERROR;
> > +
> > +	*dev = devp;
> > +
> > +	return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * tpm2_get_max_command_size() - TPM2 command to get the supported max command size
> > + *
> > + * @dev:		TPM device
> > + * @max_command_size:	output buffer for the size
> > + *
> > + * Return: 0 on success -1 on error
> > + */
> > +static int tpm2_get_max_command_size(struct udevice *dev, u16 *max_command_size)
> > +{
> > +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> > +	u32 ret;
> > +
> > +	memset(response, 0, sizeof(response));
> 
> Why is memset() required here?
> Do you want avoid the TPM chip to be able to see your bytes from the stack?
> Wouldn't it preferable that tpm2_get_capability() takes care of zeroing
> out buffers?

I generally zero out stack initialized arrays for various reasons. 
tpm2_get_capability only gets one argument (void *buf) not the length. 
Can we change that when specific tpm2_get_capability() patches are sent?

> 
> > +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> > +				  TPM2_PT_MAX_COMMAND_SIZE, response, 1);
> 
> Here you pass TPM2_PT_MAX_COMMAND_SIZE but the buffer is of size
> TPM2_RESPONSE_BUFFER_SIZE. How should tpm2_get_capability() guess the
> size of the response?

I am not sure I am following. This asks the TPMv2 to respond (in a response buffer), of
it's maximum allowed command size. The buffer size is defined from the standard.

#define TPM2_RESPONSE_BUFFER_SIZE (sizeof(struct tpms_capability_data) - \
                                   offsetof(struct tpms_capability_data, data))

> 
> > +	if (ret)
> > +		return -1;
> > +
> > +	*max_command_size = (uint16_t)get_unaligned_be32(response + properties_offset);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * tpm2_get_max_response_size() - TPM2 command to get the supported max response size
> > + *
> > + * @dev:		TPM device
> > + * @max_response_size:	output buffer for the size
> > + *
> > + * Return: 0 on success -1 on error
> > + */
> > +static int tpm2_get_max_response_size(struct udevice *dev, u16 *max_response_size)
> > +{
> > +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> > +	u32 ret;
> > +
> > +	memset(response, 0, sizeof(response));
> 
> Same here. Moving memset to tpm2_get_capability already would save code
> size.
> 
> > +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> > +				  TPM2_PT_MAX_RESPONSE_SIZE, response, 1);
> > +	if (ret)
> > +		return -1;
> > +
> > +	*max_response_size = (uint16_t)get_unaligned_be32(response + properties_offset);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * tpm2_get_manufacturer_id() - Issue a TPM2 command to get the manufacturer ID
> > + *
> > + * @dev:		TPM device
> > + * @manufacturer_id:	output buffer for the id
> > + *
> > + * Return: 0 on success -1 on error
> > + */
> > +static int tpm2_get_manufacturer_id(struct udevice *dev, u32 *manufacturer_id)
> > +{
> > +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> > +	u32 ret;
> > +
> > +	memset(response, 0, sizeof(response));
> 
> Again
> 
> > +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> > +				  TPM2_PT_MANUFACTURER, response, 1);
> > +	if (ret)
> > +		return -1;
> > +
> > +	*manufacturer_id = get_unaligned_be32(response + properties_offset);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * tpm2_get_num_pcr() - Issue a TPM2 command to get the number of PCRs
> > + *
> > + * @dev:		TPM device
> > + * @manufacturer_id:	output buffer for the number
> > + *
> > + * Return: 0 on success -1 on error
> > + */
> > +static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
> > +{
> > +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> > +	u32 ret;
> > +
> > +	memset(response, 0, sizeof(response));
> 
> Again
> 
> > +	ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> > +				  TPM2_PT_PCR_COUNT, response, 1);
> > +	if (ret)
> > +		return -1;
> > +
> > +	*num_pcr = get_unaligned_be32(response + properties_offset);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * is_active_pcr() - Check if a supported algorithm is active
> > + *
> > + * @dev:		TPM device
> > + * @selection:		struct of PCR information
> > + *
> > + * Return: true if PCR is active
> > + */
> > +bool is_active_pcr(struct tpms_pcr_selection *selection)
> > +{
> > +	int i;
> > +	/*
> > +	 * check the pcr_select. If at least one of the PCRs supports the algorithm
> > +	 * add it on the active ones
> > +	 */
> > +	for (i = 0; i < selection->size_of_select; i++) {
> > +		if (selection->pcr_select[i])
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +/**
> > + * tpm2_get_pcr_info() - Issue a TPM2 command to get the supported, active PCRs and number of banks
> > + *
> > + * @dev:		TPM device
> > + * @supported_pcr:	bitmask with the algorithms supported
> > + * @active_pcr:		bitmask with the active algorithms
> > + * @pcr_banks:		number of PCR banks
> > + *
> > + * Return: 0 on success -1 on error
> 
> %s/ -1/, -1/
> 

Ok 

> > + */
> > +static int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
> > +			     u32 *pcr_banks)
> > +{
> > +	u8 response[TPM2_RESPONSE_BUFFER_SIZE];
> > +	struct tpml_pcr_selection pcrs;
> > +	u32 ret, num_pcr;
> > +	int i, tpm_ret;
> > +
> > +	memset(response, 0, sizeof(response));
> 
> Again
> 
> > +	ret = tpm2_get_capability(dev, TPM2_CAP_PCRS, 0, response, 1);
> > +	if (ret)
> > +		return -1;
> > +
> > +	pcrs.count = get_unaligned_be32(response);
> > +	if (pcrs.count > MAX_HASH_COUNT || pcrs.count < 1)
> > +		return -1;
> > +
> > +	tpm_ret = tpm2_get_num_pcr(dev, &num_pcr);
> > +	if (tpm_ret)
> > +		return -1;
> > +	for (i = 0; i < pcrs.count; i++) {
> > +		/*
> > +		 * Definition of TPMS_PCR_SELECTION Structure
> > +		 * hash: u16
> > +		 * size_of_select: u8
> > +		 * pcr_select: u8 array
> > +		 *
> > +		 * The offsets depend on the number of the device PCRs
> > +		 * so we have to calculate them based on that
> > +		 */
> > +		u32 hash_offset = offsetof(struct tpml_pcr_selection, selection) +
> > +			i * offsetof(struct tpms_pcr_selection, pcr_select) +
> > +			i * ((num_pcr + 7) / 8);
> > +		u32 size_select_offset =
> > +			hash_offset + offsetof(struct tpms_pcr_selection, size_of_select);
> > +		u32 pcr_select_offset =
> > +			hash_offset + offsetof(struct tpms_pcr_selection, pcr_select);
> > +
> > +		pcrs.selection[i].hash = get_unaligned_be16(response + hash_offset);
> > +		pcrs.selection[i].size_of_select =
> > +			__get_unaligned_be(response + size_select_offset);
> > +		/* copy the array of pcr_select */
> > +		memcpy(pcrs.selection[i].pcr_select, response + pcr_select_offset,
> > +		       pcrs.selection[i].size_of_select);
> > +	}
> > +
> > +	for (i = 0; i < pcrs.count; i++) {
> > +		switch (pcrs.selection[i].hash) {
> > +		case TPM2_ALG_SHA1:
> > +			 *supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
> > +			if (is_active_pcr(&pcrs.selection[i]))
> > +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
> > +			break;
> > +		case TPM2_ALG_SHA256:
> > +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
> > +			if (is_active_pcr(&pcrs.selection[i]))
> > +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
> > +			break;
> > +		case TPM2_ALG_SHA384:
> > +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
> > +			if (is_active_pcr(&pcrs.selection[i]))
> > +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
> > +			break;
> > +		case TPM2_ALG_SHA512:
> > +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
> > +			if (is_active_pcr(&pcrs.selection[i]))
> > +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
> > +			break;
> > +		case TPM2_ALG_SM3_256:
> > +			*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
> > +			if (is_active_pcr(&pcrs.selection[i]))
> > +				*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
> > +			break;
> > +		default:
> > +			EFI_PRINT("Unknown algorithm %x\n", pcrs.selection[i].hash);
> > +			break;
> > +		}
> > +	}
> > +
> > +	*pcr_banks = pcrs.count;
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * get_capability() - provide protocol capability information and state information
> > + *
> > + * @this:		TCG2 protocol instance
> > + * @capability:		caller allocated memory with size field to the size of the
> > + *			structure allocated
> > +
> > + * Return:	status code
> > + */
> > +static efi_status_t EFIAPI
> > +get_capability(struct efi_tcg2_protocol *this,
> > +	       struct efi_tcg2_boot_service_capability *capability)
> > +{
> > +	struct udevice *dev;
> > +	efi_status_t efi_ret;
> > +	int ret;
> > +
> > +	EFI_ENTRY("%p, %p", this, capability);
> > +
> > +	if (!this || !capability)
> > +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +
> > +	if (capability->size < boot_service_capability_min) {
> > +		capability->size = boot_service_capability_min;
> > +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> > +	}
> > +
> > +	if (capability->size < sizeof(*capability)) {
> > +		capability->size = sizeof(*capability);
> > +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> > +	}
> > +
> > +	capability->structure_version.major = 1;
> > +	capability->structure_version.minor = 1;
> > +	capability->protocol_version.major = 1;
> > +	capability->protocol_version.minor = 1;
> > +
> > +	efi_ret = platform_get_tpm2_device(&dev);
> > +	if (efi_ret != EFI_SUCCESS) {
> > +		capability->supported_event_logs = 0;
> > +		capability->hash_algorithm_bitmap = 0;
> > +		capability->tpm_present_flag = false;
> > +		capability->max_command_size = 0;
> > +		capability->max_response_size = 0;
> > +		capability->manufacturer_id = 0;
> > +		capability->number_of_pcr_banks = 0;
> > +		capability->active_pcr_banks = 0;
> > +
> > +		return EFI_EXIT(EFI_SUCCESS);
> 
> Instead of hoping that the compiler reduces the code size I would prefer
> to use a common exit point where you call EFI_EXIT().

Sure I'll change this on v3

> 
> Best regards
> 
> Heinrich
> 
[...]

Regards
/Ilias

[PATCH 1/3 v2] tpm: Change response length of tpm2_get_capability()

[Simon Glass]

On Thu, 5 Nov 2020 at 14:58, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> For implementing the EFI_TCG2_PROTOCOL we need the count field returned by
> the TPM when reading capabilities via tpm2_get_capability().
>
> Adjust the implementation of the 'tpm2 get_capability' command accordingly.
>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Changes since v1:
> - Unconditionally get the extra 4 bytes on the response and account for them
>   in do_tpm_get_capability()
>  cmd/tpm-v2.c | 4 ++--
>  lib/tpm-v2.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>