From: Jerry Snitselaar <jsnitsel@redhat.com>
To: Stefan Berger <stefanb@linux.ibm.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
Peter Huewe <PeterHuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
Tomas Winkler <tomas.winkler@intel.com>,
Tadeusz Struk <tadeusz.struk@intel.com>,
Stefan Berger <stefanb@linux.vnet.ibm.com>,
Nayna Jain <nayna@linux.ibm.com>
Subject: Re: [PATCH v11 15/16] tpm: take TPM chip power gating out of tpm_transmit()
Date: Thu, 7 Feb 2019 17:02:51 -0700 [thread overview]
Message-ID: <20190208000251.4g5gnsalxh7i5dva@cantor> (raw)
In-Reply-To: <dfb224e4-82b8-26c0-e69a-c72d19712d48@linux.ibm.com>
On Thu Feb 07 19, Stefan Berger wrote:
>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@linux.intel.com>
>>Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>Tested-by: Stefan Berger <stefanb@linux.ibm.com>
>>Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
>>Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>>---
>> 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().
>
Reproduced here with discrete chip. Will test fix in a bit.
next prev parent reply other threads:[~2019-02-08 0:02 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-05 22:47 [PATCH v11 00/16] Remove nested TPM operations Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 02/16] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 04/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 05/16] tpm: declare struct tpm_header Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 06/16] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 07/16] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 08/16] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
2019-02-07 23:36 ` Stefan Berger
2019-02-05 22:47 ` [PATCH v11 09/16] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 10/16] tpm: move TPM space code out of tpm_transmit() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 11/16] tpm: remove @space from tpm_transmit() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 12/16] tpm: use tpm_try_get_ops() in tpm-sysfs.c Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 13/16] tpm: remove TPM_TRANSMIT_UNLOCKED flag Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 14/16] tpm: introduce tpm_chip_start() and tpm_chip_stop() Jarkko Sakkinen
2019-02-05 22:47 ` [PATCH v11 15/16] tpm: take TPM chip power gating out of tpm_transmit() Jarkko Sakkinen
2019-02-07 23:32 ` Stefan Berger
2019-02-08 0:02 ` Jerry Snitselaar [this message]
2019-02-05 22:47 ` [PATCH v11 16/16] tpm: remove @flags from tpm_transmit() Jarkko Sakkinen
2019-02-06 12:06 ` [PATCH v11 00/16] Remove nested TPM operations Jarkko Sakkinen
2019-02-07 18:41 ` Alexander Steffen
2019-02-07 21:14 ` Stefan Berger
2019-02-07 21:29 ` Jarkko Sakkinen
2019-02-07 23:29 ` Stefan Berger
2019-02-08 0:33 ` Jarkko Sakkinen
2019-02-08 1:51 ` Stefan Berger
2019-02-08 2:14 ` Stefan Berger
2019-02-08 11:50 ` Jarkko Sakkinen
2019-02-08 12:22 ` Stefan Berger
2019-02-08 13:12 ` Jarkko Sakkinen
2019-02-08 13:28 ` Alexander Steffen
2019-02-08 14:09 ` Jarkko Sakkinen
2019-02-08 18:02 ` Alexander Steffen
2019-02-08 11:14 ` Jarkko Sakkinen
2019-02-08 12:05 ` Stefan Berger
2019-02-08 13:02 ` Jarkko Sakkinen
2019-02-08 13:10 ` Stefan Berger
2019-02-08 13:17 ` Jarkko Sakkinen
2019-02-08 13:33 ` Jarkko Sakkinen
2019-02-08 14:02 ` Stefan Berger
2019-02-08 14:08 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190208000251.4g5gnsalxh7i5dva@cantor \
--to=jsnitsel@redhat.com \
--cc=PeterHuewe@gmx.de \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jgg@ziepe.ca \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=nayna@linux.ibm.com \
--cc=stefanb@linux.ibm.com \
--cc=stefanb@linux.vnet.ibm.com \
--cc=tadeusz.struk@intel.com \
--cc=tomas.winkler@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.