The TCG SAPI specification [1] defines a set of functions, which allow
applications to use the TPM device in either blocking or non-blocking fashion.
Each command defined by the specification has a corresponding
Tss2_Sys_<COMMAND>_Prepare() and Tss2_Sys_<COMMAND>_Complete() call, which
together with Tss2_Sys_ExecuteAsync() is designed to allow asynchronous
mode of operation. Currently the TPM driver supports only blocking calls,
which doesn't allow asynchronous IO operations.
This patch changes it and adds support for nonblocking write and a new poll
function to enable applications, which want to take advantage of this feature.
The new functionality can be tested using standard TPM tools implemented
in [2], together with modified TCTI from [3], and an example application
by Philip Tricca [4]. Here is a short description from Philip:
"The example application `glib-tss2-event` uses a glib main event loop
to create an RSA 2048 primary key in the TPM2 NULL hierarchy while
using a glib timer event to time the operation. A GSource object is
used to generate an event when the FD underlying the tss2 function
call has data ready. While the application waits for an event indicating
that the CreatePrimary operation is complete, it counts timer events
that occur every 100ms. Once the CreatePrimary operation completes the
number of timer events that occurred is used to make a rough calculation
of the elapsed time. This value is then printed to the console.
This takes ~300 lines of C code and requires no management or
synchronization of threads. The glib GMainContext is "just a poll()
loop" according to the glib documentation here:
https://developer.gnome.org/programming-guidelines/stable/main-contexts.html.en
and so supporting 'poll' is the easiest way to integrate with glib /
gtk+. This is true of any other event system that relies on 'poll'
instead of worker threads."
[1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_SAPI_Version-1.1_Revision-22_review_030918.pdf
[2] https://github.com/tpm2-software/tpm2-tools
[3] https://github.com/tstruk/tpm2-tss/tree/async
[4] https://github.com/flihp/glib-tss2-async-example
---
Changes in v5:
- Changed the workqueue allocation time back from the first user interface
open to module init.
Changes in v4:
- Changed the way buffer_mutex is handled in nonblocking mode so that
it is not held when write() returns to user space.
Changes in v3:
- Fixed problem reported by 0-dey kbuild test robot around __exitcall.
It complained because there is a module_exit() in another file already.
- Added info on example application from Philip
Changes in v2:
- Split the change into two separate patches. First patch adds a pointer
to the space to the struct file_priv to have access to it from the async job.
This is to avoid memory allocations on every write call. Now everything
what's needed is in the file_priv struct.
- Renamed the 'work' member of the timer to avoid confusion.
Now there are 'timeout_work' and 'async_work'.
- Removed the global wait queue and moved it to file_priv.
- Only creating the work queue when the first file is opened.
Tadeusz Struk (2):
tpm: add ptr to the tpm_space struct to file_priv
tpm: add support for nonblocking operation
drivers/char/tpm/tpm-dev-common.c | 150 +++++++++++++++++++++++++++----------
drivers/char/tpm/tpm-dev.c | 22 +++--
drivers/char/tpm/tpm-dev.h | 19 +++--
drivers/char/tpm/tpm-interface.c | 1
drivers/char/tpm/tpm.h | 1
drivers/char/tpm/tpmrm-dev.c | 31 ++++----
6 files changed, 152 insertions(+), 72 deletions(-)
--
TS
Add a ptr to struct tpm_space to the file_priv to have an easy
access to it in the async job without the need to allocate memory.
This also allows to consolidate of the write operations for
the two interfaces.
Tested-by: Philip Tricca <[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
drivers/char/tpm/tpm-dev-common.c | 8 +++++---
drivers/char/tpm/tpm-dev.c | 10 ++--------
drivers/char/tpm/tpm-dev.h | 5 +++--
drivers/char/tpm/tpmrm-dev.c | 14 ++------------
4 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index e4a04b2d3c32..f0c033b69b62 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -43,9 +43,11 @@ static void timeout_work(struct work_struct *work)
}
void tpm_common_open(struct file *file, struct tpm_chip *chip,
- struct file_priv *priv)
+ struct file_priv *priv, struct tpm_space *space)
{
priv->chip = chip;
+ priv->space = space;
+
mutex_init(&priv->buffer_mutex);
timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
INIT_WORK(&priv->work, timeout_work);
@@ -79,7 +81,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
}
ssize_t tpm_common_write(struct file *file, const char __user *buf,
- size_t size, loff_t *off, struct tpm_space *space)
+ size_t size, loff_t *off)
{
struct file_priv *priv = file->private_data;
size_t in_size = size;
@@ -119,7 +121,7 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
mutex_unlock(&priv->buffer_mutex);
return -EPIPE;
}
- out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
+ out_size = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
sizeof(priv->data_buffer), 0);
tpm_put_ops(priv->chip);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index ebd74ab5abef..98b9630c3a36 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -39,7 +39,7 @@ static int tpm_open(struct inode *inode, struct file *file)
if (priv == NULL)
goto out;
- tpm_common_open(file, chip, priv);
+ tpm_common_open(file, chip, priv, NULL);
return 0;
@@ -48,12 +48,6 @@ static int tpm_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
-static ssize_t tpm_write(struct file *file, const char __user *buf,
- size_t size, loff_t *off)
-{
- return tpm_common_write(file, buf, size, off, NULL);
-}
-
/*
* Called on file close
*/
@@ -73,6 +67,6 @@ const struct file_operations tpm_fops = {
.llseek = no_llseek,
.open = tpm_open,
.read = tpm_common_read,
- .write = tpm_write,
+ .write = tpm_common_write,
.release = tpm_release,
};
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
index b24cfb4d3ee1..4048677bbd78 100644
--- a/drivers/char/tpm/tpm-dev.h
+++ b/drivers/char/tpm/tpm-dev.h
@@ -6,6 +6,7 @@
struct file_priv {
struct tpm_chip *chip;
+ struct tpm_space *space;
/* Data passed to and from the tpm via the read/write calls */
size_t data_pending;
@@ -18,11 +19,11 @@ struct file_priv {
};
void tpm_common_open(struct file *file, struct tpm_chip *chip,
- struct file_priv *priv);
+ struct file_priv *priv, struct tpm_space *space);
ssize_t tpm_common_read(struct file *file, char __user *buf,
size_t size, loff_t *off);
ssize_t tpm_common_write(struct file *file, const char __user *buf,
- size_t size, loff_t *off, struct tpm_space *space);
+ size_t size, loff_t *off);
void tpm_common_release(struct file *file, struct file_priv *priv);
#endif
diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
index 1a0e97a5da5a..96006c6b9696 100644
--- a/drivers/char/tpm/tpmrm-dev.c
+++ b/drivers/char/tpm/tpmrm-dev.c
@@ -28,7 +28,7 @@ static int tpmrm_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
- tpm_common_open(file, chip, &priv->priv);
+ tpm_common_open(file, chip, &priv->priv, &priv->space);
return 0;
}
@@ -45,21 +45,11 @@ static int tpmrm_release(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t tpmrm_write(struct file *file, const char __user *buf,
- size_t size, loff_t *off)
-{
- struct file_priv *fpriv = file->private_data;
- struct tpmrm_priv *priv = container_of(fpriv, struct tpmrm_priv, priv);
-
- return tpm_common_write(file, buf, size, off, &priv->space);
-}
-
const struct file_operations tpmrm_fops = {
.owner = THIS_MODULE,
.llseek = no_llseek,
.open = tpmrm_open,
.read = tpm_common_read,
- .write = tpmrm_write,
+ .write = tpm_common_write,
.release = tpmrm_release,
};
-
Currently the TPM driver only supports blocking calls, which doesn't allow
asynchronous IO operations to the TPM hardware.
This patch changes it and adds support for nonblocking write and a new poll
function to enable applications, which want to take advantage of this.
Tested-by: Philip Tricca <[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
drivers/char/tpm/tpm-dev-common.c | 142 ++++++++++++++++++++++++++++---------
drivers/char/tpm/tpm-dev.c | 1
drivers/char/tpm/tpm-dev.h | 13 ++-
drivers/char/tpm/tpm-interface.c | 24 +++++-
drivers/char/tpm/tpm.h | 2 +
drivers/char/tpm/tpmrm-dev.c | 1
6 files changed, 138 insertions(+), 45 deletions(-)
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index f0c033b69b62..a7ae0072044b 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -17,11 +17,36 @@
* License.
*
*/
+#include <linux/poll.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/workqueue.h>
#include "tpm.h"
#include "tpm-dev.h"
+static struct workqueue_struct *tpm_dev_wq;
+static DEFINE_MUTEX(tpm_dev_wq_lock);
+
+static void tpm_async_work(struct work_struct *work)
+{
+ struct file_priv *priv =
+ container_of(work, struct file_priv, async_work);
+ ssize_t ret;
+
+ mutex_lock(&priv->buffer_mutex);
+ priv->command_enqueued = false;
+ ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
+ sizeof(priv->data_buffer), 0);
+
+ tpm_put_ops(priv->chip);
+ if (ret > 0) {
+ priv->data_pending = ret;
+ mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
+ }
+ mutex_unlock(&priv->buffer_mutex);
+ wake_up_interruptible(&priv->async_wait);
+}
+
static void user_reader_timeout(struct timer_list *t)
{
struct file_priv *priv = from_timer(priv, t, user_read_timer);
@@ -29,17 +54,19 @@ static void user_reader_timeout(struct timer_list *t)
pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
task_tgid_nr(current));
- schedule_work(&priv->work);
+ schedule_work(&priv->timeout_work);
}
-static void timeout_work(struct work_struct *work)
+static void tpm_timeout_work(struct work_struct *work)
{
- struct file_priv *priv = container_of(work, struct file_priv, work);
+ struct file_priv *priv = container_of(work, struct file_priv,
+ timeout_work);
mutex_lock(&priv->buffer_mutex);
priv->data_pending = 0;
memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
mutex_unlock(&priv->buffer_mutex);
+ wake_up_interruptible(&priv->async_wait);
}
void tpm_common_open(struct file *file, struct tpm_chip *chip,
@@ -50,8 +77,9 @@ void tpm_common_open(struct file *file, struct tpm_chip *chip,
mutex_init(&priv->buffer_mutex);
timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
- INIT_WORK(&priv->work, timeout_work);
-
+ INIT_WORK(&priv->timeout_work, tpm_timeout_work);
+ INIT_WORK(&priv->async_work, tpm_async_work);
+ init_waitqueue_head(&priv->async_wait);
file->private_data = priv;
}
@@ -63,15 +91,17 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
int rc;
del_singleshot_timer_sync(&priv->user_read_timer);
- flush_work(&priv->work);
+ flush_work(&priv->timeout_work);
mutex_lock(&priv->buffer_mutex);
if (priv->data_pending) {
ret_size = min_t(ssize_t, size, priv->data_pending);
- rc = copy_to_user(buf, priv->data_buffer, ret_size);
- memset(priv->data_buffer, 0, priv->data_pending);
- if (rc)
- ret_size = -EFAULT;
+ if (ret_size > 0) {
+ rc = copy_to_user(buf, priv->data_buffer, ret_size);
+ memset(priv->data_buffer, 0, priv->data_pending);
+ if (rc)
+ ret_size = -EFAULT;
+ }
priv->data_pending = 0;
}
@@ -84,10 +114,9 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
size_t size, loff_t *off)
{
struct file_priv *priv = file->private_data;
- size_t in_size = size;
- ssize_t out_size;
+ int ret = 0;
- if (in_size > TPM_BUFSIZE)
+ if (size > TPM_BUFSIZE)
return -E2BIG;
mutex_lock(&priv->buffer_mutex);
@@ -96,21 +125,20 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
* tpm_read or a user_read_timer timeout. This also prevents split
* buffered writes from blocking here.
*/
- if (priv->data_pending != 0) {
- mutex_unlock(&priv->buffer_mutex);
- return -EBUSY;
+ if (priv->data_pending != 0 || priv->command_enqueued) {
+ ret = -EBUSY;
+ goto out;
}
- if (copy_from_user
- (priv->data_buffer, (void __user *) buf, in_size)) {
- mutex_unlock(&priv->buffer_mutex);
- return -EFAULT;
+ if (copy_from_user(priv->data_buffer, buf, size)) {
+ ret = -EFAULT;
+ goto out;
}
- if (in_size < 6 ||
- in_size < be32_to_cpu(*((__be32 *) (priv->data_buffer + 2)))) {
- mutex_unlock(&priv->buffer_mutex);
- return -EINVAL;
+ if (size < 6 ||
+ size < be32_to_cpu(*((__be32 *)(priv->data_buffer + 2)))) {
+ ret = -EINVAL;
+ goto out;
}
/* atomic tpm command send and result receive. We only hold the ops
@@ -118,25 +146,50 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
* the char dev is held open.
*/
if (tpm_try_get_ops(priv->chip)) {
- mutex_unlock(&priv->buffer_mutex);
- return -EPIPE;
+ ret = -EPIPE;
+ goto out;
}
- out_size = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
- sizeof(priv->data_buffer), 0);
- tpm_put_ops(priv->chip);
- if (out_size < 0) {
+ /*
+ * If in nonblocking mode schedule an async job to send
+ * the command return the size.
+ * In case of error the err code will be returned in
+ * the subsequent read call.
+ */
+ if (file->f_flags & O_NONBLOCK) {
+ priv->command_enqueued = true;
+ queue_work(tpm_dev_wq, &priv->async_work);
mutex_unlock(&priv->buffer_mutex);
- return out_size;
+ return size;
}
- priv->data_pending = out_size;
+ ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
+ sizeof(priv->data_buffer), 0);
+ tpm_put_ops(priv->chip);
+
+ if (ret > 0) {
+ priv->data_pending = ret;
+ mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
+ ret = size;
+ }
+out:
mutex_unlock(&priv->buffer_mutex);
+ return ret;
+}
- /* Set a timeout by which the reader must come claim the result */
- mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
+__poll_t tpm_common_poll(struct file *file, poll_table *wait)
+{
+ struct file_priv *priv = file->private_data;
+ __poll_t mask = 0;
+
+ poll_wait(file, &priv->async_wait, wait);
- return in_size;
+ if (priv->data_pending)
+ mask = EPOLLIN | EPOLLRDNORM;
+ else
+ mask = EPOLLOUT | EPOLLWRNORM;
+
+ return mask;
}
/*
@@ -144,8 +197,25 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
*/
void tpm_common_release(struct file *file, struct file_priv *priv)
{
+ flush_work(&priv->async_work);
del_singleshot_timer_sync(&priv->user_read_timer);
- flush_work(&priv->work);
+ flush_work(&priv->timeout_work);
file->private_data = NULL;
priv->data_pending = 0;
}
+
+
+int __init tpm_dev_common_init(void)
+{
+ tpm_dev_wq = alloc_workqueue("tpm_dev_wq", WQ_MEM_RECLAIM, 0);
+
+ return !tpm_dev_wq ? -ENOMEM : 0;
+}
+
+void __exit tpm_dev_common_exit(void)
+{
+ if (tpm_dev_wq) {
+ destroy_workqueue(tpm_dev_wq);
+ tpm_dev_wq = NULL;
+ }
+}
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 98b9630c3a36..32f9738f1cb2 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -68,5 +68,6 @@ const struct file_operations tpm_fops = {
.open = tpm_open,
.read = tpm_common_read,
.write = tpm_common_write,
+ .poll = tpm_common_poll,
.release = tpm_release,
};
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
index 4048677bbd78..a126b575cb8c 100644
--- a/drivers/char/tpm/tpm-dev.h
+++ b/drivers/char/tpm/tpm-dev.h
@@ -2,18 +2,22 @@
#ifndef _TPM_DEV_H
#define _TPM_DEV_H
+#include <linux/poll.h>
#include "tpm.h"
struct file_priv {
struct tpm_chip *chip;
struct tpm_space *space;
- /* Data passed to and from the tpm via the read/write calls */
- size_t data_pending;
+ /* Holds the amount of data passed or an error code from async op */
+ ssize_t data_pending;
struct mutex buffer_mutex;
struct timer_list user_read_timer; /* user needs to claim result */
- struct work_struct work;
+ struct work_struct timeout_work;
+ struct work_struct async_work;
+ wait_queue_head_t async_wait;
+ bool command_enqueued;
u8 data_buffer[TPM_BUFSIZE];
};
@@ -24,6 +28,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
size_t size, loff_t *off);
ssize_t tpm_common_write(struct file *file, const char __user *buf,
size_t size, loff_t *off);
-void tpm_common_release(struct file *file, struct file_priv *priv);
+__poll_t tpm_common_poll(struct file *file, poll_table *wait);
+void tpm_common_release(struct file *file, struct file_priv *priv);
#endif
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1a803b0cf980..9e280e9b4c5b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1407,19 +1407,32 @@ static int __init tpm_init(void)
tpmrm_class = class_create(THIS_MODULE, "tpmrm");
if (IS_ERR(tpmrm_class)) {
pr_err("couldn't create tpmrm class\n");
- class_destroy(tpm_class);
- return PTR_ERR(tpmrm_class);
+ rc = PTR_ERR(tpmrm_class);
+ goto destroy_tpm_class;
}
rc = alloc_chrdev_region(&tpm_devt, 0, 2*TPM_NUM_DEVICES, "tpm");
if (rc < 0) {
pr_err("tpm: failed to allocate char dev region\n");
- class_destroy(tpmrm_class);
- class_destroy(tpm_class);
- return rc;
+ goto destroy_tpmrm_class;
+ }
+
+ rc = tpm_dev_common_init();
+ if (rc) {
+ pr_err("tpm: failed to allocate char dev region\n");
+ goto unreg_chrdev;
}
return 0;
+
+unreg_chrdev:
+ unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
+destroy_tpmrm_class:
+ class_destroy(tpmrm_class);
+destroy_tpm_class:
+ class_destroy(tpm_class);
+
+ return rc;
}
static void __exit tpm_exit(void)
@@ -1428,6 +1441,7 @@ static void __exit tpm_exit(void)
class_destroy(tpm_class);
class_destroy(tpmrm_class);
unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
+ tpm_dev_common_exit();
}
subsys_initcall(tpm_init);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f3501d05264f..f20dc8ece348 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -604,4 +604,6 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
int tpm_bios_log_setup(struct tpm_chip *chip);
void tpm_bios_log_teardown(struct tpm_chip *chip);
+int tpm_dev_common_init(void);
+void tpm_dev_common_exit(void);
#endif
diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
index 96006c6b9696..0c751a79bbed 100644
--- a/drivers/char/tpm/tpmrm-dev.c
+++ b/drivers/char/tpm/tpmrm-dev.c
@@ -51,5 +51,6 @@ const struct file_operations tpmrm_fops = {
.open = tpmrm_open,
.read = tpm_common_read,
.write = tpm_common_write,
+ .poll = tpm_common_poll,
.release = tpmrm_release,
};
On Mon, Aug 13, 2018 at 01:32:53PM -0700, Tadeusz Struk wrote:
> Add a ptr to struct tpm_space to the file_priv to have an easy
> access to it in the async job without the need to allocate memory.
> This also allows to consolidate of the write operations for
> the two interfaces.
>
> Tested-by: Philip Tricca <[email protected]>
> Signed-off-by: Tadeusz Struk <[email protected]>
NAK
A rule thumb: do not add "links" between commits unless it is you cannot
find any other way. The cover letter is made for describing feature or
features that are defined a the set of commits.
What is "async job" anyway? It is a bogus term.
Please refer to only merged code in your commit message, not futures.
/Jarkko
On Tue, Aug 14, 2018 at 07:06:05PM +0300, Jarkko Sakkinen wrote:
> On Mon, Aug 13, 2018 at 01:32:53PM -0700, Tadeusz Struk wrote:
> > Add a ptr to struct tpm_space to the file_priv to have an easy
> > access to it in the async job without the need to allocate memory.
> > This also allows to consolidate of the write operations for
> > the two interfaces.
> >
> > Tested-by: Philip Tricca <[email protected]>
> > Signed-off-by: Tadeusz Struk <[email protected]>
>
> NAK
>
> A rule thumb: do not add "links" between commits unless it is you cannot
> find any other way. The cover letter is made for describing feature or
> features that are defined a the set of commits.
~~~~~
by a
/Jarkko
Are you planning to send v6 soon fixing the minor things in 1/2 (typo
+ change for the commit message)?
/Jarkko
On Mon, Aug 13, 2018 at 01:32:48PM -0700, Tadeusz Struk wrote:
> The TCG SAPI specification [1] defines a set of functions, which allow
> applications to use the TPM device in either blocking or non-blocking fashion.
> Each command defined by the specification has a corresponding
> Tss2_Sys_<COMMAND>_Prepare() and Tss2_Sys_<COMMAND>_Complete() call, which
> together with Tss2_Sys_ExecuteAsync() is designed to allow asynchronous
> mode of operation. Currently the TPM driver supports only blocking calls,
> which doesn't allow asynchronous IO operations.
> This patch changes it and adds support for nonblocking write and a new poll
> function to enable applications, which want to take advantage of this feature.
> The new functionality can be tested using standard TPM tools implemented
> in [2], together with modified TCTI from [3], and an example application
> by Philip Tricca [4]. Here is a short description from Philip:
>
> "The example application `glib-tss2-event` uses a glib main event loop
> to create an RSA 2048 primary key in the TPM2 NULL hierarchy while
> using a glib timer event to time the operation. A GSource object is
> used to generate an event when the FD underlying the tss2 function
> call has data ready. While the application waits for an event indicating
> that the CreatePrimary operation is complete, it counts timer events
> that occur every 100ms. Once the CreatePrimary operation completes the
> number of timer events that occurred is used to make a rough calculation
> of the elapsed time. This value is then printed to the console.
> This takes ~300 lines of C code and requires no management or
> synchronization of threads. The glib GMainContext is "just a poll()
> loop" according to the glib documentation here:
>
> https://developer.gnome.org/programming-guidelines/stable/main-contexts.html.en
>
> and so supporting 'poll' is the easiest way to integrate with glib /
> gtk+. This is true of any other event system that relies on 'poll'
> instead of worker threads."
>
> [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_SAPI_Version-1.1_Revision-22_review_030918.pdf
> [2] https://github.com/tpm2-software/tpm2-tools
> [3] https://github.com/tstruk/tpm2-tss/tree/async
> [4] https://github.com/flihp/glib-tss2-async-example
>
> ---
> Changes in v5:
> - Changed the workqueue allocation time back from the first user interface
> open to module init.
>
> Changes in v4:
> - Changed the way buffer_mutex is handled in nonblocking mode so that
> it is not held when write() returns to user space.
>
> Changes in v3:
> - Fixed problem reported by 0-dey kbuild test robot around __exitcall.
> It complained because there is a module_exit() in another file already.
> - Added info on example application from Philip
>
> Changes in v2:
> - Split the change into two separate patches. First patch adds a pointer
> to the space to the struct file_priv to have access to it from the async job.
> This is to avoid memory allocations on every write call. Now everything
> what's needed is in the file_priv struct.
> - Renamed the 'work' member of the timer to avoid confusion.
> Now there are 'timeout_work' and 'async_work'.
> - Removed the global wait queue and moved it to file_priv.
> - Only creating the work queue when the first file is opened.
>
> Tadeusz Struk (2):
> tpm: add ptr to the tpm_space struct to file_priv
> tpm: add support for nonblocking operation
>
> drivers/char/tpm/tpm-dev-common.c | 150 +++++++++++++++++++++++++++----------
> drivers/char/tpm/tpm-dev.c | 22 +++--
> drivers/char/tpm/tpm-dev.h | 19 +++--
> drivers/char/tpm/tpm-interface.c | 1
> drivers/char/tpm/tpm.h | 1
> drivers/char/tpm/tpmrm-dev.c | 31 ++++----
> 6 files changed, 152 insertions(+), 72 deletions(-)
>
> --
> TS
Just the change to the commit message. Mislooked patchwork, the typo was
in my response :-) I'll do recheck for 2/2. Check those comments before
v6 if there is anything else.
/Jarkko
On Fri, Aug 31, 2018 at 11:57:11AM +0300, Jarkko Sakkinen wrote:
> Are you planning to send v6 soon fixing the minor things in 1/2 (typo
> + change for the commit message)?
>
> /Jarkko
>
> On Mon, Aug 13, 2018 at 01:32:48PM -0700, Tadeusz Struk wrote:
> > The TCG SAPI specification [1] defines a set of functions, which allow
> > applications to use the TPM device in either blocking or non-blocking fashion.
> > Each command defined by the specification has a corresponding
> > Tss2_Sys_<COMMAND>_Prepare() and Tss2_Sys_<COMMAND>_Complete() call, which
> > together with Tss2_Sys_ExecuteAsync() is designed to allow asynchronous
> > mode of operation. Currently the TPM driver supports only blocking calls,
> > which doesn't allow asynchronous IO operations.
> > This patch changes it and adds support for nonblocking write and a new poll
> > function to enable applications, which want to take advantage of this feature.
> > The new functionality can be tested using standard TPM tools implemented
> > in [2], together with modified TCTI from [3], and an example application
> > by Philip Tricca [4]. Here is a short description from Philip:
> >
> > "The example application `glib-tss2-event` uses a glib main event loop
> > to create an RSA 2048 primary key in the TPM2 NULL hierarchy while
> > using a glib timer event to time the operation. A GSource object is
> > used to generate an event when the FD underlying the tss2 function
> > call has data ready. While the application waits for an event indicating
> > that the CreatePrimary operation is complete, it counts timer events
> > that occur every 100ms. Once the CreatePrimary operation completes the
> > number of timer events that occurred is used to make a rough calculation
> > of the elapsed time. This value is then printed to the console.
> > This takes ~300 lines of C code and requires no management or
> > synchronization of threads. The glib GMainContext is "just a poll()
> > loop" according to the glib documentation here:
> >
> > https://developer.gnome.org/programming-guidelines/stable/main-contexts.html.en
> >
> > and so supporting 'poll' is the easiest way to integrate with glib /
> > gtk+. This is true of any other event system that relies on 'poll'
> > instead of worker threads."
> >
> > [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_SAPI_Version-1.1_Revision-22_review_030918.pdf
> > [2] https://github.com/tpm2-software/tpm2-tools
> > [3] https://github.com/tstruk/tpm2-tss/tree/async
> > [4] https://github.com/flihp/glib-tss2-async-example
> >
> > ---
> > Changes in v5:
> > - Changed the workqueue allocation time back from the first user interface
> > open to module init.
> >
> > Changes in v4:
> > - Changed the way buffer_mutex is handled in nonblocking mode so that
> > it is not held when write() returns to user space.
> >
> > Changes in v3:
> > - Fixed problem reported by 0-dey kbuild test robot around __exitcall.
> > It complained because there is a module_exit() in another file already.
> > - Added info on example application from Philip
> >
> > Changes in v2:
> > - Split the change into two separate patches. First patch adds a pointer
> > to the space to the struct file_priv to have access to it from the async job.
> > This is to avoid memory allocations on every write call. Now everything
> > what's needed is in the file_priv struct.
> > - Renamed the 'work' member of the timer to avoid confusion.
> > Now there are 'timeout_work' and 'async_work'.
> > - Removed the global wait queue and moved it to file_priv.
> > - Only creating the work queue when the first file is opened.
> >
> > Tadeusz Struk (2):
> > tpm: add ptr to the tpm_space struct to file_priv
> > tpm: add support for nonblocking operation
> >
> > drivers/char/tpm/tpm-dev-common.c | 150 +++++++++++++++++++++++++++----------
> > drivers/char/tpm/tpm-dev.c | 22 +++--
> > drivers/char/tpm/tpm-dev.h | 19 +++--
> > drivers/char/tpm/tpm-interface.c | 1
> > drivers/char/tpm/tpm.h | 1
> > drivers/char/tpm/tpmrm-dev.c | 31 ++++----
> > 6 files changed, 152 insertions(+), 72 deletions(-)
> >
> > --
> > TS
Mon, Aug 13, 2018 at 01:32:58PM -0700, Tadeusz Struk wrote:
> Currently the TPM driver only supports blocking calls, which doesn't allow
> asynchronous IO operations to the TPM hardware.
> This patch changes it and adds support for nonblocking write and a new poll
> function to enable applications, which want to take advantage of this.
>
> Tested-by: Philip Tricca <[email protected]>
> Signed-off-by: Tadeusz Struk <[email protected]>
> ---
> drivers/char/tpm/tpm-dev-common.c | 142 ++++++++++++++++++++++++++++---------
> drivers/char/tpm/tpm-dev.c | 1
> drivers/char/tpm/tpm-dev.h | 13 ++-
> drivers/char/tpm/tpm-interface.c | 24 +++++-
> drivers/char/tpm/tpm.h | 2 +
> drivers/char/tpm/tpmrm-dev.c | 1
> 6 files changed, 138 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index f0c033b69b62..a7ae0072044b 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -17,11 +17,36 @@
> * License.
> *
> */
> +#include <linux/poll.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> +#include <linux/workqueue.h>
> #include "tpm.h"
> #include "tpm-dev.h"
>
> +static struct workqueue_struct *tpm_dev_wq;
> +static DEFINE_MUTEX(tpm_dev_wq_lock);
> +
> +static void tpm_async_work(struct work_struct *work)
> +{
> + struct file_priv *priv =
> + container_of(work, struct file_priv, async_work);
> + ssize_t ret;
> +
> + mutex_lock(&priv->buffer_mutex);
> + priv->command_enqueued = false;
> + ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
> + sizeof(priv->data_buffer), 0);
> +
> + tpm_put_ops(priv->chip);
> + if (ret > 0) {
> + priv->data_pending = ret;
> + mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
> + }
> + mutex_unlock(&priv->buffer_mutex);
> + wake_up_interruptible(&priv->async_wait);
> +}
> +
> static void user_reader_timeout(struct timer_list *t)
> {
> struct file_priv *priv = from_timer(priv, t, user_read_timer);
> @@ -29,17 +54,19 @@ static void user_reader_timeout(struct timer_list *t)
> pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
> task_tgid_nr(current));
>
> - schedule_work(&priv->work);
> + schedule_work(&priv->timeout_work);
> }
>
> -static void timeout_work(struct work_struct *work)
> +static void tpm_timeout_work(struct work_struct *work)
> {
> - struct file_priv *priv = container_of(work, struct file_priv, work);
> + struct file_priv *priv = container_of(work, struct file_priv,
> + timeout_work);
>
> mutex_lock(&priv->buffer_mutex);
> priv->data_pending = 0;
> memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
> mutex_unlock(&priv->buffer_mutex);
> + wake_up_interruptible(&priv->async_wait);
> }
>
> void tpm_common_open(struct file *file, struct tpm_chip *chip,
> @@ -50,8 +77,9 @@ void tpm_common_open(struct file *file, struct tpm_chip *chip,
>
> mutex_init(&priv->buffer_mutex);
> timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
> - INIT_WORK(&priv->work, timeout_work);
> -
> + INIT_WORK(&priv->timeout_work, tpm_timeout_work);
> + INIT_WORK(&priv->async_work, tpm_async_work);
> + init_waitqueue_head(&priv->async_wait);
> file->private_data = priv;
> }
>
> @@ -63,15 +91,17 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
> int rc;
>
> del_singleshot_timer_sync(&priv->user_read_timer);
> - flush_work(&priv->work);
> + flush_work(&priv->timeout_work);
> mutex_lock(&priv->buffer_mutex);
>
> if (priv->data_pending) {
> ret_size = min_t(ssize_t, size, priv->data_pending);
> - rc = copy_to_user(buf, priv->data_buffer, ret_size);
> - memset(priv->data_buffer, 0, priv->data_pending);
> - if (rc)
> - ret_size = -EFAULT;
> + if (ret_size > 0) {
> + rc = copy_to_user(buf, priv->data_buffer, ret_size);
> + memset(priv->data_buffer, 0, priv->data_pending);
> + if (rc)
> + ret_size = -EFAULT;
> + }
>
> priv->data_pending = 0;
> }
> @@ -84,10 +114,9 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> size_t size, loff_t *off)
> {
> struct file_priv *priv = file->private_data;
> - size_t in_size = size;
> - ssize_t out_size;
> + int ret = 0;
>
> - if (in_size > TPM_BUFSIZE)
> + if (size > TPM_BUFSIZE)
> return -E2BIG;
>
> mutex_lock(&priv->buffer_mutex);
> @@ -96,21 +125,20 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> * tpm_read or a user_read_timer timeout. This also prevents split
> * buffered writes from blocking here.
> */
> - if (priv->data_pending != 0) {
> - mutex_unlock(&priv->buffer_mutex);
> - return -EBUSY;
> + if (priv->data_pending != 0 || priv->command_enqueued) {
> + ret = -EBUSY;
> + goto out;
> }
>
> - if (copy_from_user
> - (priv->data_buffer, (void __user *) buf, in_size)) {
> - mutex_unlock(&priv->buffer_mutex);
> - return -EFAULT;
> + if (copy_from_user(priv->data_buffer, buf, size)) {
> + ret = -EFAULT;
> + goto out;
> }
>
> - if (in_size < 6 ||
> - in_size < be32_to_cpu(*((__be32 *) (priv->data_buffer + 2)))) {
> - mutex_unlock(&priv->buffer_mutex);
> - return -EINVAL;
> + if (size < 6 ||
> + size < be32_to_cpu(*((__be32 *)(priv->data_buffer + 2)))) {
> + ret = -EINVAL;
> + goto out;
> }
>
> /* atomic tpm command send and result receive. We only hold the ops
> @@ -118,25 +146,50 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> * the char dev is held open.
> */
> if (tpm_try_get_ops(priv->chip)) {
> - mutex_unlock(&priv->buffer_mutex);
> - return -EPIPE;
> + ret = -EPIPE;
> + goto out;
> }
> - out_size = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
> - sizeof(priv->data_buffer), 0);
>
> - tpm_put_ops(priv->chip);
> - if (out_size < 0) {
> + /*
> + * If in nonblocking mode schedule an async job to send
> + * the command return the size.
> + * In case of error the err code will be returned in
> + * the subsequent read call.
> + */
> + if (file->f_flags & O_NONBLOCK) {
> + priv->command_enqueued = true;
> + queue_work(tpm_dev_wq, &priv->async_work);
> mutex_unlock(&priv->buffer_mutex);
> - return out_size;
> + return size;
> }
>
> - priv->data_pending = out_size;
> + ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
> + sizeof(priv->data_buffer), 0);
> + tpm_put_ops(priv->chip);
> +
> + if (ret > 0) {
> + priv->data_pending = ret;
> + mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
> + ret = size;
> + }
> +out:
> mutex_unlock(&priv->buffer_mutex);
> + return ret;
> +}
>
> - /* Set a timeout by which the reader must come claim the result */
> - mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
> +__poll_t tpm_common_poll(struct file *file, poll_table *wait)
> +{
> + struct file_priv *priv = file->private_data;
> + __poll_t mask = 0;
> +
> + poll_wait(file, &priv->async_wait, wait);
>
> - return in_size;
> + if (priv->data_pending)
> + mask = EPOLLIN | EPOLLRDNORM;
> + else
> + mask = EPOLLOUT | EPOLLWRNORM;
> +
> + return mask;
> }
>
> /*
> @@ -144,8 +197,25 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> */
> void tpm_common_release(struct file *file, struct file_priv *priv)
> {
> + flush_work(&priv->async_work);
> del_singleshot_timer_sync(&priv->user_read_timer);
> - flush_work(&priv->work);
> + flush_work(&priv->timeout_work);
> file->private_data = NULL;
> priv->data_pending = 0;
> }
> +
> +
> +int __init tpm_dev_common_init(void)
> +{
> + tpm_dev_wq = alloc_workqueue("tpm_dev_wq", WQ_MEM_RECLAIM, 0);
> +
> + return !tpm_dev_wq ? -ENOMEM : 0;
> +}
> +
> +void __exit tpm_dev_common_exit(void)
> +{
> + if (tpm_dev_wq) {
> + destroy_workqueue(tpm_dev_wq);
> + tpm_dev_wq = NULL;
> + }
> +}
> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> index 98b9630c3a36..32f9738f1cb2 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -68,5 +68,6 @@ const struct file_operations tpm_fops = {
> .open = tpm_open,
> .read = tpm_common_read,
> .write = tpm_common_write,
> + .poll = tpm_common_poll,
> .release = tpm_release,
> };
> diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
> index 4048677bbd78..a126b575cb8c 100644
> --- a/drivers/char/tpm/tpm-dev.h
> +++ b/drivers/char/tpm/tpm-dev.h
> @@ -2,18 +2,22 @@
> #ifndef _TPM_DEV_H
> #define _TPM_DEV_H
>
> +#include <linux/poll.h>
> #include "tpm.h"
>
> struct file_priv {
> struct tpm_chip *chip;
> struct tpm_space *space;
>
> - /* Data passed to and from the tpm via the read/write calls */
> - size_t data_pending;
> + /* Holds the amount of data passed or an error code from async op */
> + ssize_t data_pending;
> struct mutex buffer_mutex;
>
> struct timer_list user_read_timer; /* user needs to claim result */
> - struct work_struct work;
> + struct work_struct timeout_work;
> + struct work_struct async_work;
> + wait_queue_head_t async_wait;
> + bool command_enqueued;
>
> u8 data_buffer[TPM_BUFSIZE];
> };
> @@ -24,6 +28,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
> size_t size, loff_t *off);
> ssize_t tpm_common_write(struct file *file, const char __user *buf,
> size_t size, loff_t *off);
> -void tpm_common_release(struct file *file, struct file_priv *priv);
> +__poll_t tpm_common_poll(struct file *file, poll_table *wait);
>
> +void tpm_common_release(struct file *file, struct file_priv *priv);
> #endif
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1a803b0cf980..9e280e9b4c5b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1407,19 +1407,32 @@ static int __init tpm_init(void)
> tpmrm_class = class_create(THIS_MODULE, "tpmrm");
> if (IS_ERR(tpmrm_class)) {
> pr_err("couldn't create tpmrm class\n");
> - class_destroy(tpm_class);
> - return PTR_ERR(tpmrm_class);
> + rc = PTR_ERR(tpmrm_class);
> + goto destroy_tpm_class;
> }
>
> rc = alloc_chrdev_region(&tpm_devt, 0, 2*TPM_NUM_DEVICES, "tpm");
> if (rc < 0) {
> pr_err("tpm: failed to allocate char dev region\n");
> - class_destroy(tpmrm_class);
> - class_destroy(tpm_class);
> - return rc;
> + goto destroy_tpmrm_class;
> + }
> +
> + rc = tpm_dev_common_init();
> + if (rc) {
> + pr_err("tpm: failed to allocate char dev region\n");
> + goto unreg_chrdev;
> }
>
> return 0;
> +
> +unreg_chrdev:
> + unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
> +destroy_tpmrm_class:
> + class_destroy(tpmrm_class);
> +destroy_tpm_class:
> + class_destroy(tpm_class);
out_*
> +
> + return rc;
> }
>
> static void __exit tpm_exit(void)
> @@ -1428,6 +1441,7 @@ static void __exit tpm_exit(void)
> class_destroy(tpm_class);
> class_destroy(tpmrm_class);
> unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
> + tpm_dev_common_exit();
> }
>
> subsys_initcall(tpm_init);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f3501d05264f..f20dc8ece348 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -604,4 +604,6 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>
> int tpm_bios_log_setup(struct tpm_chip *chip);
> void tpm_bios_log_teardown(struct tpm_chip *chip);
> +int tpm_dev_common_init(void);
> +void tpm_dev_common_exit(void);
> #endif
> diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
> index 96006c6b9696..0c751a79bbed 100644
> --- a/drivers/char/tpm/tpmrm-dev.c
> +++ b/drivers/char/tpm/tpmrm-dev.c
> @@ -51,5 +51,6 @@ const struct file_operations tpmrm_fops = {
> .open = tpmrm_open,
> .read = tpm_common_read,
> .write = tpm_common_write,
> + .poll = tpm_common_poll,
> .release = tpmrm_release,
> };
>
Otherwise, I did not spot anything obviously wrong. I guess this patch
set is about ready to be taken soon...
/Jarkko
On 08/31/2018 01:58 AM, Jarkko Sakkinen wrote:
> Just the change to the commit message. Mislooked patchwork, the typo was
> in my response :-) I'll do recheck for 2/2. Check those comments before
> v6 if there is anything else.
Hi,
I have done the changes you requested and ran the "checkpatch.pl --strict"
on it and it says that "no obvious style problems and is ready for submission".
Thanks,
--
Tadeusz