| Message ID | 20181106222142.94537-18-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | sandbox: Changes and improvements to support verified boot | expand |
Hi Simon, Simon Glass <sjg@chromium.org> wrote on Tue, 6 Nov 2018 15:21:34 -0700: > At present these functions are not accessible outside the TPM library, but > in some cases we need to call them. I was not aware, what is the use case? I don't get it. > Export them in the header file and add > a define for the SHA1 digest size. > > Also adjust tpm_open() to call tpm_close() first so that the TPM is in a > known state before opening (e.g. by a previous phase of U-Boot). > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > [...] > @@ -408,29 +435,12 @@ static int tpm_tis_lpc_open(struct udevice *dev) > return ret; > } > > + /* Certain TPMs need some delay here or they hang */ > + udelay(10); > + > tpm_write_word(priv, TIS_STS_COMMAND_READY, > ®s[locality].tpm_status); This is not in the commit message. Perhaps, due to the nature of the changes, this patch would be best split in 2 or 3? Thanks, Miquèl
Hi Miquel, On Wed, 7 Nov 2018 at 00:52, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Simon, > > Simon Glass <sjg@chromium.org> wrote on Tue, 6 Nov 2018 15:21:34 -0700: > > > At present these functions are not accessible outside the TPM library, but > > in some cases we need to call them. > > I was not aware, what is the use case? I don't get it. I believe this is for when TPL sets up the TPM but we need to access it again in U-Boot proper, so close it before opening it again. I'm not 100% sure though. > > > Export them in the header file and add > > a define for the SHA1 digest size. > > > > Also adjust tpm_open() to call tpm_close() first so that the TPM is in a > > known state before opening (e.g. by a previous phase of U-Boot). > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > [...] > > > @@ -408,29 +435,12 @@ static int tpm_tis_lpc_open(struct udevice *dev) > > return ret; > > } > > > > + /* Certain TPMs need some delay here or they hang */ > > + udelay(10); > > + > > tpm_write_word(priv, TIS_STS_COMMAND_READY, > > ®s[locality].tpm_status); > > This is not in the commit message. > > Perhaps, due to the nature of the changes, this patch would be best > split in 2 or 3? OK will do. Regards, Simon
Hi Miquel, On Wed, 7 Nov 2018 at 00:52, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Simon, > > Simon Glass <sjg@chromium.org> wrote on Tue, 6 Nov 2018 15:21:34 -0700: > > > At present these functions are not accessible outside the TPM library, but > > in some cases we need to call them. > > I was not aware, what is the use case? I don't get it. I believe this is for when TPL sets up the TPM but we need to access it again in U-Boot proper, so close it before opening it again. I'm not 100% sure though. > > > Export them in the header file and add > > a define for the SHA1 digest size. > > > > Also adjust tpm_open() to call tpm_close() first so that the TPM is in a > > known state before opening (e.g. by a previous phase of U-Boot). > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > [...] > > > @@ -408,29 +435,12 @@ static int tpm_tis_lpc_open(struct udevice *dev) > > return ret; > > } > > > > + /* Certain TPMs need some delay here or they hang */ > > + udelay(10); > > + > > tpm_write_word(priv, TIS_STS_COMMAND_READY, > > ®s[locality].tpm_status); > > This is not in the commit message. > > Perhaps, due to the nature of the changes, this patch would be best > split in 2 or 3? OK will do. Regards, Simon Applied to u-boot-dm/master, thanks!
diff --git a/drivers/tpm/tpm_tis_lpc.c b/drivers/tpm/tpm_tis_lpc.c index e993fd9f833..d76d7ca75af 100644 --- a/drivers/tpm/tpm_tis_lpc.c +++ b/drivers/tpm/tpm_tis_lpc.c @@ -388,6 +388,27 @@ static int tis_readresponse(struct udevice *dev, u8 *buffer, size_t len) return offset; } +static int tpm_tis_lpc_close(struct udevice *dev) +{ + struct tpm_tis_lpc_priv *priv = dev_get_priv(dev); + struct tpm_locality *regs = priv->regs; + u8 locality = 0; + + if (tpm_read_word(priv, ®s[locality].access) & + TIS_ACCESS_ACTIVE_LOCALITY) { + tpm_write_word(priv, TIS_ACCESS_ACTIVE_LOCALITY, + ®s[locality].access); + + if (tis_wait_reg(priv, ®s[locality].access, + TIS_ACCESS_ACTIVE_LOCALITY, 0) == -ETIMEDOUT) { + printf("%s:%d - failed to release locality %d\n", + __FILE__, __LINE__, locality); + return -ETIMEDOUT; + } + } + return 0; +} + static int tpm_tis_lpc_open(struct udevice *dev) { struct tpm_tis_lpc_priv *priv = dev_get_priv(dev); @@ -395,6 +416,12 @@ static int tpm_tis_lpc_open(struct udevice *dev) u8 locality = 0; /* we use locality zero for everything. */ int ret; + ret = tpm_tis_lpc_close(dev); + if (ret) { + printf("%s: Failed to close TPM\n", __func__); + return ret; + } + /* now request access to locality. */ tpm_write_word(priv, TIS_ACCESS_REQUEST_USE, ®s[locality].access); @@ -408,29 +435,12 @@ static int tpm_tis_lpc_open(struct udevice *dev) return ret; } + /* Certain TPMs need some delay here or they hang */ + udelay(10); + tpm_write_word(priv, TIS_STS_COMMAND_READY, ®s[locality].tpm_status); - return 0; -} - -static int tpm_tis_lpc_close(struct udevice *dev) -{ - struct tpm_tis_lpc_priv *priv = dev_get_priv(dev); - struct tpm_locality *regs = priv->regs; - u8 locality = 0; - - if (tpm_read_word(priv, ®s[locality].access) & - TIS_ACCESS_ACTIVE_LOCALITY) { - tpm_write_word(priv, TIS_ACCESS_ACTIVE_LOCALITY, - ®s[locality].access); - if (tis_wait_reg(priv, ®s[locality].access, - TIS_ACCESS_ACTIVE_LOCALITY, 0) == -ETIMEDOUT) { - printf("%s:%d - failed to release locality %d\n", - __FILE__, __LINE__, locality); - return -ETIMEDOUT; - } - } return 0; } diff --git a/include/tpm-common.h b/include/tpm-common.h index 5f8bc6bc528..f8c5569003e 100644 --- a/include/tpm-common.h +++ b/include/tpm-common.h @@ -26,6 +26,8 @@ enum tpm_duration { /* Max buffer size supported by our tpm */ #define TPM_DEV_BUFSIZE 1260 +#define TPM_PCR_MINIMUM_DIGEST_SIZE 20 + /** * enum tpm_version - The version of the TPM stack to be used * @TPM_V1: Use TPM v1.x stack @@ -179,6 +181,24 @@ int do_##cmd(cmd_tbl_t *cmdtp, int flag, \ return report_return_code(cmd()); \ } +/** + * tpm_open() - Request access to locality 0 for the caller + * + * After all commands have been completed the caller is supposed to + * call tpm_close(). + * + * Returns 0 on success, -ve on failure. + */ +int tpm_open(struct udevice *dev); + +/** + * tpm_close() - Close the current session + * + * Releasing the locked locality. Returns 0 on success, -ve 1 on + * failure (in case lock removal did not succeed). + */ +int tpm_close(struct udevice *dev); + /** * tpm_get_desc() - Get a text description of the TPM * diff --git a/lib/tpm-utils.h b/lib/tpm-utils.h index a9cb7dc7ee5..ac95f262f56 100644 --- a/lib/tpm-utils.h +++ b/lib/tpm-utils.h @@ -18,24 +18,6 @@ #define tpm_u16(x) __MSB(x), __LSB(x) #define tpm_u32(x) tpm_u16((x) >> 16), tpm_u16((x) & 0xFFFF) -/** - * tpm_open() - Request access to locality 0 for the caller - * - * After all commands have been completed the caller is supposed to - * call tpm_close(). - * - * Returns 0 on success, -ve on failure. - */ -int tpm_open(struct udevice *dev); - -/** - * tpm_close() - Close the current session - * - * Releasing the locked locality. Returns 0 on success, -ve 1 on - * failure (in case lock removal did not succeed). - */ -int tpm_close(struct udevice *dev); - /** * Pack data into a byte string. The data types are specified in * the format string: 'b' means unsigned byte, 'w' unsigned word,
At present these functions are not accessible outside the TPM library, but in some cases we need to call them. Export them in the header file and add a define for the SHA1 digest size. Also adjust tpm_open() to call tpm_close() first so that the TPM is in a known state before opening (e.g. by a previous phase of U-Boot). Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/tpm/tpm_tis_lpc.c | 50 +++++++++++++++++++++++---------------- include/tpm-common.h | 20 ++++++++++++++++ lib/tpm-utils.h | 18 -------------- 3 files changed, 50 insertions(+), 38 deletions(-)