* [PATCH v3 0/4] additional TPM performance improvements
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
After further discussions with the TCG Device Driver working group (ddwg),
the following changes were made:
* Check for burstcount at least once to confirm the TPM is ready to accept
the data. Similarly, query for the TPM Expect status as sanity check at
the end.
* Make the sleep for status check in the loop less than 5msec.
* Make the sleep in the loop while querying for burstcount less than
5msec.
Below is the list of patches along with the performance improvements
seen with a TPM 1.2 with an 8 byte burstcount for 1000 extends:
Patch |Improvement(time in sec)
tpm: ignore burstcount to improve tpm_tis | ~41 - ~14
send() performance.
tpm: reduce polling delay in tpm_tis | ~14 - ~10
wait_for_tpm_stat()
tpm: reduce tpm_msleep() time in | ~10 - ~9
get_burstcount()
tpm: modify tpm_msleep() function to have | ~9 - ~8
max range
Changelog v3:
* Include Jarkko's feedback to move wait_for_tpm_stat() from
tpm-interface.c to respective driver files tpm_tis_core.c and
xen-tpmfront.c.
* Remove use of module parameter ignore_burst_count and is
now alwaysed ignored in tpm tis send() operation.
* Reduce sleep() time during polling in wait_for_tpm_stat() from
5msec to 1msec for both recv() and send() operation. Version
2 of the patches had it only for send() operation.
Changelog v2:
* Add module parameter to handle ignoring of burst count during
tpm tis send() operation.
* Add improvements over sleep time to reduce delays.
Nayna Jain (5):
tpm: move wait_for_tpm_stat() to respective driver files
tpm: ignore burstcount to improve tpm_tis send() performance
tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
tpm: reduce tpm_msleep() time in get_burstcount()
tpm: use tpm_msleep() value as max delay
drivers/char/tpm/tpm-interface.c | 60 ----------------------
drivers/char/tpm/tpm.h | 6 +--
drivers/char/tpm/tpm_tis_core.c | 106 +++++++++++++++++++++++++++++----------
drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++
4 files changed, 141 insertions(+), 91 deletions(-)
--
2.13.3
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 0/4] additional TPM performance improvements
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
After further discussions with the TCG Device Driver working group (ddwg),
the following changes were made:
* Check for burstcount at least once to confirm the TPM is ready to accept
the data. Similarly, query for the TPM Expect status as sanity check at
the end.
* Make the sleep for status check in the loop less than 5msec.
* Make the sleep in the loop while querying for burstcount less than
5msec.
Below is the list of patches along with the performance improvements
seen with a TPM 1.2 with an 8 byte burstcount for 1000 extends:
Patch |Improvement(time in sec)
tpm: ignore burstcount to improve tpm_tis | ~41 - ~14
send() performance.
tpm: reduce polling delay in tpm_tis | ~14 - ~10
wait_for_tpm_stat()
tpm: reduce tpm_msleep() time in | ~10 - ~9
get_burstcount()
tpm: modify tpm_msleep() function to have | ~9 - ~8
max range
Changelog v3:
* Include Jarkko's feedback to move wait_for_tpm_stat() from
tpm-interface.c to respective driver files tpm_tis_core.c and
xen-tpmfront.c.
* Remove use of module parameter ignore_burst_count and is
now alwaysed ignored in tpm tis send() operation.
* Reduce sleep() time during polling in wait_for_tpm_stat() from
5msec to 1msec for both recv() and send() operation. Version
2 of the patches had it only for send() operation.
Changelog v2:
* Add module parameter to handle ignoring of burst count during
tpm tis send() operation.
* Add improvements over sleep time to reduce delays.
Nayna Jain (5):
tpm: move wait_for_tpm_stat() to respective driver files
tpm: ignore burstcount to improve tpm_tis send() performance
tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
tpm: reduce tpm_msleep() time in get_burstcount()
tpm: use tpm_msleep() value as max delay
drivers/char/tpm/tpm-interface.c | 60 ----------------------
drivers/char/tpm/tpm.h | 6 +--
drivers/char/tpm/tpm_tis_core.c | 106 +++++++++++++++++++++++++++++----------
drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++
4 files changed, 141 insertions(+), 91 deletions(-)
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 1/5] tpm: move wait_for_tpm_stat() to respective driver files
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-04 10:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
The function wait_for_tpm_stat() is currently defined in
tpm-interface file. It is a hardware specific function used
only by tpm_tis and xen-tpmfront, so it is removed from
tpm-interface.c and defined in respective driver files.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm-interface.c | 60 ----------------------------------------
drivers/char/tpm/tpm.h | 2 --
drivers/char/tpm/tpm_tis_core.c | 60 ++++++++++++++++++++++++++++++++++++++++
drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 62 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1d6729be4cd6..313f7618d569 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1035,66 +1035,6 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
}
EXPORT_SYMBOL_GPL(tpm_send);
-static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
- bool check_cancel, bool *canceled)
-{
- u8 status = chip->ops->status(chip);
-
- *canceled = false;
- if ((status & mask) == mask)
- return true;
- if (check_cancel && chip->ops->req_canceled(chip, status)) {
- *canceled = true;
- return true;
- }
- return false;
-}
-
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
- wait_queue_head_t *queue, bool check_cancel)
-{
- unsigned long stop;
- long rc;
- u8 status;
- bool canceled = false;
-
- /* check current status */
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
-
- stop = jiffies + timeout;
-
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
-again:
- timeout = stop - jiffies;
- if ((long)timeout <= 0)
- return -ETIME;
- rc = wait_event_interruptible_timeout(*queue,
- wait_for_tpm_stat_cond(chip, mask, check_cancel,
- &canceled),
- timeout);
- if (rc > 0) {
- if (canceled)
- return -ECANCELED;
- return 0;
- }
- if (rc == -ERESTARTSYS && freezing(current)) {
- clear_thread_flag(TIF_SIGPENDING);
- goto again;
- }
- } else {
- do {
- tpm_msleep(TPM_TIMEOUT);
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
- } while (time_before(jiffies, stop));
- }
- return -ETIME;
-}
-EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
-
#define TPM_ORD_SAVESTATE 152
#define SAVESTATE_RESULT_SIZE 10
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2d5466a72e40..4fc83ac7abeb 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -525,8 +525,6 @@ int tpm_do_selftest(struct tpm_chip *chip);
unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm_pm_suspend(struct device *dev);
int tpm_pm_resume(struct device *dev);
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
- wait_queue_head_t *queue, bool check_cancel);
static inline void tpm_msleep(unsigned int delay_msec)
{
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 63bc6c3b949e..b33126a35694 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,66 @@
#include "tpm.h"
#include "tpm_tis_core.h"
+static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
+ bool check_cancel, bool *canceled)
+{
+ u8 status = chip->ops->status(chip);
+
+ *canceled = false;
+ if ((status & mask) == mask)
+ return true;
+ if (check_cancel && chip->ops->req_canceled(chip, status)) {
+ *canceled = true;
+ return true;
+ }
+ return false;
+}
+
+static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+ unsigned long timeout, wait_queue_head_t *queue,
+ bool check_cancel)
+{
+ unsigned long stop;
+ long rc;
+ u8 status;
+ bool canceled = false;
+
+ /* check current status */
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+
+ stop = jiffies + timeout;
+
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+again:
+ timeout = stop - jiffies;
+ if ((long)timeout <= 0)
+ return -ETIME;
+ rc = wait_event_interruptible_timeout(*queue,
+ wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ &canceled),
+ timeout);
+ if (rc > 0) {
+ if (canceled)
+ return -ECANCELED;
+ return 0;
+ }
+ if (rc == -ERESTARTSYS && freezing(current)) {
+ clear_thread_flag(TIF_SIGPENDING);
+ goto again;
+ }
+ } else {
+ do {
+ tpm_msleep(TPM_TIMEOUT);
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ } while (time_before(jiffies, stop));
+ }
+ return -ETIME;
+}
+
/* Before we attempt to access the TPM we must see that the valid bit is set.
* The specification says that this bit is 0 at reset and remains 0 until the
* 'TPM has gone through its self test and initialization and has established
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 656e8af95d52..d20a0a9ded27 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -39,6 +39,66 @@ enum status_bits {
VTPM_STATUS_CANCELED = 0x8,
};
+static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
+ bool check_cancel, bool *canceled)
+{
+ u8 status = chip->ops->status(chip);
+
+ *canceled = false;
+ if ((status & mask) == mask)
+ return true;
+ if (check_cancel && chip->ops->req_canceled(chip, status)) {
+ *canceled = true;
+ return true;
+ }
+ return false;
+}
+
+static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+ unsigned long timeout, wait_queue_head_t *queue,
+ bool check_cancel)
+{
+ unsigned long stop;
+ long rc;
+ u8 status;
+ bool canceled = false;
+
+ /* check current status */
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+
+ stop = jiffies + timeout;
+
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+again:
+ timeout = stop - jiffies;
+ if ((long)timeout <= 0)
+ return -ETIME;
+ rc = wait_event_interruptible_timeout(*queue,
+ wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ &canceled),
+ timeout);
+ if (rc > 0) {
+ if (canceled)
+ return -ECANCELED;
+ return 0;
+ }
+ if (rc == -ERESTARTSYS && freezing(current)) {
+ clear_thread_flag(TIF_SIGPENDING);
+ goto again;
+ }
+ } else {
+ do {
+ tpm_msleep(TPM_TIMEOUT);
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ } while (time_before(jiffies, stop));
+ }
+ return -ETIME;
+}
+
static u8 vtpm_status(struct tpm_chip *chip)
{
struct tpm_private *priv = dev_get_drvdata(&chip->dev);
--
2.13.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 1/5] tpm: move wait_for_tpm_stat() to respective driver files
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
The function wait_for_tpm_stat() is currently defined in
tpm-interface file. It is a hardware specific function used
only by tpm_tis and xen-tpmfront, so it is removed from
tpm-interface.c and defined in respective driver files.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm-interface.c | 60 ----------------------------------------
drivers/char/tpm/tpm.h | 2 --
drivers/char/tpm/tpm_tis_core.c | 60 ++++++++++++++++++++++++++++++++++++++++
drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 62 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1d6729be4cd6..313f7618d569 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1035,66 +1035,6 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
}
EXPORT_SYMBOL_GPL(tpm_send);
-static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
- bool check_cancel, bool *canceled)
-{
- u8 status = chip->ops->status(chip);
-
- *canceled = false;
- if ((status & mask) == mask)
- return true;
- if (check_cancel && chip->ops->req_canceled(chip, status)) {
- *canceled = true;
- return true;
- }
- return false;
-}
-
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
- wait_queue_head_t *queue, bool check_cancel)
-{
- unsigned long stop;
- long rc;
- u8 status;
- bool canceled = false;
-
- /* check current status */
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
-
- stop = jiffies + timeout;
-
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
-again:
- timeout = stop - jiffies;
- if ((long)timeout <= 0)
- return -ETIME;
- rc = wait_event_interruptible_timeout(*queue,
- wait_for_tpm_stat_cond(chip, mask, check_cancel,
- &canceled),
- timeout);
- if (rc > 0) {
- if (canceled)
- return -ECANCELED;
- return 0;
- }
- if (rc == -ERESTARTSYS && freezing(current)) {
- clear_thread_flag(TIF_SIGPENDING);
- goto again;
- }
- } else {
- do {
- tpm_msleep(TPM_TIMEOUT);
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
- } while (time_before(jiffies, stop));
- }
- return -ETIME;
-}
-EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
-
#define TPM_ORD_SAVESTATE 152
#define SAVESTATE_RESULT_SIZE 10
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2d5466a72e40..4fc83ac7abeb 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -525,8 +525,6 @@ int tpm_do_selftest(struct tpm_chip *chip);
unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm_pm_suspend(struct device *dev);
int tpm_pm_resume(struct device *dev);
-int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
- wait_queue_head_t *queue, bool check_cancel);
static inline void tpm_msleep(unsigned int delay_msec)
{
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 63bc6c3b949e..b33126a35694 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,66 @@
#include "tpm.h"
#include "tpm_tis_core.h"
+static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
+ bool check_cancel, bool *canceled)
+{
+ u8 status = chip->ops->status(chip);
+
+ *canceled = false;
+ if ((status & mask) == mask)
+ return true;
+ if (check_cancel && chip->ops->req_canceled(chip, status)) {
+ *canceled = true;
+ return true;
+ }
+ return false;
+}
+
+static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+ unsigned long timeout, wait_queue_head_t *queue,
+ bool check_cancel)
+{
+ unsigned long stop;
+ long rc;
+ u8 status;
+ bool canceled = false;
+
+ /* check current status */
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+
+ stop = jiffies + timeout;
+
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+again:
+ timeout = stop - jiffies;
+ if ((long)timeout <= 0)
+ return -ETIME;
+ rc = wait_event_interruptible_timeout(*queue,
+ wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ &canceled),
+ timeout);
+ if (rc > 0) {
+ if (canceled)
+ return -ECANCELED;
+ return 0;
+ }
+ if (rc == -ERESTARTSYS && freezing(current)) {
+ clear_thread_flag(TIF_SIGPENDING);
+ goto again;
+ }
+ } else {
+ do {
+ tpm_msleep(TPM_TIMEOUT);
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ } while (time_before(jiffies, stop));
+ }
+ return -ETIME;
+}
+
/* Before we attempt to access the TPM we must see that the valid bit is set.
* The specification says that this bit is 0 at reset and remains 0 until the
* 'TPM has gone through its self test and initialization and has established
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 656e8af95d52..d20a0a9ded27 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -39,6 +39,66 @@ enum status_bits {
VTPM_STATUS_CANCELED = 0x8,
};
+static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
+ bool check_cancel, bool *canceled)
+{
+ u8 status = chip->ops->status(chip);
+
+ *canceled = false;
+ if ((status & mask) == mask)
+ return true;
+ if (check_cancel && chip->ops->req_canceled(chip, status)) {
+ *canceled = true;
+ return true;
+ }
+ return false;
+}
+
+static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
+ unsigned long timeout, wait_queue_head_t *queue,
+ bool check_cancel)
+{
+ unsigned long stop;
+ long rc;
+ u8 status;
+ bool canceled = false;
+
+ /* check current status */
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+
+ stop = jiffies + timeout;
+
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+again:
+ timeout = stop - jiffies;
+ if ((long)timeout <= 0)
+ return -ETIME;
+ rc = wait_event_interruptible_timeout(*queue,
+ wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ &canceled),
+ timeout);
+ if (rc > 0) {
+ if (canceled)
+ return -ECANCELED;
+ return 0;
+ }
+ if (rc == -ERESTARTSYS && freezing(current)) {
+ clear_thread_flag(TIF_SIGPENDING);
+ goto again;
+ }
+ } else {
+ do {
+ tpm_msleep(TPM_TIMEOUT);
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ } while (time_before(jiffies, stop));
+ }
+ return -ETIME;
+}
+
static u8 vtpm_status(struct tpm_chip *chip)
{
struct tpm_private *priv = dev_get_drvdata(&chip->dev);
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-04 10:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
The TPM burstcount status indicates the number of bytes that can
be sent to the TPM without causing bus wait states. Effectively,
it is the number of empty bytes in the command FIFO. Further,
some TPMs have a static burstcount, when the value remains zero
until the entire FIFO is empty.
This patch optimizes the tpm_tis_send_data() function by checking
the burstcount only once. And if the burstcount is valid, it writes
all the bytes at once, permitting wait states. The performance of a
34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
41msec to 14msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~41sec to ~14sec.
Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
conjunction with the TPM Device Driver work group.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index b33126a35694..8da425e1783f 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int rc, status, burstcnt;
- size_t count = 0;
bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
status = tpm_tis_status(chip);
@@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
}
- while (count < len - 1) {
- burstcnt = get_burstcount(chip);
- if (burstcnt < 0) {
- dev_err(&chip->dev, "Unable to read burstcount\n");
- rc = burstcnt;
- goto out_err;
- }
- burstcnt = min_t(int, burstcnt, len - count - 1);
- rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
- burstcnt, buf + count);
- if (rc < 0)
- goto out_err;
+ /*
+ * Get the initial burstcount to ensure TPM is ready to
+ * accept data.
+ */
+ burstcnt = get_burstcount(chip);
+ if (burstcnt < 0) {
+ dev_err(&chip->dev, "Unable to read burstcount\n");
+ rc = burstcnt;
+ goto out_err;
+ }
- count += burstcnt;
+ burstcnt = len - 1;
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
- rc = -ETIME;
- goto out_err;
- }
- status = tpm_tis_status(chip);
- if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
- rc = -EIO;
- goto out_err;
- }
- }
+ rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
+ burstcnt, buf);
+ if (rc < 0)
+ goto out_err;
/* write last byte */
- rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[count]);
+ rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[len-1]);
if (rc < 0)
goto out_err;
--
2.13.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
The TPM burstcount status indicates the number of bytes that can
be sent to the TPM without causing bus wait states. Effectively,
it is the number of empty bytes in the command FIFO. Further,
some TPMs have a static burstcount, when the value remains zero
until the entire FIFO is empty.
This patch optimizes the tpm_tis_send_data() function by checking
the burstcount only once. And if the burstcount is valid, it writes
all the bytes at once, permitting wait states. The performance of a
34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
41msec to 14msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~41sec to ~14sec.
Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
conjunction with the TPM Device Driver work group.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index b33126a35694..8da425e1783f 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int rc, status, burstcnt;
- size_t count = 0;
bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
status = tpm_tis_status(chip);
@@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
}
- while (count < len - 1) {
- burstcnt = get_burstcount(chip);
- if (burstcnt < 0) {
- dev_err(&chip->dev, "Unable to read burstcount\n");
- rc = burstcnt;
- goto out_err;
- }
- burstcnt = min_t(int, burstcnt, len - count - 1);
- rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
- burstcnt, buf + count);
- if (rc < 0)
- goto out_err;
+ /*
+ * Get the initial burstcount to ensure TPM is ready to
+ * accept data.
+ */
+ burstcnt = get_burstcount(chip);
+ if (burstcnt < 0) {
+ dev_err(&chip->dev, "Unable to read burstcount\n");
+ rc = burstcnt;
+ goto out_err;
+ }
- count += burstcnt;
+ burstcnt = len - 1;
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
- rc = -ETIME;
- goto out_err;
- }
- status = tpm_tis_status(chip);
- if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
- rc = -EIO;
- goto out_err;
- }
- }
+ rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
+ burstcnt, buf);
+ if (rc < 0)
+ goto out_err;
/* write last byte */
- rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[count]);
+ rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[len-1]);
if (rc < 0)
goto out_err;
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 3/5] tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-04 10:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
The existing wait_for_tpm_stat() polls for the chip status after
5msec sleep. As per TCG ddwg input, it is expected that tpm might
return status in few usec. So, reducing the delay in polling to
1msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~14sec to ~10sec.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8da425e1783f..224842e06105 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,8 @@
#include "tpm.h"
#include "tpm_tis_core.h"
+#define TPM_POLL_SLEEP 1
+
static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
bool check_cancel, bool *canceled)
{
@@ -82,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
}
} else {
do {
- tpm_msleep(TPM_TIMEOUT);
+ tpm_msleep(TPM_POLL_SLEEP);
status = chip->ops->status(chip);
if ((status & mask) == mask)
return 0;
--
2.13.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 3/5] tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
The existing wait_for_tpm_stat() polls for the chip status after
5msec sleep. As per TCG ddwg input, it is expected that tpm might
return status in few usec. So, reducing the delay in polling to
1msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~14sec to ~10sec.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8da425e1783f..224842e06105 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -31,6 +31,8 @@
#include "tpm.h"
#include "tpm_tis_core.h"
+#define TPM_POLL_SLEEP 1
+
static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
bool check_cancel, bool *canceled)
{
@@ -82,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
}
} else {
do {
- tpm_msleep(TPM_TIMEOUT);
+ tpm_msleep(TPM_POLL_SLEEP);
status = chip->ops->status(chip);
if ((status & mask) == mask)
return 0;
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-04 10:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
Currently, get_burstcount() function sleeps for 5msec in a loop
before retrying for next query to burstcount. However, if it takes
lesser time for TPM to return, this 5msec delay is longer
than necessary.
This patch replaces the tpm_msleep time from 5msec to 1msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~10sec to ~9sec.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 224842e06105..826a0b9c9201 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
burstcnt = (value >> 8) & 0xFFFF;
if (burstcnt)
return burstcnt;
- tpm_msleep(TPM_TIMEOUT);
+ tpm_msleep(TPM_POLL_SLEEP);
} while (time_before(jiffies, stop));
return -EBUSY;
}
--
2.13.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
Currently, get_burstcount() function sleeps for 5msec in a loop
before retrying for next query to burstcount. However, if it takes
lesser time for TPM to return, this 5msec delay is longer
than necessary.
This patch replaces the tpm_msleep time from 5msec to 1msec.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~10sec to ~9sec.
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm_tis_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 224842e06105..826a0b9c9201 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
burstcnt = (value >> 8) & 0xFFFF;
if (burstcnt)
return burstcnt;
- tpm_msleep(TPM_TIMEOUT);
+ tpm_msleep(TPM_POLL_SLEEP);
} while (time_before(jiffies, stop));
return -EBUSY;
}
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 5/5] tpm: use tpm_msleep() value as max delay
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-04 10:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, linux-security-module, linux-kernel, peterhuewe,
jarkko.sakkinen, tpmdd, jgunthorpe, patrickc, Nayna Jain
Currently, tpm_msleep() uses delay_msec as the minimum value in
usleep_range. However, that is the maximum time we want to wait.
The function is modified to use the delay_msec as the maximum
value, not the minimum value.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~9sec to ~8sec.
Fixes: 3b9af007869("tpm: replace msleep() with usleep_range() in TPM 1.2/
2.0 generic drivers")
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4fc83ac7abeb..644de70de2cc 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -528,8 +528,8 @@ int tpm_pm_resume(struct device *dev);
static inline void tpm_msleep(unsigned int delay_msec)
{
- usleep_range(delay_msec * 1000,
- (delay_msec * 1000) + TPM_TIMEOUT_RANGE_US);
+ usleep_range((delay_msec * 1000) - TPM_TIMEOUT_RANGE_US,
+ delay_msec * 1000);
};
struct tpm_chip *tpm_chip_find_get(int chip_num);
--
2.13.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 5/5] tpm: use tpm_msleep() value as max delay
@ 2017-10-04 10:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-04 10:29 UTC (permalink / raw)
To: linux-security-module
Currently, tpm_msleep() uses delay_msec as the minimum value in
usleep_range. However, that is the maximum time we want to wait.
The function is modified to use the delay_msec as the maximum
value, not the minimum value.
After this change, performance on a TPM 1.2 with an 8 byte
burstcount for 1000 extends improved from ~9sec to ~8sec.
Fixes: 3b9af007869("tpm: replace msleep() with usleep_range() in TPM 1.2/
2.0 generic drivers")
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4fc83ac7abeb..644de70de2cc 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -528,8 +528,8 @@ int tpm_pm_resume(struct device *dev);
static inline void tpm_msleep(unsigned int delay_msec)
{
- usleep_range(delay_msec * 1000,
- (delay_msec * 1000) + TPM_TIMEOUT_RANGE_US);
+ usleep_range((delay_msec * 1000) - TPM_TIMEOUT_RANGE_US,
+ delay_msec * 1000);
};
struct tpm_chip *tpm_chip_find_get(int chip_num);
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/5] tpm: move wait_for_tpm_stat() to respective driver files
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-10 14:57 ` Jarkko Sakkinen
-1 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-10 14:57 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On Wed, Oct 04, 2017 at 06:29:20AM -0400, Nayna Jain wrote:
> The function wait_for_tpm_stat() is currently defined in
> tpm-interface file. It is a hardware specific function used
> only by tpm_tis and xen-tpmfront, so it is removed from
> tpm-interface.c and defined in respective driver files.
>
> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm-interface.c | 60 ----------------------------------------
> drivers/char/tpm/tpm.h | 2 --
> drivers/char/tpm/tpm_tis_core.c | 60 ++++++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 120 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1d6729be4cd6..313f7618d569 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1035,66 +1035,6 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
> }
> EXPORT_SYMBOL_GPL(tpm_send);
>
> -static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> - bool check_cancel, bool *canceled)
> -{
> - u8 status = chip->ops->status(chip);
> -
> - *canceled = false;
> - if ((status & mask) == mask)
> - return true;
> - if (check_cancel && chip->ops->req_canceled(chip, status)) {
> - *canceled = true;
> - return true;
> - }
> - return false;
> -}
> -
> -int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> - wait_queue_head_t *queue, bool check_cancel)
> -{
> - unsigned long stop;
> - long rc;
> - u8 status;
> - bool canceled = false;
> -
> - /* check current status */
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> -
> - stop = jiffies + timeout;
> -
> - if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> -again:
> - timeout = stop - jiffies;
> - if ((long)timeout <= 0)
> - return -ETIME;
> - rc = wait_event_interruptible_timeout(*queue,
> - wait_for_tpm_stat_cond(chip, mask, check_cancel,
> - &canceled),
> - timeout);
> - if (rc > 0) {
> - if (canceled)
> - return -ECANCELED;
> - return 0;
> - }
> - if (rc == -ERESTARTSYS && freezing(current)) {
> - clear_thread_flag(TIF_SIGPENDING);
> - goto again;
> - }
> - } else {
> - do {
> - tpm_msleep(TPM_TIMEOUT);
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> - } while (time_before(jiffies, stop));
> - }
> - return -ETIME;
> -}
> -EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
> -
> #define TPM_ORD_SAVESTATE 152
> #define SAVESTATE_RESULT_SIZE 10
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 2d5466a72e40..4fc83ac7abeb 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -525,8 +525,6 @@ int tpm_do_selftest(struct tpm_chip *chip);
> unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> int tpm_pm_suspend(struct device *dev);
> int tpm_pm_resume(struct device *dev);
> -int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> - wait_queue_head_t *queue, bool check_cancel);
>
> static inline void tpm_msleep(unsigned int delay_msec)
> {
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 63bc6c3b949e..b33126a35694 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -31,6 +31,66 @@
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> +static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> + bool check_cancel, bool *canceled)
> +{
> + u8 status = chip->ops->status(chip);
> +
> + *canceled = false;
> + if ((status & mask) == mask)
> + return true;
> + if (check_cancel && chip->ops->req_canceled(chip, status)) {
> + *canceled = true;
> + return true;
> + }
> + return false;
> +}
> +
> +static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> + unsigned long timeout, wait_queue_head_t *queue,
> + bool check_cancel)
> +{
> + unsigned long stop;
> + long rc;
> + u8 status;
> + bool canceled = false;
> +
> + /* check current status */
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> +
> + stop = jiffies + timeout;
> +
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> +again:
> + timeout = stop - jiffies;
> + if ((long)timeout <= 0)
> + return -ETIME;
> + rc = wait_event_interruptible_timeout(*queue,
> + wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + &canceled),
> + timeout);
> + if (rc > 0) {
> + if (canceled)
> + return -ECANCELED;
> + return 0;
> + }
> + if (rc == -ERESTARTSYS && freezing(current)) {
> + clear_thread_flag(TIF_SIGPENDING);
> + goto again;
> + }
> + } else {
> + do {
> + tpm_msleep(TPM_TIMEOUT);
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (time_before(jiffies, stop));
> + }
> + return -ETIME;
> +}
> +
> /* Before we attempt to access the TPM we must see that the valid bit is set.
> * The specification says that this bit is 0 at reset and remains 0 until the
> * 'TPM has gone through its self test and initialization and has established
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 656e8af95d52..d20a0a9ded27 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -39,6 +39,66 @@ enum status_bits {
> VTPM_STATUS_CANCELED = 0x8,
> };
>
> +static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> + bool check_cancel, bool *canceled)
> +{
> + u8 status = chip->ops->status(chip);
> +
> + *canceled = false;
> + if ((status & mask) == mask)
> + return true;
> + if (check_cancel && chip->ops->req_canceled(chip, status)) {
> + *canceled = true;
> + return true;
> + }
> + return false;
> +}
> +
> +static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> + unsigned long timeout, wait_queue_head_t *queue,
> + bool check_cancel)
> +{
> + unsigned long stop;
> + long rc;
> + u8 status;
> + bool canceled = false;
> +
> + /* check current status */
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> +
> + stop = jiffies + timeout;
> +
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> +again:
> + timeout = stop - jiffies;
> + if ((long)timeout <= 0)
> + return -ETIME;
> + rc = wait_event_interruptible_timeout(*queue,
> + wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + &canceled),
> + timeout);
> + if (rc > 0) {
> + if (canceled)
> + return -ECANCELED;
> + return 0;
> + }
> + if (rc == -ERESTARTSYS && freezing(current)) {
> + clear_thread_flag(TIF_SIGPENDING);
> + goto again;
> + }
> + } else {
> + do {
> + tpm_msleep(TPM_TIMEOUT);
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (time_before(jiffies, stop));
> + }
> + return -ETIME;
> +}
> +
> static u8 vtpm_status(struct tpm_chip *chip)
> {
> struct tpm_private *priv = dev_get_drvdata(&chip->dev);
> --
> 2.13.3
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 1/5] tpm: move wait_for_tpm_stat() to respective driver files
@ 2017-10-10 14:57 ` Jarkko Sakkinen
0 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-10 14:57 UTC (permalink / raw)
To: linux-security-module
On Wed, Oct 04, 2017 at 06:29:20AM -0400, Nayna Jain wrote:
> The function wait_for_tpm_stat() is currently defined in
> tpm-interface file. It is a hardware specific function used
> only by tpm_tis and xen-tpmfront, so it is removed from
> tpm-interface.c and defined in respective driver files.
>
> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm-interface.c | 60 ----------------------------------------
> drivers/char/tpm/tpm.h | 2 --
> drivers/char/tpm/tpm_tis_core.c | 60 ++++++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/xen-tpmfront.c | 60 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 120 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1d6729be4cd6..313f7618d569 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1035,66 +1035,6 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
> }
> EXPORT_SYMBOL_GPL(tpm_send);
>
> -static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> - bool check_cancel, bool *canceled)
> -{
> - u8 status = chip->ops->status(chip);
> -
> - *canceled = false;
> - if ((status & mask) == mask)
> - return true;
> - if (check_cancel && chip->ops->req_canceled(chip, status)) {
> - *canceled = true;
> - return true;
> - }
> - return false;
> -}
> -
> -int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> - wait_queue_head_t *queue, bool check_cancel)
> -{
> - unsigned long stop;
> - long rc;
> - u8 status;
> - bool canceled = false;
> -
> - /* check current status */
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> -
> - stop = jiffies + timeout;
> -
> - if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> -again:
> - timeout = stop - jiffies;
> - if ((long)timeout <= 0)
> - return -ETIME;
> - rc = wait_event_interruptible_timeout(*queue,
> - wait_for_tpm_stat_cond(chip, mask, check_cancel,
> - &canceled),
> - timeout);
> - if (rc > 0) {
> - if (canceled)
> - return -ECANCELED;
> - return 0;
> - }
> - if (rc == -ERESTARTSYS && freezing(current)) {
> - clear_thread_flag(TIF_SIGPENDING);
> - goto again;
> - }
> - } else {
> - do {
> - tpm_msleep(TPM_TIMEOUT);
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> - } while (time_before(jiffies, stop));
> - }
> - return -ETIME;
> -}
> -EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
> -
> #define TPM_ORD_SAVESTATE 152
> #define SAVESTATE_RESULT_SIZE 10
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 2d5466a72e40..4fc83ac7abeb 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -525,8 +525,6 @@ int tpm_do_selftest(struct tpm_chip *chip);
> unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> int tpm_pm_suspend(struct device *dev);
> int tpm_pm_resume(struct device *dev);
> -int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> - wait_queue_head_t *queue, bool check_cancel);
>
> static inline void tpm_msleep(unsigned int delay_msec)
> {
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 63bc6c3b949e..b33126a35694 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -31,6 +31,66 @@
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> +static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> + bool check_cancel, bool *canceled)
> +{
> + u8 status = chip->ops->status(chip);
> +
> + *canceled = false;
> + if ((status & mask) == mask)
> + return true;
> + if (check_cancel && chip->ops->req_canceled(chip, status)) {
> + *canceled = true;
> + return true;
> + }
> + return false;
> +}
> +
> +static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> + unsigned long timeout, wait_queue_head_t *queue,
> + bool check_cancel)
> +{
> + unsigned long stop;
> + long rc;
> + u8 status;
> + bool canceled = false;
> +
> + /* check current status */
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> +
> + stop = jiffies + timeout;
> +
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> +again:
> + timeout = stop - jiffies;
> + if ((long)timeout <= 0)
> + return -ETIME;
> + rc = wait_event_interruptible_timeout(*queue,
> + wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + &canceled),
> + timeout);
> + if (rc > 0) {
> + if (canceled)
> + return -ECANCELED;
> + return 0;
> + }
> + if (rc == -ERESTARTSYS && freezing(current)) {
> + clear_thread_flag(TIF_SIGPENDING);
> + goto again;
> + }
> + } else {
> + do {
> + tpm_msleep(TPM_TIMEOUT);
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (time_before(jiffies, stop));
> + }
> + return -ETIME;
> +}
> +
> /* Before we attempt to access the TPM we must see that the valid bit is set.
> * The specification says that this bit is 0 at reset and remains 0 until the
> * 'TPM has gone through its self test and initialization and has established
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 656e8af95d52..d20a0a9ded27 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -39,6 +39,66 @@ enum status_bits {
> VTPM_STATUS_CANCELED = 0x8,
> };
>
> +static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> + bool check_cancel, bool *canceled)
> +{
> + u8 status = chip->ops->status(chip);
> +
> + *canceled = false;
> + if ((status & mask) == mask)
> + return true;
> + if (check_cancel && chip->ops->req_canceled(chip, status)) {
> + *canceled = true;
> + return true;
> + }
> + return false;
> +}
> +
> +static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> + unsigned long timeout, wait_queue_head_t *queue,
> + bool check_cancel)
> +{
> + unsigned long stop;
> + long rc;
> + u8 status;
> + bool canceled = false;
> +
> + /* check current status */
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> +
> + stop = jiffies + timeout;
> +
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> +again:
> + timeout = stop - jiffies;
> + if ((long)timeout <= 0)
> + return -ETIME;
> + rc = wait_event_interruptible_timeout(*queue,
> + wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + &canceled),
> + timeout);
> + if (rc > 0) {
> + if (canceled)
> + return -ECANCELED;
> + return 0;
> + }
> + if (rc == -ERESTARTSYS && freezing(current)) {
> + clear_thread_flag(TIF_SIGPENDING);
> + goto again;
> + }
> + } else {
> + do {
> + tpm_msleep(TPM_TIMEOUT);
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (time_before(jiffies, stop));
> + }
> + return -ETIME;
> +}
> +
> static u8 vtpm_status(struct tpm_chip *chip)
> {
> struct tpm_private *priv = dev_get_drvdata(&chip->dev);
> --
> 2.13.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-10 15:04 ` Jarkko Sakkinen
-1 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-10 15:04 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On Wed, Oct 04, 2017 at 06:29:21AM -0400, Nayna Jain wrote:
> The TPM burstcount status indicates the number of bytes that can
> be sent to the TPM without causing bus wait states. Effectively,
> it is the number of empty bytes in the command FIFO. Further,
> some TPMs have a static burstcount, when the value remains zero
> until the entire FIFO is empty.
>
> This patch optimizes the tpm_tis_send_data() function by checking
> the burstcount only once. And if the burstcount is valid, it writes
> all the bytes at once, permitting wait states. The performance of a
> 34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
> 41msec to 14msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~41sec to ~14sec.
>
> Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
> conjunction with the TPM Device Driver work group.
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
> 1 file changed, 16 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index b33126a35694..8da425e1783f 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> int rc, status, burstcnt;
> - size_t count = 0;
> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>
> status = tpm_tis_status(chip);
> @@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
> }
> }
>
> - while (count < len - 1) {
> - burstcnt = get_burstcount(chip);
> - if (burstcnt < 0) {
> - dev_err(&chip->dev, "Unable to read burstcount\n");
> - rc = burstcnt;
> - goto out_err;
> - }
> - burstcnt = min_t(int, burstcnt, len - count - 1);
> - rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> - burstcnt, buf + count);
> - if (rc < 0)
> - goto out_err;
> + /*
> + * Get the initial burstcount to ensure TPM is ready to
> + * accept data.
> + */
> + burstcnt = get_burstcount(chip);
> + if (burstcnt < 0) {
> + dev_err(&chip->dev, "Unable to read burstcount\n");
> + rc = burstcnt;
> + goto out_err;
> + }
>
> - count += burstcnt;
> + burstcnt = len - 1;
>
> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> - &priv->int_queue, false) < 0) {
> - rc = -ETIME;
> - goto out_err;
> - }
> - status = tpm_tis_status(chip);
> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
> - rc = -EIO;
> - goto out_err;
> - }
> - }
> + rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> + burstcnt, buf);
Otherwise, this looks good but I don't understand why you assign 'len -
1' to 'brustcnt' and pass it to tpm_tis_write_bytes() instead of just
passing 'len - 1'. I mean no relation to burst count, right?
/Jarkko
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
@ 2017-10-10 15:04 ` Jarkko Sakkinen
0 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-10 15:04 UTC (permalink / raw)
To: linux-security-module
On Wed, Oct 04, 2017 at 06:29:21AM -0400, Nayna Jain wrote:
> The TPM burstcount status indicates the number of bytes that can
> be sent to the TPM without causing bus wait states. Effectively,
> it is the number of empty bytes in the command FIFO. Further,
> some TPMs have a static burstcount, when the value remains zero
> until the entire FIFO is empty.
>
> This patch optimizes the tpm_tis_send_data() function by checking
> the burstcount only once. And if the burstcount is valid, it writes
> all the bytes at once, permitting wait states. The performance of a
> 34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
> 41msec to 14msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~41sec to ~14sec.
>
> Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
> conjunction with the TPM Device Driver work group.
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
> 1 file changed, 16 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index b33126a35694..8da425e1783f 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> int rc, status, burstcnt;
> - size_t count = 0;
> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>
> status = tpm_tis_status(chip);
> @@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
> }
> }
>
> - while (count < len - 1) {
> - burstcnt = get_burstcount(chip);
> - if (burstcnt < 0) {
> - dev_err(&chip->dev, "Unable to read burstcount\n");
> - rc = burstcnt;
> - goto out_err;
> - }
> - burstcnt = min_t(int, burstcnt, len - count - 1);
> - rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> - burstcnt, buf + count);
> - if (rc < 0)
> - goto out_err;
> + /*
> + * Get the initial burstcount to ensure TPM is ready to
> + * accept data.
> + */
> + burstcnt = get_burstcount(chip);
> + if (burstcnt < 0) {
> + dev_err(&chip->dev, "Unable to read burstcount\n");
> + rc = burstcnt;
> + goto out_err;
> + }
>
> - count += burstcnt;
> + burstcnt = len - 1;
>
> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> - &priv->int_queue, false) < 0) {
> - rc = -ETIME;
> - goto out_err;
> - }
> - status = tpm_tis_status(chip);
> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
> - rc = -EIO;
> - goto out_err;
> - }
> - }
> + rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> + burstcnt, buf);
Otherwise, this looks good but I don't understand why you assign 'len -
1' to 'brustcnt' and pass it to tpm_tis_write_bytes() instead of just
passing 'len - 1'. I mean no relation to burst count, right?
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
2017-10-10 15:04 ` Jarkko Sakkinen
(?)
@ 2017-10-11 11:29 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-11 11:29 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On 10/10/2017 08:34 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:21AM -0400, Nayna Jain wrote:
>> The TPM burstcount status indicates the number of bytes that can
>> be sent to the TPM without causing bus wait states. Effectively,
>> it is the number of empty bytes in the command FIFO. Further,
>> some TPMs have a static burstcount, when the value remains zero
>> until the entire FIFO is empty.
>>
>> This patch optimizes the tpm_tis_send_data() function by checking
>> the burstcount only once. And if the burstcount is valid, it writes
>> all the bytes at once, permitting wait states. The performance of a
>> 34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
>> 41msec to 14msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~41sec to ~14sec.
>>
>> Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
>> conjunction with the TPM Device Driver work group.
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index b33126a35694..8da425e1783f 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> {
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> int rc, status, burstcnt;
>> - size_t count = 0;
>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>>
>> status = tpm_tis_status(chip);
>> @@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> }
>> }
>>
>> - while (count < len - 1) {
>> - burstcnt = get_burstcount(chip);
>> - if (burstcnt < 0) {
>> - dev_err(&chip->dev, "Unable to read burstcount\n");
>> - rc = burstcnt;
>> - goto out_err;
>> - }
>> - burstcnt = min_t(int, burstcnt, len - count - 1);
>> - rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> - burstcnt, buf + count);
>> - if (rc < 0)
>> - goto out_err;
>> + /*
>> + * Get the initial burstcount to ensure TPM is ready to
>> + * accept data.
>> + */
>> + burstcnt = get_burstcount(chip);
>> + if (burstcnt < 0) {
>> + dev_err(&chip->dev, "Unable to read burstcount\n");
>> + rc = burstcnt;
>> + goto out_err;
>> + }
>>
>> - count += burstcnt;
>> + burstcnt = len - 1;
>>
>> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
>> - &priv->int_queue, false) < 0) {
>> - rc = -ETIME;
>> - goto out_err;
>> - }
>> - status = tpm_tis_status(chip);
>> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
>> - rc = -EIO;
>> - goto out_err;
>> - }
>> - }
>> + rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> + burstcnt, buf);
> Otherwise, this looks good but I don't understand why you assign 'len -
> 1' to 'brustcnt' and pass it to tpm_tis_write_bytes() instead of just
> passing 'len - 1'. I mean no relation to burst count, right?
Yeah, I can just send 'len - 1'. Will do this change.
Thanks & Regards,
- Nayna
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
@ 2017-10-11 11:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-11 11:29 UTC (permalink / raw)
To: linux-security-module
On 10/10/2017 08:34 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:21AM -0400, Nayna Jain wrote:
>> The TPM burstcount status indicates the number of bytes that can
>> be sent to the TPM without causing bus wait states. Effectively,
>> it is the number of empty bytes in the command FIFO. Further,
>> some TPMs have a static burstcount, when the value remains zero
>> until the entire FIFO is empty.
>>
>> This patch optimizes the tpm_tis_send_data() function by checking
>> the burstcount only once. And if the burstcount is valid, it writes
>> all the bytes at once, permitting wait states. The performance of a
>> 34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
>> 41msec to 14msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~41sec to ~14sec.
>>
>> Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
>> conjunction with the TPM Device Driver work group.
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index b33126a35694..8da425e1783f 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> {
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> int rc, status, burstcnt;
>> - size_t count = 0;
>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>>
>> status = tpm_tis_status(chip);
>> @@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> }
>> }
>>
>> - while (count < len - 1) {
>> - burstcnt = get_burstcount(chip);
>> - if (burstcnt < 0) {
>> - dev_err(&chip->dev, "Unable to read burstcount\n");
>> - rc = burstcnt;
>> - goto out_err;
>> - }
>> - burstcnt = min_t(int, burstcnt, len - count - 1);
>> - rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> - burstcnt, buf + count);
>> - if (rc < 0)
>> - goto out_err;
>> + /*
>> + * Get the initial burstcount to ensure TPM is ready to
>> + * accept data.
>> + */
>> + burstcnt = get_burstcount(chip);
>> + if (burstcnt < 0) {
>> + dev_err(&chip->dev, "Unable to read burstcount\n");
>> + rc = burstcnt;
>> + goto out_err;
>> + }
>>
>> - count += burstcnt;
>> + burstcnt = len - 1;
>>
>> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
>> - &priv->int_queue, false) < 0) {
>> - rc = -ETIME;
>> - goto out_err;
>> - }
>> - status = tpm_tis_status(chip);
>> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
>> - rc = -EIO;
>> - goto out_err;
>> - }
>> - }
>> + rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> + burstcnt, buf);
> Otherwise, this looks good but I don't understand why you assign 'len -
> 1' to 'brustcnt' and pass it to tpm_tis_write_bytes() instead of just
> passing 'len - 1'. I mean no relation to burst count, right?
Yeah, I can just send 'len - 1'. Will do this change.
Thanks & Regards,
???? - Nayna
>
> /Jarkko
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance
@ 2017-10-11 11:29 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-11 11:29 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On 10/10/2017 08:34 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:21AM -0400, Nayna Jain wrote:
>> The TPM burstcount status indicates the number of bytes that can
>> be sent to the TPM without causing bus wait states. Effectively,
>> it is the number of empty bytes in the command FIFO. Further,
>> some TPMs have a static burstcount, when the value remains zero
>> until the entire FIFO is empty.
>>
>> This patch optimizes the tpm_tis_send_data() function by checking
>> the burstcount only once. And if the burstcount is valid, it writes
>> all the bytes at once, permitting wait states. The performance of a
>> 34 byte extend on a TPM 1.2 with an 8 byte burstcount improved from
>> 41msec to 14msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~41sec to ~14sec.
>>
>> Suggested-by: Ken Goldman <kgold@linux.vnet.ibm.com> in
>> conjunction with the TPM Device Driver work group.
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 42 ++++++++++++++++-------------------------
>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index b33126a35694..8da425e1783f 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -316,7 +316,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> {
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> int rc, status, burstcnt;
>> - size_t count = 0;
>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>>
>> status = tpm_tis_status(chip);
>> @@ -330,35 +329,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
>> }
>> }
>>
>> - while (count < len - 1) {
>> - burstcnt = get_burstcount(chip);
>> - if (burstcnt < 0) {
>> - dev_err(&chip->dev, "Unable to read burstcount\n");
>> - rc = burstcnt;
>> - goto out_err;
>> - }
>> - burstcnt = min_t(int, burstcnt, len - count - 1);
>> - rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> - burstcnt, buf + count);
>> - if (rc < 0)
>> - goto out_err;
>> + /*
>> + * Get the initial burstcount to ensure TPM is ready to
>> + * accept data.
>> + */
>> + burstcnt = get_burstcount(chip);
>> + if (burstcnt < 0) {
>> + dev_err(&chip->dev, "Unable to read burstcount\n");
>> + rc = burstcnt;
>> + goto out_err;
>> + }
>>
>> - count += burstcnt;
>> + burstcnt = len - 1;
>>
>> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
>> - &priv->int_queue, false) < 0) {
>> - rc = -ETIME;
>> - goto out_err;
>> - }
>> - status = tpm_tis_status(chip);
>> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
>> - rc = -EIO;
>> - goto out_err;
>> - }
>> - }
>> + rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
>> + burstcnt, buf);
> Otherwise, this looks good but I don't understand why you assign 'len -
> 1' to 'brustcnt' and pass it to tpm_tis_write_bytes() instead of just
> passing 'len - 1'. I mean no relation to burst count, right?
Yeah, I can just send 'len - 1'. Will do this change.
Thanks & Regards,
- Nayna
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 3/5] tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-12 11:17 ` Jarkko Sakkinen
-1 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:17 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On Wed, Oct 04, 2017 at 06:29:22AM -0400, Nayna Jain wrote:
> The existing wait_for_tpm_stat() polls for the chip status after
> 5msec sleep. As per TCG ddwg input, it is expected that tpm might
> return status in few usec. So, reducing the delay in polling to
> 1msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~14sec to ~10sec.
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 8da425e1783f..224842e06105 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -31,6 +31,8 @@
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> +#define TPM_POLL_SLEEP 1
Should have a comment above that explains the choice.
> +
> static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> bool check_cancel, bool *canceled)
> {
> @@ -82,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> }
> } else {
> do {
> - tpm_msleep(TPM_TIMEOUT);
> + tpm_msleep(TPM_POLL_SLEEP);
> status = chip->ops->status(chip);
> if ((status & mask) == mask)
> return 0;
> --
> 2.13.3
>
/Jarkko
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 3/5] tpm: reduce polling delay in tpm_tis wait_for_tpm_stat()
@ 2017-10-12 11:17 ` Jarkko Sakkinen
0 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:17 UTC (permalink / raw)
To: linux-security-module
On Wed, Oct 04, 2017 at 06:29:22AM -0400, Nayna Jain wrote:
> The existing wait_for_tpm_stat() polls for the chip status after
> 5msec sleep. As per TCG ddwg input, it is expected that tpm might
> return status in few usec. So, reducing the delay in polling to
> 1msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~14sec to ~10sec.
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 8da425e1783f..224842e06105 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -31,6 +31,8 @@
> #include "tpm.h"
> #include "tpm_tis_core.h"
>
> +#define TPM_POLL_SLEEP 1
Should have a comment above that explains the choice.
> +
> static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> bool check_cancel, bool *canceled)
> {
> @@ -82,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> }
> } else {
> do {
> - tpm_msleep(TPM_TIMEOUT);
> + tpm_msleep(TPM_POLL_SLEEP);
> status = chip->ops->status(chip);
> if ((status & mask) == mask)
> return 0;
> --
> 2.13.3
>
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-12 11:18 ` Jarkko Sakkinen
-1 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:18 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On Wed, Oct 04, 2017 at 06:29:23AM -0400, Nayna Jain wrote:
> Currently, get_burstcount() function sleeps for 5msec in a loop
> before retrying for next query to burstcount. However, if it takes
> lesser time for TPM to return, this 5msec delay is longer
> than necessary.
>
> This patch replaces the tpm_msleep time from 5msec to 1msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~10sec to ~9sec.
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 224842e06105..826a0b9c9201 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
> burstcnt = (value >> 8) & 0xFFFF;
> if (burstcnt)
> return burstcnt;
> - tpm_msleep(TPM_TIMEOUT);
> + tpm_msleep(TPM_POLL_SLEEP);
> } while (time_before(jiffies, stop));
> return -EBUSY;
> }
> --
> 2.13.3
>
Would it make sense to squash this to 3/5 and merge the commit
messages?
/Jarkko
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
@ 2017-10-12 11:18 ` Jarkko Sakkinen
0 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:18 UTC (permalink / raw)
To: linux-security-module
On Wed, Oct 04, 2017 at 06:29:23AM -0400, Nayna Jain wrote:
> Currently, get_burstcount() function sleeps for 5msec in a loop
> before retrying for next query to burstcount. However, if it takes
> lesser time for TPM to return, this 5msec delay is longer
> than necessary.
>
> This patch replaces the tpm_msleep time from 5msec to 1msec.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~10sec to ~9sec.
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 224842e06105..826a0b9c9201 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
> burstcnt = (value >> 8) & 0xFFFF;
> if (burstcnt)
> return burstcnt;
> - tpm_msleep(TPM_TIMEOUT);
> + tpm_msleep(TPM_POLL_SLEEP);
> } while (time_before(jiffies, stop));
> return -EBUSY;
> }
> --
> 2.13.3
>
Would it make sense to squash this to 3/5 and merge the commit
messages?
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 5/5] tpm: use tpm_msleep() value as max delay
2017-10-04 10:29 ` Nayna Jain
@ 2017-10-12 11:19 ` Jarkko Sakkinen
-1 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:19 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On Wed, Oct 04, 2017 at 06:29:24AM -0400, Nayna Jain wrote:
> Currently, tpm_msleep() uses delay_msec as the minimum value in
> usleep_range. However, that is the maximum time we want to wait.
> The function is modified to use the delay_msec as the maximum
> value, not the minimum value.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~9sec to ~8sec.
>
> Fixes: 3b9af007869("tpm: replace msleep() with usleep_range() in TPM 1.2/
> 2.0 generic drivers")
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4fc83ac7abeb..644de70de2cc 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -528,8 +528,8 @@ int tpm_pm_resume(struct device *dev);
>
> static inline void tpm_msleep(unsigned int delay_msec)
> {
> - usleep_range(delay_msec * 1000,
> - (delay_msec * 1000) + TPM_TIMEOUT_RANGE_US);
> + usleep_range((delay_msec * 1000) - TPM_TIMEOUT_RANGE_US,
> + delay_msec * 1000);
> };
>
> struct tpm_chip *tpm_chip_find_get(int chip_num);
> --
> 2.13.3
>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 5/5] tpm: use tpm_msleep() value as max delay
@ 2017-10-12 11:19 ` Jarkko Sakkinen
0 siblings, 0 replies; 28+ messages in thread
From: Jarkko Sakkinen @ 2017-10-12 11:19 UTC (permalink / raw)
To: linux-security-module
On Wed, Oct 04, 2017 at 06:29:24AM -0400, Nayna Jain wrote:
> Currently, tpm_msleep() uses delay_msec as the minimum value in
> usleep_range. However, that is the maximum time we want to wait.
> The function is modified to use the delay_msec as the maximum
> value, not the minimum value.
>
> After this change, performance on a TPM 1.2 with an 8 byte
> burstcount for 1000 extends improved from ~9sec to ~8sec.
>
> Fixes: 3b9af007869("tpm: replace msleep() with usleep_range() in TPM 1.2/
> 2.0 generic drivers")
>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4fc83ac7abeb..644de70de2cc 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -528,8 +528,8 @@ int tpm_pm_resume(struct device *dev);
>
> static inline void tpm_msleep(unsigned int delay_msec)
> {
> - usleep_range(delay_msec * 1000,
> - (delay_msec * 1000) + TPM_TIMEOUT_RANGE_US);
> + usleep_range((delay_msec * 1000) - TPM_TIMEOUT_RANGE_US,
> + delay_msec * 1000);
> };
>
> struct tpm_chip *tpm_chip_find_get(int chip_num);
> --
> 2.13.3
>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
2017-10-12 11:18 ` Jarkko Sakkinen
(?)
@ 2017-10-12 13:14 ` Nayna Jain
-1 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-12 13:14 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On 10/12/2017 04:48 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:23AM -0400, Nayna Jain wrote:
>> Currently, get_burstcount() function sleeps for 5msec in a loop
>> before retrying for next query to burstcount. However, if it takes
>> lesser time for TPM to return, this 5msec delay is longer
>> than necessary.
>>
>> This patch replaces the tpm_msleep time from 5msec to 1msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~10sec to ~9sec.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 224842e06105..826a0b9c9201 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
>> burstcnt = (value >> 8) & 0xFFFF;
>> if (burstcnt)
>> return burstcnt;
>> - tpm_msleep(TPM_TIMEOUT);
>> + tpm_msleep(TPM_POLL_SLEEP);
>> } while (time_before(jiffies, stop));
>> return -EBUSY;
>> }
>> --
>> 2.13.3
>>
> Would it make sense to squash this to 3/5 and merge the commit
> messages?
Yeah.. it sounds reasonable.. both are reducing the sleep delay time for
same purpose, just in different functions.
Thanks & Regards,
- Nayna
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
@ 2017-10-12 13:14 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-12 13:14 UTC (permalink / raw)
To: linux-security-module
On 10/12/2017 04:48 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:23AM -0400, Nayna Jain wrote:
>> Currently, get_burstcount() function sleeps for 5msec in a loop
>> before retrying for next query to burstcount. However, if it takes
>> lesser time for TPM to return, this 5msec delay is longer
>> than necessary.
>>
>> This patch replaces the tpm_msleep time from 5msec to 1msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~10sec to ~9sec.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 224842e06105..826a0b9c9201 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
>> burstcnt = (value >> 8) & 0xFFFF;
>> if (burstcnt)
>> return burstcnt;
>> - tpm_msleep(TPM_TIMEOUT);
>> + tpm_msleep(TPM_POLL_SLEEP);
>> } while (time_before(jiffies, stop));
>> return -EBUSY;
>> }
>> --
>> 2.13.3
>>
> Would it make sense to squash this to 3/5 and merge the commit
> messages?
Yeah.. it sounds reasonable.. both are reducing the sleep delay time for
same purpose, just in different functions.
Thanks & Regards,
??? - Nayna
>
> /Jarkko
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount()
@ 2017-10-12 13:14 ` Nayna Jain
0 siblings, 0 replies; 28+ messages in thread
From: Nayna Jain @ 2017-10-12 13:14 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, zohar, linux-security-module, linux-kernel,
peterhuewe, tpmdd, jgunthorpe, patrickc
On 10/12/2017 04:48 PM, Jarkko Sakkinen wrote:
> On Wed, Oct 04, 2017 at 06:29:23AM -0400, Nayna Jain wrote:
>> Currently, get_burstcount() function sleeps for 5msec in a loop
>> before retrying for next query to burstcount. However, if it takes
>> lesser time for TPM to return, this 5msec delay is longer
>> than necessary.
>>
>> This patch replaces the tpm_msleep time from 5msec to 1msec.
>>
>> After this change, performance on a TPM 1.2 with an 8 byte
>> burstcount for 1000 extends improved from ~10sec to ~9sec.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 224842e06105..826a0b9c9201 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -226,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
>> burstcnt = (value >> 8) & 0xFFFF;
>> if (burstcnt)
>> return burstcnt;
>> - tpm_msleep(TPM_TIMEOUT);
>> + tpm_msleep(TPM_POLL_SLEEP);
>> } while (time_before(jiffies, stop));
>> return -EBUSY;
>> }
>> --
>> 2.13.3
>>
> Would it make sense to squash this to 3/5 and merge the commit
> messages?
Yeah.. it sounds reasonable.. both are reducing the sleep delay time for
same purpose, just in different functions.
Thanks & Regards,
- Nayna
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-10-12 13:14 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-04 10:29 [PATCH v3 0/4] additional TPM performance improvements Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-04 10:29 ` [PATCH v3 1/5] tpm: move wait_for_tpm_stat() to respective driver files Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-10 14:57 ` Jarkko Sakkinen
2017-10-10 14:57 ` Jarkko Sakkinen
2017-10-04 10:29 ` [PATCH v3 2/5] tpm: ignore burstcount to improve tpm_tis send() performance Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-10 15:04 ` Jarkko Sakkinen
2017-10-10 15:04 ` Jarkko Sakkinen
2017-10-11 11:29 ` Nayna Jain
2017-10-11 11:29 ` Nayna Jain
2017-10-11 11:29 ` Nayna Jain
2017-10-04 10:29 ` [PATCH v3 3/5] tpm: reduce polling delay in tpm_tis wait_for_tpm_stat() Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-12 11:17 ` Jarkko Sakkinen
2017-10-12 11:17 ` Jarkko Sakkinen
2017-10-04 10:29 ` [PATCH v3 4/5] tpm: reduce tpm_msleep() time in get_burstcount() Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-12 11:18 ` Jarkko Sakkinen
2017-10-12 11:18 ` Jarkko Sakkinen
2017-10-12 13:14 ` Nayna Jain
2017-10-12 13:14 ` Nayna Jain
2017-10-12 13:14 ` Nayna Jain
2017-10-04 10:29 ` [PATCH v3 5/5] tpm: use tpm_msleep() value as max delay Nayna Jain
2017-10-04 10:29 ` Nayna Jain
2017-10-12 11:19 ` Jarkko Sakkinen
2017-10-12 11:19 ` Jarkko Sakkinen
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.