Re: [PATCH v11 15/16] tpm: take TPM chip power gating out of tpm_transmit()

From: Stefan Berger
Date: Thu Feb 07 2019 - 18:32:30 EST

On 2/5/19 5:47 PM, Jarkko Sakkinen wrote:
Call tpm_chip_start() and tpm_chip_stop() in

* tpm_try_get_ops() and tpm_put_ops()
* tpm_chip_register()
* tpm2_del_space()

And remove these calls from tpm_transmit(). The core reason for this
change is that in tpm_vtpm_proxy a locality change requires a virtual
TPM command (a command made up just for that driver).

The consequence of this is that this commit removes the remaining nested
calls.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Tested-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Reviewed-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>
Reviewed-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/char/tpm/tpm-chip.c | 25 ++++++++++++-------------
drivers/char/tpm/tpm-interface.c | 6 ------
drivers/char/tpm/tpm.h | 9 ---------
drivers/char/tpm/tpm2-space.c | 5 ++++-
drivers/char/tpm/tpm_vtpm_proxy.c | 3 +--
5 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 65f1561eba81..7ad4d9045e4c 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -41,9 +41,6 @@ static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
{
int rc;

- if (flags & TPM_TRANSMIT_NESTED)
- return 0;
-
if (!chip->ops->request_locality)
return 0;

@@ -59,9 +56,6 @@ static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
{
int rc;

- if (flags & TPM_TRANSMIT_NESTED)
- return;
-
if (!chip->ops->relinquish_locality)
return;

@@ -74,9 +68,6 @@ static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)

static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
{
- if (flags & TPM_TRANSMIT_NESTED)
- return 0;
-
if (!chip->ops->cmd_ready)
return 0;

@@ -85,9 +76,6 @@ static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)

static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
{
- if (flags & TPM_TRANSMIT_NESTED)
- return 0;
-
if (!chip->ops->go_idle)
return 0;

@@ -166,11 +154,17 @@ int tpm_try_get_ops(struct tpm_chip *chip)

down_read(&chip->ops_sem);
if (!chip->ops)
- goto out_lock;
+ goto out_ops;

mutex_lock(&chip->tpm_mutex);
+ rc = tpm_chip_start(chip, 0);
+ if (rc)
+ goto out_lock;
+
return 0;
out_lock:
+ mutex_unlock(&chip->tpm_mutex);
+out_ops:
up_read(&chip->ops_sem);
put_device(&chip->dev);
return rc;
@@ -186,6 +180,7 @@ EXPORT_SYMBOL_GPL(tpm_try_get_ops);
*/
void tpm_put_ops(struct tpm_chip *chip)
{
+ tpm_chip_stop(chip, 0);
mutex_unlock(&chip->tpm_mutex);
up_read(&chip->ops_sem);
put_device(&chip->dev);
@@ -563,7 +558,11 @@ int tpm_chip_register(struct tpm_chip *chip)
{
int rc;

+ rc = tpm_chip_start(chip, 0);
+ if (rc)
+ return rc;
rc = tpm_auto_startup(chip);
+ tpm_chip_stop(chip, 0);
if (rc)
return rc;

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f7b7e4e75fcf..f20c78055731 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -168,13 +168,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
memcpy(save, buf, save_size);

for (;;) {
- ret = tpm_chip_start(chip, flags);
- if (ret)
- return ret;
-
ret = tpm_try_transmit(chip, buf, bufsiz, flags);
-
- tpm_chip_stop(chip, flags);
if (ret < 0)
break;
rc = be32_to_cpu(header->return_code);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2d6d934f1c8b..53e4208759ee 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -485,15 +485,6 @@ extern const struct file_operations tpm_fops;
extern const struct file_operations tpmrm_fops;
extern struct idr dev_nums_idr;

-/**
- * enum tpm_transmit_flags - flags for tpm_transmit()
- *
- * %TPM_TRANSMIT_NESTED: discard setup steps (power management, locality)
- */
-enum tpm_transmit_flags {
- TPM_TRANSMIT_NESTED = BIT(0),
-};
-
ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
unsigned int flags);
ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 9c9ccf2c0681..6c6ad2d4d31b 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -60,7 +60,10 @@ int tpm2_init_space(struct tpm_space *space)
void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
{
mutex_lock(&chip->tpm_mutex);
- tpm2_flush_sessions(chip, space);
+ if (!tpm_chip_start(chip, 0)) {
+ tpm2_flush_sessions(chip, space);
+ tpm_chip_stop(chip, 0);
+ }
mutex_unlock(&chip->tpm_mutex);
kfree(space->context_buf);
kfree(space->session_buf);
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index e8a1da2810a9..a4bb60e163cc 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -417,8 +417,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)

proxy_dev->state |= STATE_DRIVER_COMMAND;

- rc = tpm_transmit_cmd(chip, &buf, 0, TPM_TRANSMIT_NESTED,
- "attempting to set locality");
+ rc = tpm_transmit_cmd(chip, &buf, 0, 0, "attempting to set locality");

proxy_dev->state &= ~STATE_DRIVER_COMMAND;

This patch seems to be missing a hunk along these lines here

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e74c5b7b64bf..52afe20cc8a1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -799,7 +799,9 @@ int tpm2_probe(struct tpm_chip *chip)
ÂÂÂÂ tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
ÂÂÂÂ tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
ÂÂÂÂ tpm_buf_append_u32(&buf, 1);
+ÂÂÂ tpm_chip_start(chip, 0);
ÂÂÂÂ rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ÂÂÂ tpm_chip_stop(chip, 0);
ÂÂÂÂ /* We ignore TPM return codes on purpose. */
ÂÂÂÂ if (rc >=Â 0) {
ÂÂÂÂ ÂÂÂ out = (struct tpm_header *)buf.data;


Of course you need to check the error from tpm_chip_start().