On Fri, Jun 08, 2018 at 12:20:40AM +0300, Tomas Winkler wrote:
We cannot use go_idle cmd_ready commands via runtime_pm handles as with the introduction of localities this is no longer an optional feature, while runtime pm can be not enabled. Though cmd_ready/go_idle provides a power saving, it's also a part of TPM2 protocol and should be called explicitly. This patch exposes cmd_read/go_idle via tpm class ops and removes runtime pm support as it is not used by any driver.
A new tpm transmit flag is added TPM_TRANSMIT_NESTED, which implies TPM_TRANSMIT_UNLOCKED and TPM_TRANSMIT_RAW. Both are needed
to resolve
tpm spaces and locality request recursive calls to tpm_transmit().
New wrappers are added tpm_cmd_ready() and tpm_go_idle() to
streamline
tpm_try_transmit code.
tpm_crb no longer needs own power saving functions and can drop using tpm_pm_suspend/resume.
This patch cannot be really separated from the locality fix. Fixes: 888d867df441 (tpm: cmd_ready command can be issued only after granting locality)
Cc: stable@vger.kernel.org Fixes: 888d867df441 (tpm: cmd_ready command can be issued only after granting locality) Signed-off-by: Tomas Winkler tomas.winkler@intel.com
V2-3:resent V4: 1. Use tpm_pm_suspend/resume in tpm_crb 2. Drop cmd_ready/go_idle around tpm_chip_register, as there is no usage counter like in runtime_pm. V5: 1. add tpm_cmd_ready and tpm_go_idle wrappers. 2. Abuse TPM_TRANSMIT_UNLOCKED flag to resolve tpm space recursion. V6: 1. Add new flags TPM_TRANSMIT_NESTED which implies TPM_TRANSMIT_UNLOCKED and TPM_TRANSMIT_RAW, as not all unlocked flows are recursive i.e. tpm2_unseal_trusted.
drivers/char/tpm/tpm-interface.c | 51 +++++++++++++++---- drivers/char/tpm/tpm.h | 13 ++++- drivers/char/tpm/tpm2-space.c | 12 ++--- drivers/char/tpm/tpm_crb.c | 101 ++++++++++---------------------------- drivers/char/tpm/tpm_vtpm_proxy.c | 2 +- include/linux/tpm.h | 2 + 6 files changed, 88 insertions(+), 93 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index e32f6e85dc6d..1abd8261651b 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -29,7 +29,6 @@ #include <linux/mutex.h> #include <linux/spinlock.h> #include <linux/freezer.h> -#include <linux/pm_runtime.h> #include <linux/tpm_eventlog.h>
#include "tpm.h" @@ -369,10 +368,13 @@ static int tpm_validate_command(struct
tpm_chip *chip,
return -EINVAL; }
-static int tpm_request_locality(struct tpm_chip *chip) +static int tpm_request_locality(struct tpm_chip *chip, unsigned int +flags) { int rc;
- if (flags & __TPM_TRANSMIT_RAW)
return 0;- if (!chip->ops->request_locality) return 0;
@@ -385,10 +387,13 @@ static int tpm_request_locality(struct tpm_chip
*chip)
return 0; }
-static void tpm_relinquish_locality(struct tpm_chip *chip) +static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned +int flags) { int rc;
- if (flags & __TPM_TRANSMIT_RAW)
return;- if (!chip->ops->relinquish_locality) return;
@@ -399,6 +404,28 @@ static void tpm_relinquish_locality(struct
tpm_chip *chip)
chip->locality = -1; }
+static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags) {
- if (flags & __TPM_TRANSMIT_RAW)
return 0;- if (!chip->ops->cmd_ready)
return 0;- return chip->ops->cmd_ready(chip);
+}
+static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags) {
- if (flags & __TPM_TRANSMIT_RAW)
return 0;- if (!chip->ops->go_idle)
return 0;- return chip->ops->go_idle(chip);
+}
static ssize_t tpm_try_transmit(struct tpm_chip *chip, struct tpm_space *space, u8 *buf, size_t bufsiz, @@ -449,14 +476,15 @@ static ssize_t tpm_try_transmit(struct tpm_chip
*chip,
/* Store the decision as chip->locality will be changed. */ need_locality = chip->locality == -1;
- if (!(flags & TPM_TRANSMIT_RAW) && need_locality) {
rc = tpm_request_locality(chip);
- if (need_locality) {
rc = tpm_request_locality(chip, flags);
- if (chip->dev.parent)
pm_runtime_get_sync(chip->dev.parent);
rc = tpm_cmd_ready(chip, flags);
if (rc)
goto out;rc = tpm2_prepare_space(chip, space, ordinal, buf); if (rc)
@@ -516,13 +544,16 @@ static ssize_t tpm_try_transmit(struct tpm_chip
*chip,
}
rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
- if (rc)
dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);out:
- if (chip->dev.parent)
pm_runtime_put_sync(chip->dev.parent);
rc = tpm_go_idle(chip, flags);
if (rc)
goto out;if (need_locality)
tpm_relinquish_locality(chip);
tpm_relinquish_locality(chip, flags);out_no_locality: if (chip->ops->clk_enable != NULL) diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 4426649e431c..beb0a763016c 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -511,9 +511,18 @@ extern const struct file_operations tpm_fops; extern const struct file_operations tpmrm_fops; extern struct idr dev_nums_idr;
+/**
- enum tpm_transmit_flags
- @TPM_TRANSMIT_UNLOCKED: used to lock sequence of tpm_transmit
calls.
- @__TPM_TRANSMIT_RAW: prevent recursive calls into setup steps
(go idle, locality,..). Don't use directly.
- @TPM_TRANSMIT_NESTED: Use from nested tpm_transmit calls */
enum tpm_transmit_flags {
- TPM_TRANSMIT_UNLOCKED = BIT(0),
- TPM_TRANSMIT_RAW = BIT(1),
- TPM_TRANSMIT_UNLOCKED = BIT(0),
- __TPM_TRANSMIT_RAW = BIT(1),
NAK for the naming convention.
What do you suggest Tomas