* [PATCH v5 0/4] Introduce TEE bus driver framework
@ 2019-01-24 5:54 Sumit Garg
2019-01-24 5:54 ` [PATCH v5 1/4] tee: add bus driver framework for TEE based devices Sumit Garg
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 5:54 UTC (permalink / raw)
To: jens.wiklander, herbert, ard.biesheuvel
Cc: linux-arm-kernel, linux-crypto, linux-kernel, yamada.masahiro,
michal.lkml, mpm, robh+dt, mark.rutland, arnd, gregkh,
daniel.thompson, bhsharma, tee-dev, Sumit Garg
This series introduces a generic TEE bus driver concept for TEE based
kernel drivers which would like to communicate with TEE based devices/
services.
Patch #1 adds TEE bus concept where devices/services are identified via
Universally Unique Identifier (UUID) and drivers register a table of
device UUIDs which they can support. This concept also allows for device
enumeration to be specific to corresponding TEE implementation like
OP-TEE etc.
Patch #2 adds supp_nowait flag for non-blocking requests arising via
TEE internal client interface.
Patch #3 adds TEE bus device enumeration support for OP-TEE. OP-TEE
provides a pseudo TA to enumerate TAs which can act as devices/services
for TEE bus.
Patch #4 adds OP-TEE based hwrng driver which act as TEE bus driver.
On ARM SoC's with TrustZone enabled, peripherals like entropy sources
might not be accessible to normal world (linux in this case) and rather
accessible to secure world (OP-TEE in this case) only. So this driver
aims to provides a generic interface to OP-TEE based random number
generator service.
Example case is Developerbox based on Socionext's Synquacer SoC [1]
which provides 7 thermal sensors accessible from secure world only which
could be used as entropy sources (thermal/measurement noise).
[1] https://www.96boards.org/product/developerbox/
Changes in v5:
1. Add support in module device table for TEE bus devices.
2. Correct license for optee-rng module.
Changes in v4:
1. Use typedef instead of single member tee_client_device_id struct.
2. Incorporate TEE bus nitpicks.
Changes in v3:
1. Fixed bus error path in Patch #1.
2. Reversed order of Patch #2 and #3.
3. Fixed miscellaneous syntax comments and memory leak.
4. Added comments in Patch #2 for supp_nowait flag.
Changes in v2:
Based on review comments, the scope of this series has increased as
follows:
1. Added TEE bus driver framework.
2. Added OP-TEE based device enumeration.
3. Register optee-rng driver as TEE bus driver.
4. Removed DT dependency for optee-rng device UUID.
5. Added supp_nowait flag.
Sumit Garg (4):
tee: add bus driver framework for TEE based devices
tee: add supp_nowait flag in tee_context struct
tee: optee: add TEE bus device enumeration support
hwrng: add OP-TEE based rng driver
MAINTAINERS | 5 +
drivers/char/hw_random/Kconfig | 15 ++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/optee-rng.c | 274 +++++++++++++++++++++++++++++++++++++
drivers/tee/optee/Makefile | 1 +
drivers/tee/optee/core.c | 4 +
drivers/tee/optee/device.c | 153 +++++++++++++++++++++
drivers/tee/optee/optee_private.h | 3 +
drivers/tee/optee/supp.c | 10 +-
drivers/tee/tee_core.c | 70 +++++++++-
include/linux/mod_devicetable.h | 9 ++
include/linux/tee_drv.h | 38 ++++-
scripts/mod/devicetable-offsets.c | 3 +
scripts/mod/file2alias.c | 19 +++
14 files changed, 599 insertions(+), 6 deletions(-)
create mode 100644 drivers/char/hw_random/optee-rng.c
create mode 100644 drivers/tee/optee/device.c
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 1/4] tee: add bus driver framework for TEE based devices
2019-01-24 5:54 [PATCH v5 0/4] Introduce TEE bus driver framework Sumit Garg
@ 2019-01-24 5:54 ` Sumit Garg
2019-01-24 9:45 ` Daniel Thompson
2019-01-24 5:54 ` [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct Sumit Garg
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 5:54 UTC (permalink / raw)
To: jens.wiklander, herbert, ard.biesheuvel
Cc: linux-arm-kernel, linux-crypto, linux-kernel, yamada.masahiro,
michal.lkml, mpm, robh+dt, mark.rutland, arnd, gregkh,
daniel.thompson, bhsharma, tee-dev, Sumit Garg
Introduce a generic TEE bus driver concept for TEE based kernel drivers
which would like to communicate with TEE based devices/services. Also
add support in module device table for these new TEE based devices.
In this TEE bus concept, devices/services are identified via Universally
Unique Identifier (UUID) and drivers register a table of device UUIDs
which they can support.
So this TEE bus framework registers following apis:
- match(): Iterates over the driver UUID table to find a corresponding
match for device UUID. If a match is found, then this particular device
is probed via corresponding probe api registered by the driver. This
process happens whenever a device or a driver is registered with TEE
bus.
- uevent(): Notifies user-space (udev) whenever a new device is registered
on this bus for auto-loading of out-of-tree drivers.
Also this framework allows for device enumeration to be specific to
corresponding TEE implementation like OP-TEE etc.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
drivers/tee/tee_core.c | 57 ++++++++++++++++++++++++++++++++++++---
include/linux/mod_devicetable.h | 9 +++++++
include/linux/tee_drv.h | 32 +++++++++++++++++++++-
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 19 +++++++++++++
5 files changed, 115 insertions(+), 5 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 7b2bb4c..9f4c8bc 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -15,7 +15,6 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
#include <linux/cdev.h>
-#include <linux/device.h>
#include <linux/fs.h>
#include <linux/idr.h>
#include <linux/module.h>
@@ -1027,6 +1026,42 @@ int tee_client_invoke_func(struct tee_context *ctx,
}
EXPORT_SYMBOL_GPL(tee_client_invoke_func);
+static int tee_client_device_match(struct device *dev,
+ struct device_driver *drv)
+{
+ const struct tee_client_device_id *id_table;
+ struct tee_client_device *tee_device;
+
+ id_table = to_tee_client_driver(drv)->id_table;
+ tee_device = to_tee_client_device(dev);
+
+ while (!uuid_is_null(&id_table->uuid)) {
+ if (uuid_equal(&tee_device->id.uuid, &id_table->uuid))
+ return 1;
+ id_table++;
+ }
+
+ return 0;
+}
+
+static int tee_client_device_uevent(struct device *dev,
+ struct kobj_uevent_env *env)
+{
+ uuid_t *dev_id = &to_tee_client_device(dev)->id.uuid;
+
+ if (add_uevent_var(env, "MODALIAS=tee:%pUb", dev_id))
+ return -ENOMEM;
+
+ return 0;
+}
+
+struct bus_type tee_bus_type = {
+ .name = "tee",
+ .match = tee_client_device_match,
+ .uevent = tee_client_device_uevent,
+};
+EXPORT_SYMBOL_GPL(tee_bus_type);
+
static int __init tee_init(void)
{
int rc;
@@ -1040,18 +1075,32 @@ static int __init tee_init(void)
rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
if (rc) {
pr_err("failed to allocate char dev region\n");
- class_destroy(tee_class);
- tee_class = NULL;
+ goto out_unreg_class;
}
+ rc = bus_register(&tee_bus_type);
+ if (rc) {
+ pr_err("failed to register tee bus\n");
+ goto out_unreg_chrdev;
+ }
+
+ return 0;
+
+out_unreg_chrdev:
+ unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
+out_unreg_class:
+ class_destroy(tee_class);
+ tee_class = NULL;
+
return rc;
}
static void __exit tee_exit(void)
{
+ bus_unregister(&tee_bus_type);
+ unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
class_destroy(tee_class);
tee_class = NULL;
- unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
}
subsys_initcall(tee_init);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index f9bd2f3..14eaeeb 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -779,4 +779,13 @@ struct typec_device_id {
kernel_ulong_t driver_data;
};
+/**
+ * struct tee_client_device_id - tee based device identifier
+ * @uuid: For TEE based client devices we use the device uuid as
+ * the identifier.
+ */
+struct tee_client_device_id {
+ uuid_t uuid;
+};
+
#endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 6cfe058..ce957ce 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -15,11 +15,14 @@
#ifndef __TEE_DRV_H
#define __TEE_DRV_H
-#include <linux/types.h>
+#include <linux/device.h>
#include <linux/idr.h>
#include <linux/kref.h>
#include <linux/list.h>
+#include <linux/mod_devicetable.h>
#include <linux/tee.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
/*
* The file describes the API provided by the generic TEE driver to the
@@ -538,4 +541,31 @@ static inline bool tee_param_is_memref(struct tee_param *param)
}
}
+extern struct bus_type tee_bus_type;
+
+/**
+ * struct tee_client_device - tee based device
+ * @id: device identifier
+ * @dev: device structure
+ */
+struct tee_client_device {
+ struct tee_client_device_id id;
+ struct device dev;
+};
+
+#define to_tee_client_device(d) container_of(d, struct tee_client_device, dev)
+
+/**
+ * struct tee_client_driver - tee client driver
+ * @id_table: device id table supported by this driver
+ * @driver: driver structure
+ */
+struct tee_client_driver {
+ const struct tee_client_device_id *id_table;
+ struct device_driver driver;
+};
+
+#define to_tee_client_driver(d) \
+ container_of(d, struct tee_client_driver, driver)
+
#endif /*__TEE_DRV_H*/
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 2930044..1607183 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -225,5 +225,8 @@ int main(void)
DEVID_FIELD(typec_device_id, svid);
DEVID_FIELD(typec_device_id, mode);
+ DEVID(tee_client_device_id);
+ DEVID_FIELD(tee_client_device_id, uuid);
+
return 0;
}
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index a37af7d..d0e4172 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -37,6 +37,9 @@ typedef unsigned char __u8;
typedef struct {
__u8 b[16];
} uuid_le;
+typedef struct {
+ __u8 b[16];
+} uuid_t;
/* Big exception to the "don't include kernel headers into userspace, which
* even potentially has different endianness and word sizes, since
@@ -1287,6 +1290,21 @@ static int do_typec_entry(const char *filename, void *symval, char *alias)
return 1;
}
+/* Looks like: tee:uuid */
+static int do_tee_entry(const char *filename, void *symval, char *alias)
+{
+ DEF_FIELD(symval, tee_client_device_id, uuid);
+
+ sprintf(alias, "tee:%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+ uuid.b[0], uuid.b[1], uuid.b[2], uuid.b[3], uuid.b[4],
+ uuid.b[5], uuid.b[6], uuid.b[7], uuid.b[8], uuid.b[9],
+ uuid.b[10], uuid.b[11], uuid.b[12], uuid.b[13], uuid.b[14],
+ uuid.b[15]);
+
+ add_wildcard(alias);
+ return 1;
+}
+
/* Does namelen bytes of name exactly match the symbol? */
static bool sym_is(const char *name, unsigned namelen, const char *symbol)
{
@@ -1357,6 +1375,7 @@ static const struct devtable devtable[] = {
{"fslmc", SIZE_fsl_mc_device_id, do_fsl_mc_entry},
{"tbsvc", SIZE_tb_service_id, do_tbsvc_entry},
{"typec", SIZE_typec_device_id, do_typec_entry},
+ {"tee", SIZE_tee_client_device_id, do_tee_entry},
};
/* Create MODULE_ALIAS() statements.
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct
2019-01-24 5:54 [PATCH v5 0/4] Introduce TEE bus driver framework Sumit Garg
2019-01-24 5:54 ` [PATCH v5 1/4] tee: add bus driver framework for TEE based devices Sumit Garg
@ 2019-01-24 5:54 ` Sumit Garg
2019-01-24 9:48 ` Daniel Thompson
2019-01-24 5:54 ` [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support Sumit Garg
2019-01-24 5:54 ` [PATCH v5 4/4] hwrng: add OP-TEE based rng driver Sumit Garg
3 siblings, 1 reply; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 5:54 UTC (permalink / raw)
To: jens.wiklander, herbert, ard.biesheuvel
Cc: linux-arm-kernel, linux-crypto, linux-kernel, yamada.masahiro,
michal.lkml, mpm, robh+dt, mark.rutland, arnd, gregkh,
daniel.thompson, bhsharma, tee-dev, Sumit Garg
This flag indicates that requests in this context should not wait for
tee-supplicant daemon to be started if not present and just return
with an error code. It is needed for requests which should be
non-blocking in nature like ones arising from TEE based kernel drivers
or any in kernel api that uses TEE internal client interface.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
drivers/tee/optee/supp.c | 10 +++++++++-
drivers/tee/tee_core.c | 13 +++++++++++++
include/linux/tee_drv.h | 6 ++++++
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c
index 43626e1..92f56b8 100644
--- a/drivers/tee/optee/supp.c
+++ b/drivers/tee/optee/supp.c
@@ -88,10 +88,18 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
struct optee_supp *supp = &optee->supp;
- struct optee_supp_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
+ struct optee_supp_req *req;
bool interruptable;
u32 ret;
+ /*
+ * Return in case there is no supplicant available and
+ * non-blocking request.
+ */
+ if (!supp->ctx && ctx->supp_nowait)
+ return TEEC_ERROR_COMMUNICATION;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
if (!req)
return TEEC_ERROR_OUT_OF_MEMORY;
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 9f4c8bc..2612dfa 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -105,6 +105,11 @@ static int tee_open(struct inode *inode, struct file *filp)
if (IS_ERR(ctx))
return PTR_ERR(ctx);
+ /*
+ * Default user-space behaviour is to wait for tee-supplicant
+ * if not present for any requests in this context.
+ */
+ ctx->supp_nowait = false;
filp->private_data = ctx;
return 0;
}
@@ -981,6 +986,14 @@ tee_client_open_context(struct tee_context *start,
} while (IS_ERR(ctx) && PTR_ERR(ctx) != -ENOMEM);
put_device(put_dev);
+ /*
+ * Default behaviour for in kernel client is to not wait for
+ * tee-supplicant if not present for any requests in this context.
+ * Also this flag could be configured again before call to
+ * tee_client_open_session() if any in kernel client requires
+ * different behaviour.
+ */
+ ctx->supp_nowait = true;
return ctx;
}
EXPORT_SYMBOL_GPL(tee_client_open_context);
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index ce957ce..56d7f1b 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -50,6 +50,11 @@ struct tee_shm_pool;
* @releasing: flag that indicates if context is being released right now.
* It is needed to break circular dependency on context during
* shared memory release.
+ * @supp_nowait: flag that indicates that requests in this context should not
+ * wait for tee-supplicant daemon to be started if not present
+ * and just return with an error code. It is needed for requests
+ * that arises from TEE based kernel drivers that should be
+ * non-blocking in nature.
*/
struct tee_context {
struct tee_device *teedev;
@@ -57,6 +62,7 @@ struct tee_context {
void *data;
struct kref refcount;
bool releasing;
+ bool supp_nowait;
};
struct tee_param_memref {
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support
2019-01-24 5:54 [PATCH v5 0/4] Introduce TEE bus driver framework Sumit Garg
2019-01-24 5:54 ` [PATCH v5 1/4] tee: add bus driver framework for TEE based devices Sumit Garg
2019-01-24 5:54 ` [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct Sumit Garg
@ 2019-01-24 5:54 ` Sumit Garg
2019-01-24 10:15 ` Daniel Thompson
2019-01-24 5:54 ` [PATCH v5 4/4] hwrng: add OP-TEE based rng driver Sumit Garg
3 siblings, 1 reply; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 5:54 UTC (permalink / raw)
To: jens.wiklander, herbert, ard.biesheuvel
Cc: linux-arm-kernel, linux-crypto, linux-kernel, yamada.masahiro,
michal.lkml, mpm, robh+dt, mark.rutland, arnd, gregkh,
daniel.thompson, bhsharma, tee-dev, Sumit Garg
OP-TEE provides a pseudo TA to enumerate TAs which can act as devices/
services for TEE bus. So implement device enumeration using invoke
function: PTA_CMD_GET_DEVICES provided by pseudo TA to fetch array of
device UUIDs. Also register these enumerated devices with TEE bus as
"optee-clntX" device.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
drivers/tee/optee/Makefile | 1 +
drivers/tee/optee/core.c | 4 +
drivers/tee/optee/device.c | 153 ++++++++++++++++++++++++++++++++++++++
drivers/tee/optee/optee_private.h | 3 +
4 files changed, 161 insertions(+)
create mode 100644 drivers/tee/optee/device.c
diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index 48d262a..56263ae 100644
--- a/drivers/tee/optee/Makefile
+++ b/drivers/tee/optee/Makefile
@@ -5,3 +5,4 @@ optee-objs += call.o
optee-objs += rpc.o
optee-objs += supp.o
optee-objs += shm_pool.o
+optee-objs += device.o
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index e5efce3..ac59c77 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -634,6 +634,10 @@ static struct optee *optee_probe(struct device_node *np)
if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
pr_info("dynamic shared memory is enabled\n");
+ rc = optee_enumerate_devices();
+ if (rc)
+ goto err;
+
pr_info("initialized driver\n");
return optee;
err:
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
new file mode 100644
index 0000000..987e552
--- /dev/null
+++ b/drivers/tee/optee/device.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Linaro Ltd.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include "optee_private.h"
+
+/*
+ * Get device UUIDs
+ *
+ * [out] memref[0] Array of device UUIDs
+ *
+ * Return codes:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
+ * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required
+ */
+#define PTA_CMD_GET_DEVICES 0x0
+
+static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ if (ver->impl_id == TEE_IMPL_ID_OPTEE)
+ return 1;
+ else
+ return 0;
+}
+
+static int get_devices(struct tee_context *ctx, u32 session,
+ struct tee_shm *device_shm, u32 *shm_size)
+{
+ u32 ret = 0;
+ struct tee_ioctl_invoke_arg inv_arg = {0};
+ struct tee_param param[4] = {0};
+
+ /* Invoke PTA_CMD_GET_DEVICES function */
+ inv_arg.func = PTA_CMD_GET_DEVICES;
+ inv_arg.session = session;
+ inv_arg.num_params = 4;
+
+ /* Fill invoke cmd params */
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+ param[0].u.memref.shm = device_shm;
+ param[0].u.memref.size = *shm_size;
+ param[0].u.memref.shm_offs = 0;
+
+ ret = tee_client_invoke_func(ctx, &inv_arg, param);
+ if ((ret < 0) || ((inv_arg.ret != TEEC_SUCCESS) &&
+ (inv_arg.ret != TEEC_ERROR_SHORT_BUFFER))) {
+ pr_err("PTA_CMD_GET_DEVICES invoke function err: %x\n",
+ inv_arg.ret);
+ return -EINVAL;
+ }
+
+ *shm_size = param[0].u.memref.size;
+
+ return 0;
+}
+
+static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
+{
+ struct tee_client_device *optee_device = NULL;
+ int rc;
+
+ optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
+ if (!optee_device)
+ return -ENOMEM;
+
+ optee_device->dev.bus = &tee_bus_type;
+ dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
+ uuid_copy(&optee_device->id.uuid, device_uuid);
+
+ rc = device_register(&optee_device->dev);
+ if (rc) {
+ pr_err("device registration failed, err: %d\n", rc);
+ kfree(optee_device);
+ }
+
+ return rc;
+}
+
+int optee_enumerate_devices(void)
+{
+ const uuid_t pta_uuid =
+ UUID_INIT(0x7011a688, 0xddde, 0x4053,
+ 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8);
+ struct tee_ioctl_open_session_arg sess_arg = {0};
+ struct tee_shm *device_shm = NULL;
+ const uuid_t *device_uuid = NULL;
+ struct tee_context *ctx = NULL;
+ u32 shm_size = 0, idx, num_devices = 0;
+ int rc;
+
+ /* Open context with OP-TEE driver */
+ ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
+ if (IS_ERR(ctx))
+ return -ENODEV;
+
+ /* Open session with device enumeration pseudo TA */
+ memcpy(sess_arg.uuid, pta_uuid.b, TEE_IOCTL_UUID_LEN);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ sess_arg.num_params = 0;
+
+ rc = tee_client_open_session(ctx, &sess_arg, NULL);
+ if ((rc < 0) || (sess_arg.ret != TEEC_SUCCESS)) {
+ /* Device enumeration pseudo TA not found */
+ rc = 0;
+ goto out_ctx;
+ }
+
+ rc = get_devices(ctx, sess_arg.session, NULL, &shm_size);
+ if (rc < 0)
+ goto out_sess;
+
+ device_shm = tee_shm_alloc(ctx, shm_size,
+ TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
+ if (IS_ERR(device_shm)) {
+ pr_err("tee_shm_alloc failed\n");
+ rc = PTR_ERR(device_shm);
+ goto out_sess;
+ }
+
+ rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size);
+ if (rc < 0)
+ goto out_shm;
+
+ device_uuid = tee_shm_get_va(device_shm, 0);
+ if (IS_ERR(device_uuid)) {
+ pr_err("tee_shm_get_va failed\n");
+ rc = PTR_ERR(device_uuid);
+ goto out_shm;
+ }
+
+ num_devices = shm_size / sizeof(uuid_t);
+
+ for (idx = 0; idx < num_devices; idx++) {
+ rc = optee_register_device(&device_uuid[idx], idx);
+ if (rc)
+ goto out_shm;
+ }
+
+out_shm:
+ tee_shm_free(device_shm);
+out_sess:
+ tee_client_close_session(ctx, sess_arg.session);
+out_ctx:
+ tee_client_close_context(ctx);
+
+ return rc;
+}
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 35e7938..a5e84af 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -28,6 +28,7 @@
#define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
#define TEEC_ERROR_COMMUNICATION 0xFFFF000E
#define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
+#define TEEC_ERROR_SHORT_BUFFER 0xFFFF0010
#define TEEC_ORIGIN_COMMS 0x00000002
@@ -181,6 +182,8 @@ void optee_free_pages_list(void *array, size_t num_entries);
void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages,
size_t page_offset);
+int optee_enumerate_devices(void);
+
/*
* Small helpers
*/
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 4/4] hwrng: add OP-TEE based rng driver
2019-01-24 5:54 [PATCH v5 0/4] Introduce TEE bus driver framework Sumit Garg
` (2 preceding siblings ...)
2019-01-24 5:54 ` [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support Sumit Garg
@ 2019-01-24 5:54 ` Sumit Garg
2019-01-24 12:30 ` Daniel Thompson
2019-01-28 4:30 ` Sumit Garg
3 siblings, 2 replies; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 5:54 UTC (permalink / raw)
To: jens.wiklander, herbert, ard.biesheuvel
Cc: linux-arm-kernel, linux-crypto, linux-kernel, yamada.masahiro,
michal.lkml, mpm, robh+dt, mark.rutland, arnd, gregkh,
daniel.thompson, bhsharma, tee-dev, Sumit Garg
On ARM SoC's with TrustZone enabled, peripherals like entropy sources
might not be accessible to normal world (linux in this case) and rather
accessible to secure world (OP-TEE in this case) only. So this driver
aims to provides a generic interface to OP-TEE based random number
generator service.
This driver registers on TEE bus to interact with OP-TEE based rng
device/service.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
MAINTAINERS | 5 +
drivers/char/hw_random/Kconfig | 15 ++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/optee-rng.c | 274 +++++++++++++++++++++++++++++++++++++
4 files changed, 295 insertions(+)
create mode 100644 drivers/char/hw_random/optee-rng.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 51029a4..dcef7e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11262,6 +11262,11 @@ M: Jens Wiklander <jens.wiklander@linaro.org>
S: Maintained
F: drivers/tee/optee/
+OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
+M: Sumit Garg <sumit.garg@linaro.org>
+S: Maintained
+F: drivers/char/hw_random/optee-rng.c
+
OPA-VNIC DRIVER
M: Dennis Dalessandro <dennis.dalessandro@intel.com>
M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index dac895d..25a7d8f 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -424,6 +424,21 @@ config HW_RANDOM_EXYNOS
will be called exynos-trng.
If unsure, say Y.
+
+config HW_RANDOM_OPTEE
+ tristate "OP-TEE based Random Number Generator support"
+ depends on OPTEE
+ default HW_RANDOM
+ help
+ This driver provides support for OP-TEE based Random Number
+ Generator on ARM SoCs where hardware entropy sources are not
+ accessible to normal world (Linux).
+
+ To compile this driver as a module, choose M here: the module
+ will be called optee-rng.
+
+ If unsure, say Y.
+
endif # HW_RANDOM
config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index e35ec3c..7c9ef4a 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
+obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
new file mode 100644
index 0000000..4ad0eca
--- /dev/null
+++ b/drivers/char/hw_random/optee-rng.c
@@ -0,0 +1,274 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Linaro Ltd.
+ */
+
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/hw_random.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+
+#define DRIVER_NAME "optee-rng"
+
+#define TEE_ERROR_HEALTH_TEST_FAIL 0x00000001
+
+/*
+ * TA_CMD_GET_ENTROPY - Get Entropy from RNG
+ *
+ * param[0] (inout memref) - Entropy buffer memory reference
+ * param[1] unused
+ * param[2] unused
+ * param[3] unused
+ *
+ * Result:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
+ * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool
+ * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
+ */
+#define TA_CMD_GET_ENTROPY 0x0
+
+/*
+ * TA_CMD_GET_RNG_INFO - Get RNG information
+ *
+ * param[0] (out value) - value.a: RNG data-rate in bytes per second
+ * value.b: Quality/Entropy per 1024 bit of data
+ * param[1] unused
+ * param[2] unused
+ * param[3] unused
+ *
+ * Result:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
+ */
+#define TA_CMD_GET_RNG_INFO 0x1
+
+#define MAX_ENTROPY_REQ_SZ (4 * 1024)
+
+static struct tee_context *ctx;
+static struct tee_shm *entropy_shm_pool;
+static u32 ta_rng_data_rate;
+static u32 ta_rng_session_id;
+
+static size_t get_optee_rng_data(void *buf, size_t req_size)
+{
+ u32 ret = 0;
+ u8 *rng_data = NULL;
+ size_t rng_size = 0;
+ struct tee_ioctl_invoke_arg inv_arg = {0};
+ struct tee_param param[4] = {0};
+
+ /* Invoke TA_CMD_GET_ENTROPY function of Trusted App */
+ inv_arg.func = TA_CMD_GET_ENTROPY;
+ inv_arg.session = ta_rng_session_id;
+ inv_arg.num_params = 4;
+
+ /* Fill invoke cmd params */
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ param[0].u.memref.shm = entropy_shm_pool;
+ param[0].u.memref.size = req_size;
+ param[0].u.memref.shm_offs = 0;
+
+ ret = tee_client_invoke_func(ctx, &inv_arg, param);
+ if ((ret < 0) || (inv_arg.ret != 0)) {
+ pr_err("TA_CMD_GET_ENTROPY invoke function error: %x\n",
+ inv_arg.ret);
+ return 0;
+ }
+
+ rng_data = tee_shm_get_va(entropy_shm_pool, 0);
+ if (IS_ERR(rng_data)) {
+ pr_err("tee_shm_get_va failed\n");
+ return 0;
+ }
+
+ rng_size = param[0].u.memref.size;
+ memcpy(buf, rng_data, rng_size);
+
+ return rng_size;
+}
+
+static int optee_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+ u8 *data = buf;
+ size_t read = 0, rng_size = 0;
+ int timeout = 1;
+
+ if (max > MAX_ENTROPY_REQ_SZ)
+ max = MAX_ENTROPY_REQ_SZ;
+
+ while (read == 0) {
+ rng_size = get_optee_rng_data(data, (max - read));
+
+ data += rng_size;
+ read += rng_size;
+
+ if (wait) {
+ if (timeout-- == 0)
+ return read;
+ msleep((1000 * (max - read)) / ta_rng_data_rate);
+ } else {
+ return read;
+ }
+ }
+
+ return read;
+}
+
+static int optee_rng_init(struct hwrng *rng)
+{
+ entropy_shm_pool = tee_shm_alloc(ctx, MAX_ENTROPY_REQ_SZ,
+ TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
+ if (IS_ERR(entropy_shm_pool)) {
+ pr_err("tee_shm_alloc failed\n");
+ return PTR_ERR(entropy_shm_pool);
+ }
+
+ return 0;
+}
+
+static void optee_rng_cleanup(struct hwrng *rng)
+{
+ tee_shm_free(entropy_shm_pool);
+}
+
+static struct hwrng optee_rng = {
+ .name = DRIVER_NAME,
+ .init = optee_rng_init,
+ .cleanup = optee_rng_cleanup,
+ .read = optee_rng_read,
+};
+
+static int get_optee_rng_info(struct device *dev)
+{
+ u32 ret = 0;
+ struct tee_ioctl_invoke_arg inv_arg = {0};
+ struct tee_param param[4] = {0};
+
+ /* Invoke TA_CMD_GET_RNG_INFO function of Trusted App */
+ inv_arg.func = TA_CMD_GET_RNG_INFO;
+ inv_arg.session = ta_rng_session_id;
+ inv_arg.num_params = 4;
+
+ /* Fill invoke cmd params */
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
+
+ ret = tee_client_invoke_func(ctx, &inv_arg, param);
+ if ((ret < 0) || (inv_arg.ret != 0)) {
+ dev_err(dev, "TA_CMD_GET_RNG_INFO invoke func error: %x\n",
+ inv_arg.ret);
+ return -EINVAL;
+ }
+
+ ta_rng_data_rate = param[0].u.value.a;
+ optee_rng.quality = param[0].u.value.b;
+
+ return 0;
+}
+
+static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ if (ver->impl_id == TEE_IMPL_ID_OPTEE)
+ return 1;
+ else
+ return 0;
+}
+
+static int optee_rng_probe(struct device *dev)
+{
+ struct tee_client_device *rng_device = to_tee_client_device(dev);
+ int ret = 0, err = -ENODEV;
+ struct tee_ioctl_open_session_arg sess_arg = {0};
+
+ /* Open context with TEE driver */
+ ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
+ if (IS_ERR(ctx))
+ return -ENODEV;
+
+ /* Open session with hwrng Trusted App */
+ memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ sess_arg.num_params = 0;
+
+ ret = tee_client_open_session(ctx, &sess_arg, NULL);
+ if ((ret < 0) || (sess_arg.ret != 0)) {
+ dev_err(dev, "tee_client_open_session failed, error: %x\n",
+ sess_arg.ret);
+ err = -EINVAL;
+ goto out_ctx;
+ }
+ ta_rng_session_id = sess_arg.session;
+
+ err = get_optee_rng_info(dev);
+ if (err)
+ goto out_sess;
+
+ err = hwrng_register(&optee_rng);
+ if (err) {
+ dev_err(dev, "hwrng registration failed (%d)\n", err);
+ goto out_sess;
+ }
+
+ return 0;
+
+out_sess:
+ tee_client_close_session(ctx, ta_rng_session_id);
+out_ctx:
+ tee_client_close_context(ctx);
+
+ return err;
+}
+
+static int optee_rng_remove(struct device *dev)
+{
+ hwrng_unregister(&optee_rng);
+ tee_client_close_session(ctx, ta_rng_session_id);
+ tee_client_close_context(ctx);
+
+ return 0;
+}
+
+const struct tee_client_device_id optee_rng_id_table[] = {
+ {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
+ 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
+ {}
+};
+
+MODULE_DEVICE_TABLE(tee, optee_rng_id_table);
+
+static struct tee_client_driver optee_rng_driver = {
+ .id_table = optee_rng_id_table,
+ .driver = {
+ .name = DRIVER_NAME,
+ .bus = &tee_bus_type,
+ .probe = optee_rng_probe,
+ .remove = optee_rng_remove,
+ },
+};
+
+static int __init optee_rng_mod_init(void)
+{
+ int rc;
+
+ rc = driver_register(&optee_rng_driver.driver);
+ if (rc)
+ pr_warn("driver registration failed, err: %d", rc);
+
+ return rc;
+}
+
+static void __exit optee_rng_mod_exit(void)
+{
+ driver_unregister(&optee_rng_driver.driver);
+}
+
+module_init(optee_rng_mod_init);
+module_exit(optee_rng_mod_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sumit Garg <sumit.garg@linaro.org>");
+MODULE_DESCRIPTION("OP-TEE based random number generator driver");
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/4] tee: add bus driver framework for TEE based devices
2019-01-24 5:54 ` [PATCH v5 1/4] tee: add bus driver framework for TEE based devices Sumit Garg
@ 2019-01-24 9:45 ` Daniel Thompson
2019-01-24 10:19 ` Sumit Garg
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Thompson @ 2019-01-24 9:45 UTC (permalink / raw)
To: Sumit Garg
Cc: jens.wiklander, herbert, ard.biesheuvel, linux-arm-kernel,
linux-crypto, linux-kernel, yamada.masahiro, michal.lkml, mpm,
robh+dt, mark.rutland, arnd, gregkh, bhsharma, tee-dev
On Thu, Jan 24, 2019 at 11:24:36AM +0530, Sumit Garg wrote:
> Introduce a generic TEE bus driver concept for TEE based kernel drivers
> which would like to communicate with TEE based devices/services. Also
> add support in module device table for these new TEE based devices.
>
> In this TEE bus concept, devices/services are identified via Universally
> Unique Identifier (UUID) and drivers register a table of device UUIDs
> which they can support.
>
> So this TEE bus framework registers following apis:
> - match(): Iterates over the driver UUID table to find a corresponding
> match for device UUID. If a match is found, then this particular device
> is probed via corresponding probe api registered by the driver. This
> process happens whenever a device or a driver is registered with TEE
> bus.
> - uevent(): Notifies user-space (udev) whenever a new device is registered
> on this bus for auto-loading of out-of-tree drivers.
out-of-tree? Surely it can be used to auto-load any modulized driver.
> Also this framework allows for device enumeration to be specific to
> corresponding TEE implementation like OP-TEE etc.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> drivers/tee/tee_core.c | 57 ++++++++++++++++++++++++++++++++++++---
> include/linux/mod_devicetable.h | 9 +++++++
> include/linux/tee_drv.h | 32 +++++++++++++++++++++-
> scripts/mod/devicetable-offsets.c | 3 +++
> scripts/mod/file2alias.c | 19 +++++++++++++
> 5 files changed, 115 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 7b2bb4c..9f4c8bc 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -15,7 +15,6 @@
> #define pr_fmt(fmt) "%s: " fmt, __func__
>
> #include <linux/cdev.h>
> -#include <linux/device.h>
> #include <linux/fs.h>
> #include <linux/idr.h>
> #include <linux/module.h>
> @@ -1027,6 +1026,42 @@ int tee_client_invoke_func(struct tee_context *ctx,
> }
> EXPORT_SYMBOL_GPL(tee_client_invoke_func);
>
> +static int tee_client_device_match(struct device *dev,
> + struct device_driver *drv)
> +{
> + const struct tee_client_device_id *id_table;
> + struct tee_client_device *tee_device;
> +
> + id_table = to_tee_client_driver(drv)->id_table;
> + tee_device = to_tee_client_device(dev);
> +
> + while (!uuid_is_null(&id_table->uuid)) {
> + if (uuid_equal(&tee_device->id.uuid, &id_table->uuid))
> + return 1;
> + id_table++;
> + }
> +
> + return 0;
> +}
> +
> +static int tee_client_device_uevent(struct device *dev,
> + struct kobj_uevent_env *env)
> +{
> + uuid_t *dev_id = &to_tee_client_device(dev)->id.uuid;
> +
> + if (add_uevent_var(env, "MODALIAS=tee:%pUb", dev_id))
> + return -ENOMEM;
> +
> + return 0;
Can just return directly here:
return add_uevent_var(... )
Other than these nitpicks then FWIW:
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> +}
> +
> +struct bus_type tee_bus_type = {
> + .name = "tee",
> + .match = tee_client_device_match,
> + .uevent = tee_client_device_uevent,
> +};
> +EXPORT_SYMBOL_GPL(tee_bus_type);
> +
> static int __init tee_init(void)
> {
> int rc;
> @@ -1040,18 +1075,32 @@ static int __init tee_init(void)
> rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
> if (rc) {
> pr_err("failed to allocate char dev region\n");
> - class_destroy(tee_class);
> - tee_class = NULL;
> + goto out_unreg_class;
> }
>
> + rc = bus_register(&tee_bus_type);
> + if (rc) {
> + pr_err("failed to register tee bus\n");
> + goto out_unreg_chrdev;
> + }
> +
> + return 0;
> +
> +out_unreg_chrdev:
> + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> +out_unreg_class:
> + class_destroy(tee_class);
> + tee_class = NULL;
> +
> return rc;
> }
>
> static void __exit tee_exit(void)
> {
> + bus_unregister(&tee_bus_type);
> + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> class_destroy(tee_class);
> tee_class = NULL;
> - unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> }
>
> subsys_initcall(tee_init);
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index f9bd2f3..14eaeeb 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -779,4 +779,13 @@ struct typec_device_id {
> kernel_ulong_t driver_data;
> };
>
> +/**
> + * struct tee_client_device_id - tee based device identifier
> + * @uuid: For TEE based client devices we use the device uuid as
> + * the identifier.
> + */
> +struct tee_client_device_id {
> + uuid_t uuid;
> +};
> +
> #endif /* LINUX_MOD_DEVICETABLE_H */
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index 6cfe058..ce957ce 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -15,11 +15,14 @@
> #ifndef __TEE_DRV_H
> #define __TEE_DRV_H
>
> -#include <linux/types.h>
> +#include <linux/device.h>
> #include <linux/idr.h>
> #include <linux/kref.h>
> #include <linux/list.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/tee.h>
> +#include <linux/types.h>
> +#include <linux/uuid.h>
>
> /*
> * The file describes the API provided by the generic TEE driver to the
> @@ -538,4 +541,31 @@ static inline bool tee_param_is_memref(struct tee_param *param)
> }
> }
>
> +extern struct bus_type tee_bus_type;
> +
> +/**
> + * struct tee_client_device - tee based device
> + * @id: device identifier
> + * @dev: device structure
> + */
> +struct tee_client_device {
> + struct tee_client_device_id id;
> + struct device dev;
> +};
> +
> +#define to_tee_client_device(d) container_of(d, struct tee_client_device, dev)
> +
> +/**
> + * struct tee_client_driver - tee client driver
> + * @id_table: device id table supported by this driver
> + * @driver: driver structure
> + */
> +struct tee_client_driver {
> + const struct tee_client_device_id *id_table;
> + struct device_driver driver;
> +};
> +
> +#define to_tee_client_driver(d) \
> + container_of(d, struct tee_client_driver, driver)
> +
> #endif /*__TEE_DRV_H*/
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index 2930044..1607183 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -225,5 +225,8 @@ int main(void)
> DEVID_FIELD(typec_device_id, svid);
> DEVID_FIELD(typec_device_id, mode);
>
> + DEVID(tee_client_device_id);
> + DEVID_FIELD(tee_client_device_id, uuid);
> +
> return 0;
> }
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index a37af7d..d0e4172 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -37,6 +37,9 @@ typedef unsigned char __u8;
> typedef struct {
> __u8 b[16];
> } uuid_le;
> +typedef struct {
> + __u8 b[16];
> +} uuid_t;
>
> /* Big exception to the "don't include kernel headers into userspace, which
> * even potentially has different endianness and word sizes, since
> @@ -1287,6 +1290,21 @@ static int do_typec_entry(const char *filename, void *symval, char *alias)
> return 1;
> }
>
> +/* Looks like: tee:uuid */
> +static int do_tee_entry(const char *filename, void *symval, char *alias)
> +{
> + DEF_FIELD(symval, tee_client_device_id, uuid);
> +
> + sprintf(alias, "tee:%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> + uuid.b[0], uuid.b[1], uuid.b[2], uuid.b[3], uuid.b[4],
> + uuid.b[5], uuid.b[6], uuid.b[7], uuid.b[8], uuid.b[9],
> + uuid.b[10], uuid.b[11], uuid.b[12], uuid.b[13], uuid.b[14],
> + uuid.b[15]);
> +
> + add_wildcard(alias);
> + return 1;
> +}
> +
> /* Does namelen bytes of name exactly match the symbol? */
> static bool sym_is(const char *name, unsigned namelen, const char *symbol)
> {
> @@ -1357,6 +1375,7 @@ static const struct devtable devtable[] = {
> {"fslmc", SIZE_fsl_mc_device_id, do_fsl_mc_entry},
> {"tbsvc", SIZE_tb_service_id, do_tbsvc_entry},
> {"typec", SIZE_typec_device_id, do_typec_entry},
> + {"tee", SIZE_tee_client_device_id, do_tee_entry},
> };
>
> /* Create MODULE_ALIAS() statements.
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct
2019-01-24 5:54 ` [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct Sumit Garg
@ 2019-01-24 9:48 ` Daniel Thompson
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Thompson @ 2019-01-24 9:48 UTC (permalink / raw)
To: Sumit Garg
Cc: jens.wiklander, herbert, ard.biesheuvel, linux-arm-kernel,
linux-crypto, linux-kernel, yamada.masahiro, michal.lkml, mpm,
robh+dt, mark.rutland, arnd, gregkh, bhsharma, tee-dev
On Thu, Jan 24, 2019 at 11:24:37AM +0530, Sumit Garg wrote:
> This flag indicates that requests in this context should not wait for
> tee-supplicant daemon to be started if not present and just return
> with an error code. It is needed for requests which should be
> non-blocking in nature like ones arising from TEE based kernel drivers
> or any in kernel api that uses TEE internal client interface.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> drivers/tee/optee/supp.c | 10 +++++++++-
> drivers/tee/tee_core.c | 13 +++++++++++++
> include/linux/tee_drv.h | 6 ++++++
> 3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c
> index 43626e1..92f56b8 100644
> --- a/drivers/tee/optee/supp.c
> +++ b/drivers/tee/optee/supp.c
> @@ -88,10 +88,18 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
> {
> struct optee *optee = tee_get_drvdata(ctx->teedev);
> struct optee_supp *supp = &optee->supp;
> - struct optee_supp_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
> + struct optee_supp_req *req;
> bool interruptable;
> u32 ret;
>
> + /*
> + * Return in case there is no supplicant available and
> + * non-blocking request.
> + */
> + if (!supp->ctx && ctx->supp_nowait)
> + return TEEC_ERROR_COMMUNICATION;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> if (!req)
> return TEEC_ERROR_OUT_OF_MEMORY;
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 9f4c8bc..2612dfa 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -105,6 +105,11 @@ static int tee_open(struct inode *inode, struct file *filp)
> if (IS_ERR(ctx))
> return PTR_ERR(ctx);
>
> + /*
> + * Default user-space behaviour is to wait for tee-supplicant
> + * if not present for any requests in this context.
> + */
> + ctx->supp_nowait = false;
> filp->private_data = ctx;
> return 0;
> }
> @@ -981,6 +986,14 @@ tee_client_open_context(struct tee_context *start,
> } while (IS_ERR(ctx) && PTR_ERR(ctx) != -ENOMEM);
>
> put_device(put_dev);
> + /*
> + * Default behaviour for in kernel client is to not wait for
> + * tee-supplicant if not present for any requests in this context.
> + * Also this flag could be configured again before call to
> + * tee_client_open_session() if any in kernel client requires
> + * different behaviour.
> + */
> + ctx->supp_nowait = true;
> return ctx;
> }
> EXPORT_SYMBOL_GPL(tee_client_open_context);
> diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> index ce957ce..56d7f1b 100644
> --- a/include/linux/tee_drv.h
> +++ b/include/linux/tee_drv.h
> @@ -50,6 +50,11 @@ struct tee_shm_pool;
> * @releasing: flag that indicates if context is being released right now.
> * It is needed to break circular dependency on context during
> * shared memory release.
> + * @supp_nowait: flag that indicates that requests in this context should not
> + * wait for tee-supplicant daemon to be started if not present
> + * and just return with an error code. It is needed for requests
> + * that arises from TEE based kernel drivers that should be
> + * non-blocking in nature.
> */
> struct tee_context {
> struct tee_device *teedev;
> @@ -57,6 +62,7 @@ struct tee_context {
> void *data;
> struct kref refcount;
> bool releasing;
> + bool supp_nowait;
> };
>
> struct tee_param_memref {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support
2019-01-24 5:54 ` [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support Sumit Garg
@ 2019-01-24 10:15 ` Daniel Thompson
2019-01-24 11:28 ` Sumit Garg
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Thompson @ 2019-01-24 10:15 UTC (permalink / raw)
To: Sumit Garg
Cc: jens.wiklander, herbert, ard.biesheuvel, linux-arm-kernel,
linux-crypto, linux-kernel, yamada.masahiro, michal.lkml, mpm,
robh+dt, mark.rutland, arnd, gregkh, bhsharma, tee-dev
On Thu, Jan 24, 2019 at 11:24:38AM +0530, Sumit Garg wrote:
> OP-TEE provides a pseudo TA to enumerate TAs which can act as devices/
> services for TEE bus. So implement device enumeration using invoke
> function: PTA_CMD_GET_DEVICES provided by pseudo TA to fetch array of
> device UUIDs. Also register these enumerated devices with TEE bus as
> "optee-clntX" device.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> drivers/tee/optee/Makefile | 1 +
> drivers/tee/optee/core.c | 4 +
> drivers/tee/optee/device.c | 153 ++++++++++++++++++++++++++++++++++++++
> drivers/tee/optee/optee_private.h | 3 +
> 4 files changed, 161 insertions(+)
> create mode 100644 drivers/tee/optee/device.c
>
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index 48d262a..56263ae 100644
> --- a/drivers/tee/optee/Makefile
> +++ b/drivers/tee/optee/Makefile
> @@ -5,3 +5,4 @@ optee-objs += call.o
> optee-objs += rpc.o
> optee-objs += supp.o
> optee-objs += shm_pool.o
> +optee-objs += device.o
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index e5efce3..ac59c77 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -634,6 +634,10 @@ static struct optee *optee_probe(struct device_node *np)
> if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
> pr_info("dynamic shared memory is enabled\n");
>
> + rc = optee_enumerate_devices();
> + if (rc)
> + goto err;
> +
> pr_info("initialized driver\n");
> return optee;
> err:
> diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
> new file mode 100644
> index 0000000..987e552
> --- /dev/null
> +++ b/drivers/tee/optee/device.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 Linaro Ltd.
> + */
> +
Does there need to be a pr_fmt() here (to make the pr_*() messages from
this file match those from the optee/core.c):
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Alternatively you could get the probe function to pass the optee device
and use dev_*()...
TBH this is pretty severe nitpicking so feel free to add my reviewed
by the next time this patch comes around:
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/uuid.h>
> +#include "optee_private.h"
> +
> +/*
> + * Get device UUIDs
> + *
> + * [out] memref[0] Array of device UUIDs
> + *
> + * Return codes:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required
> + */
> +#define PTA_CMD_GET_DEVICES 0x0
> +
> +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> + if (ver->impl_id == TEE_IMPL_ID_OPTEE)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static int get_devices(struct tee_context *ctx, u32 session,
> + struct tee_shm *device_shm, u32 *shm_size)
> +{
> + u32 ret = 0;
> + struct tee_ioctl_invoke_arg inv_arg = {0};
> + struct tee_param param[4] = {0};
> +
> + /* Invoke PTA_CMD_GET_DEVICES function */
> + inv_arg.func = PTA_CMD_GET_DEVICES;
> + inv_arg.session = session;
> + inv_arg.num_params = 4;
> +
> + /* Fill invoke cmd params */
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
> + param[0].u.memref.shm = device_shm;
> + param[0].u.memref.size = *shm_size;
> + param[0].u.memref.shm_offs = 0;
> +
> + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> + if ((ret < 0) || ((inv_arg.ret != TEEC_SUCCESS) &&
> + (inv_arg.ret != TEEC_ERROR_SHORT_BUFFER))) {
> + pr_err("PTA_CMD_GET_DEVICES invoke function err: %x\n",
> + inv_arg.ret);
> + return -EINVAL;
> + }
> +
> + *shm_size = param[0].u.memref.size;
> +
> + return 0;
> +}
> +
> +static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
> +{
> + struct tee_client_device *optee_device = NULL;
> + int rc;
> +
> + optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
> + if (!optee_device)
> + return -ENOMEM;
> +
> + optee_device->dev.bus = &tee_bus_type;
> + dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
> + uuid_copy(&optee_device->id.uuid, device_uuid);
> +
> + rc = device_register(&optee_device->dev);
> + if (rc) {
> + pr_err("device registration failed, err: %d\n", rc);
> + kfree(optee_device);
> + }
> +
> + return rc;
> +}
> +
> +int optee_enumerate_devices(void)
> +{
> + const uuid_t pta_uuid =
> + UUID_INIT(0x7011a688, 0xddde, 0x4053,
> + 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8);
> + struct tee_ioctl_open_session_arg sess_arg = {0};
> + struct tee_shm *device_shm = NULL;
> + const uuid_t *device_uuid = NULL;
> + struct tee_context *ctx = NULL;
> + u32 shm_size = 0, idx, num_devices = 0;
> + int rc;
> +
> + /* Open context with OP-TEE driver */
> + ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> + if (IS_ERR(ctx))
> + return -ENODEV;
> +
> + /* Open session with device enumeration pseudo TA */
> + memcpy(sess_arg.uuid, pta_uuid.b, TEE_IOCTL_UUID_LEN);
> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> + sess_arg.num_params = 0;
> +
> + rc = tee_client_open_session(ctx, &sess_arg, NULL);
> + if ((rc < 0) || (sess_arg.ret != TEEC_SUCCESS)) {
> + /* Device enumeration pseudo TA not found */
> + rc = 0;
> + goto out_ctx;
> + }
> +
> + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size);
> + if (rc < 0)
> + goto out_sess;
> +
> + device_shm = tee_shm_alloc(ctx, shm_size,
> + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> + if (IS_ERR(device_shm)) {
> + pr_err("tee_shm_alloc failed\n");
> + rc = PTR_ERR(device_shm);
> + goto out_sess;
> + }
> +
> + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size);
> + if (rc < 0)
> + goto out_shm;
> +
> + device_uuid = tee_shm_get_va(device_shm, 0);
> + if (IS_ERR(device_uuid)) {
> + pr_err("tee_shm_get_va failed\n");
> + rc = PTR_ERR(device_uuid);
> + goto out_shm;
> + }
> +
> + num_devices = shm_size / sizeof(uuid_t);
> +
> + for (idx = 0; idx < num_devices; idx++) {
> + rc = optee_register_device(&device_uuid[idx], idx);
> + if (rc)
> + goto out_shm;
> + }
> +
> +out_shm:
> + tee_shm_free(device_shm);
> +out_sess:
> + tee_client_close_session(ctx, sess_arg.session);
> +out_ctx:
> + tee_client_close_context(ctx);
> +
> + return rc;
> +}
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 35e7938..a5e84af 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -28,6 +28,7 @@
> #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
> #define TEEC_ERROR_COMMUNICATION 0xFFFF000E
> #define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
> +#define TEEC_ERROR_SHORT_BUFFER 0xFFFF0010
>
> #define TEEC_ORIGIN_COMMS 0x00000002
>
> @@ -181,6 +182,8 @@ void optee_free_pages_list(void *array, size_t num_entries);
> void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages,
> size_t page_offset);
>
> +int optee_enumerate_devices(void);
> +
> /*
> * Small helpers
> */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/4] tee: add bus driver framework for TEE based devices
2019-01-24 9:45 ` Daniel Thompson
@ 2019-01-24 10:19 ` Sumit Garg
2019-01-24 10:49 ` Bhupesh Sharma
0 siblings, 1 reply; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 10:19 UTC (permalink / raw)
To: Daniel Thompson
Cc: Jens Wiklander, Herbert Xu, Ard Biesheuvel, linux-arm-kernel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, yamada.masahiro, michal.lkml, mpm,
Rob Herring, Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman,
Bhupesh Sharma, tee-dev
On Thu, 24 Jan 2019 at 15:15, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Thu, Jan 24, 2019 at 11:24:36AM +0530, Sumit Garg wrote:
> > Introduce a generic TEE bus driver concept for TEE based kernel drivers
> > which would like to communicate with TEE based devices/services. Also
> > add support in module device table for these new TEE based devices.
> >
> > In this TEE bus concept, devices/services are identified via Universally
> > Unique Identifier (UUID) and drivers register a table of device UUIDs
> > which they can support.
> >
> > So this TEE bus framework registers following apis:
> > - match(): Iterates over the driver UUID table to find a corresponding
> > match for device UUID. If a match is found, then this particular device
> > is probed via corresponding probe api registered by the driver. This
> > process happens whenever a device or a driver is registered with TEE
> > bus.
> > - uevent(): Notifies user-space (udev) whenever a new device is registered
> > on this bus for auto-loading of out-of-tree drivers.
>
> out-of-tree? Surely it can be used to auto-load any modulized driver.
>
Agree, will correct.
>
> > Also this framework allows for device enumeration to be specific to
> > corresponding TEE implementation like OP-TEE etc.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > drivers/tee/tee_core.c | 57 ++++++++++++++++++++++++++++++++++++---
> > include/linux/mod_devicetable.h | 9 +++++++
> > include/linux/tee_drv.h | 32 +++++++++++++++++++++-
> > scripts/mod/devicetable-offsets.c | 3 +++
> > scripts/mod/file2alias.c | 19 +++++++++++++
> > 5 files changed, 115 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 7b2bb4c..9f4c8bc 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -15,7 +15,6 @@
> > #define pr_fmt(fmt) "%s: " fmt, __func__
> >
> > #include <linux/cdev.h>
> > -#include <linux/device.h>
> > #include <linux/fs.h>
> > #include <linux/idr.h>
> > #include <linux/module.h>
> > @@ -1027,6 +1026,42 @@ int tee_client_invoke_func(struct tee_context *ctx,
> > }
> > EXPORT_SYMBOL_GPL(tee_client_invoke_func);
> >
> > +static int tee_client_device_match(struct device *dev,
> > + struct device_driver *drv)
> > +{
> > + const struct tee_client_device_id *id_table;
> > + struct tee_client_device *tee_device;
> > +
> > + id_table = to_tee_client_driver(drv)->id_table;
> > + tee_device = to_tee_client_device(dev);
> > +
> > + while (!uuid_is_null(&id_table->uuid)) {
> > + if (uuid_equal(&tee_device->id.uuid, &id_table->uuid))
> > + return 1;
> > + id_table++;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tee_client_device_uevent(struct device *dev,
> > + struct kobj_uevent_env *env)
> > +{
> > + uuid_t *dev_id = &to_tee_client_device(dev)->id.uuid;
> > +
> > + if (add_uevent_var(env, "MODALIAS=tee:%pUb", dev_id))
> > + return -ENOMEM;
> > +
> > + return 0;
>
> Can just return directly here:
>
> return add_uevent_var(... )
Okay will use this instead.
>
> Other than these nitpicks then FWIW:
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>
Thanks.
-Sumit
> > +}
> > +
> > +struct bus_type tee_bus_type = {
> > + .name = "tee",
> > + .match = tee_client_device_match,
> > + .uevent = tee_client_device_uevent,
> > +};
> > +EXPORT_SYMBOL_GPL(tee_bus_type);
> > +
> > static int __init tee_init(void)
> > {
> > int rc;
> > @@ -1040,18 +1075,32 @@ static int __init tee_init(void)
> > rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
> > if (rc) {
> > pr_err("failed to allocate char dev region\n");
> > - class_destroy(tee_class);
> > - tee_class = NULL;
> > + goto out_unreg_class;
> > }
> >
> > + rc = bus_register(&tee_bus_type);
> > + if (rc) {
> > + pr_err("failed to register tee bus\n");
> > + goto out_unreg_chrdev;
> > + }
> > +
> > + return 0;
> > +
> > +out_unreg_chrdev:
> > + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > +out_unreg_class:
> > + class_destroy(tee_class);
> > + tee_class = NULL;
> > +
> > return rc;
> > }
> >
> > static void __exit tee_exit(void)
> > {
> > + bus_unregister(&tee_bus_type);
> > + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > class_destroy(tee_class);
> > tee_class = NULL;
> > - unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > }
> >
> > subsys_initcall(tee_init);
> > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> > index f9bd2f3..14eaeeb 100644
> > --- a/include/linux/mod_devicetable.h
> > +++ b/include/linux/mod_devicetable.h
> > @@ -779,4 +779,13 @@ struct typec_device_id {
> > kernel_ulong_t driver_data;
> > };
> >
> > +/**
> > + * struct tee_client_device_id - tee based device identifier
> > + * @uuid: For TEE based client devices we use the device uuid as
> > + * the identifier.
> > + */
> > +struct tee_client_device_id {
> > + uuid_t uuid;
> > +};
> > +
> > #endif /* LINUX_MOD_DEVICETABLE_H */
> > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> > index 6cfe058..ce957ce 100644
> > --- a/include/linux/tee_drv.h
> > +++ b/include/linux/tee_drv.h
> > @@ -15,11 +15,14 @@
> > #ifndef __TEE_DRV_H
> > #define __TEE_DRV_H
> >
> > -#include <linux/types.h>
> > +#include <linux/device.h>
> > #include <linux/idr.h>
> > #include <linux/kref.h>
> > #include <linux/list.h>
> > +#include <linux/mod_devicetable.h>
> > #include <linux/tee.h>
> > +#include <linux/types.h>
> > +#include <linux/uuid.h>
> >
> > /*
> > * The file describes the API provided by the generic TEE driver to the
> > @@ -538,4 +541,31 @@ static inline bool tee_param_is_memref(struct tee_param *param)
> > }
> > }
> >
> > +extern struct bus_type tee_bus_type;
> > +
> > +/**
> > + * struct tee_client_device - tee based device
> > + * @id: device identifier
> > + * @dev: device structure
> > + */
> > +struct tee_client_device {
> > + struct tee_client_device_id id;
> > + struct device dev;
> > +};
> > +
> > +#define to_tee_client_device(d) container_of(d, struct tee_client_device, dev)
> > +
> > +/**
> > + * struct tee_client_driver - tee client driver
> > + * @id_table: device id table supported by this driver
> > + * @driver: driver structure
> > + */
> > +struct tee_client_driver {
> > + const struct tee_client_device_id *id_table;
> > + struct device_driver driver;
> > +};
> > +
> > +#define to_tee_client_driver(d) \
> > + container_of(d, struct tee_client_driver, driver)
> > +
> > #endif /*__TEE_DRV_H*/
> > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> > index 2930044..1607183 100644
> > --- a/scripts/mod/devicetable-offsets.c
> > +++ b/scripts/mod/devicetable-offsets.c
> > @@ -225,5 +225,8 @@ int main(void)
> > DEVID_FIELD(typec_device_id, svid);
> > DEVID_FIELD(typec_device_id, mode);
> >
> > + DEVID(tee_client_device_id);
> > + DEVID_FIELD(tee_client_device_id, uuid);
> > +
> > return 0;
> > }
> > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> > index a37af7d..d0e4172 100644
> > --- a/scripts/mod/file2alias.c
> > +++ b/scripts/mod/file2alias.c
> > @@ -37,6 +37,9 @@ typedef unsigned char __u8;
> > typedef struct {
> > __u8 b[16];
> > } uuid_le;
> > +typedef struct {
> > + __u8 b[16];
> > +} uuid_t;
> >
> > /* Big exception to the "don't include kernel headers into userspace, which
> > * even potentially has different endianness and word sizes, since
> > @@ -1287,6 +1290,21 @@ static int do_typec_entry(const char *filename, void *symval, char *alias)
> > return 1;
> > }
> >
> > +/* Looks like: tee:uuid */
> > +static int do_tee_entry(const char *filename, void *symval, char *alias)
> > +{
> > + DEF_FIELD(symval, tee_client_device_id, uuid);
> > +
> > + sprintf(alias, "tee:%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> > + uuid.b[0], uuid.b[1], uuid.b[2], uuid.b[3], uuid.b[4],
> > + uuid.b[5], uuid.b[6], uuid.b[7], uuid.b[8], uuid.b[9],
> > + uuid.b[10], uuid.b[11], uuid.b[12], uuid.b[13], uuid.b[14],
> > + uuid.b[15]);
> > +
> > + add_wildcard(alias);
> > + return 1;
> > +}
> > +
> > /* Does namelen bytes of name exactly match the symbol? */
> > static bool sym_is(const char *name, unsigned namelen, const char *symbol)
> > {
> > @@ -1357,6 +1375,7 @@ static const struct devtable devtable[] = {
> > {"fslmc", SIZE_fsl_mc_device_id, do_fsl_mc_entry},
> > {"tbsvc", SIZE_tb_service_id, do_tbsvc_entry},
> > {"typec", SIZE_typec_device_id, do_typec_entry},
> > + {"tee", SIZE_tee_client_device_id, do_tee_entry},
> > };
> >
> > /* Create MODULE_ALIAS() statements.
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/4] tee: add bus driver framework for TEE based devices
2019-01-24 10:19 ` Sumit Garg
@ 2019-01-24 10:49 ` Bhupesh Sharma
0 siblings, 0 replies; 14+ messages in thread
From: Bhupesh Sharma @ 2019-01-24 10:49 UTC (permalink / raw)
To: Sumit Garg
Cc: Daniel Thompson, Jens Wiklander, Herbert Xu, Ard Biesheuvel,
linux-arm-kernel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, yamada.masahiro, michal.lkml, mpm,
Rob Herring, Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman,
tee-dev
On Thu, Jan 24, 2019 at 3:49 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> On Thu, 24 Jan 2019 at 15:15, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Thu, Jan 24, 2019 at 11:24:36AM +0530, Sumit Garg wrote:
> > > Introduce a generic TEE bus driver concept for TEE based kernel drivers
> > > which would like to communicate with TEE based devices/services. Also
> > > add support in module device table for these new TEE based devices.
> > >
> > > In this TEE bus concept, devices/services are identified via Universally
> > > Unique Identifier (UUID) and drivers register a table of device UUIDs
> > > which they can support.
> > >
> > > So this TEE bus framework registers following apis:
> > > - match(): Iterates over the driver UUID table to find a corresponding
> > > match for device UUID. If a match is found, then this particular device
> > > is probed via corresponding probe api registered by the driver. This
> > > process happens whenever a device or a driver is registered with TEE
> > > bus.
> > > - uevent(): Notifies user-space (udev) whenever a new device is registered
> > > on this bus for auto-loading of out-of-tree drivers.
> >
> > out-of-tree? Surely it can be used to auto-load any modulized driver.
> >
>
> Agree, will correct.
>
> >
> > > Also this framework allows for device enumeration to be specific to
> > > corresponding TEE implementation like OP-TEE etc.
> > >
> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > > ---
> > > drivers/tee/tee_core.c | 57 ++++++++++++++++++++++++++++++++++++---
> > > include/linux/mod_devicetable.h | 9 +++++++
> > > include/linux/tee_drv.h | 32 +++++++++++++++++++++-
> > > scripts/mod/devicetable-offsets.c | 3 +++
> > > scripts/mod/file2alias.c | 19 +++++++++++++
> > > 5 files changed, 115 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > > index 7b2bb4c..9f4c8bc 100644
> > > --- a/drivers/tee/tee_core.c
> > > +++ b/drivers/tee/tee_core.c
> > > @@ -15,7 +15,6 @@
> > > #define pr_fmt(fmt) "%s: " fmt, __func__
> > >
> > > #include <linux/cdev.h>
> > > -#include <linux/device.h>
> > > #include <linux/fs.h>
> > > #include <linux/idr.h>
> > > #include <linux/module.h>
> > > @@ -1027,6 +1026,42 @@ int tee_client_invoke_func(struct tee_context *ctx,
> > > }
> > > EXPORT_SYMBOL_GPL(tee_client_invoke_func);
> > >
> > > +static int tee_client_device_match(struct device *dev,
> > > + struct device_driver *drv)
> > > +{
> > > + const struct tee_client_device_id *id_table;
> > > + struct tee_client_device *tee_device;
> > > +
> > > + id_table = to_tee_client_driver(drv)->id_table;
> > > + tee_device = to_tee_client_device(dev);
> > > +
> > > + while (!uuid_is_null(&id_table->uuid)) {
> > > + if (uuid_equal(&tee_device->id.uuid, &id_table->uuid))
> > > + return 1;
> > > + id_table++;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tee_client_device_uevent(struct device *dev,
> > > + struct kobj_uevent_env *env)
> > > +{
> > > + uuid_t *dev_id = &to_tee_client_device(dev)->id.uuid;
> > > +
> > > + if (add_uevent_var(env, "MODALIAS=tee:%pUb", dev_id))
> > > + return -ENOMEM;
> > > +
> > > + return 0;
> >
> > Can just return directly here:
> >
> > return add_uevent_var(... )
>
> Okay will use this instead.
>
> >
> > Other than these nitpicks then FWIW:
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> >
>
> Thanks.
> -Sumit
>
> > > +}
> > > +
> > > +struct bus_type tee_bus_type = {
> > > + .name = "tee",
> > > + .match = tee_client_device_match,
> > > + .uevent = tee_client_device_uevent,
> > > +};
> > > +EXPORT_SYMBOL_GPL(tee_bus_type);
> > > +
> > > static int __init tee_init(void)
> > > {
> > > int rc;
> > > @@ -1040,18 +1075,32 @@ static int __init tee_init(void)
> > > rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
> > > if (rc) {
> > > pr_err("failed to allocate char dev region\n");
> > > - class_destroy(tee_class);
> > > - tee_class = NULL;
> > > + goto out_unreg_class;
> > > }
> > >
> > > + rc = bus_register(&tee_bus_type);
> > > + if (rc) {
> > > + pr_err("failed to register tee bus\n");
> > > + goto out_unreg_chrdev;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +out_unreg_chrdev:
> > > + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > > +out_unreg_class:
> > > + class_destroy(tee_class);
> > > + tee_class = NULL;
> > > +
> > > return rc;
> > > }
> > >
> > > static void __exit tee_exit(void)
> > > {
> > > + bus_unregister(&tee_bus_type);
> > > + unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > > class_destroy(tee_class);
> > > tee_class = NULL;
> > > - unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> > > }
> > >
> > > subsys_initcall(tee_init);
> > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> > > index f9bd2f3..14eaeeb 100644
> > > --- a/include/linux/mod_devicetable.h
> > > +++ b/include/linux/mod_devicetable.h
> > > @@ -779,4 +779,13 @@ struct typec_device_id {
> > > kernel_ulong_t driver_data;
> > > };
> > >
> > > +/**
> > > + * struct tee_client_device_id - tee based device identifier
> > > + * @uuid: For TEE based client devices we use the device uuid as
> > > + * the identifier.
> > > + */
> > > +struct tee_client_device_id {
> > > + uuid_t uuid;
> > > +};
> > > +
> > > #endif /* LINUX_MOD_DEVICETABLE_H */
> > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
> > > index 6cfe058..ce957ce 100644
> > > --- a/include/linux/tee_drv.h
> > > +++ b/include/linux/tee_drv.h
> > > @@ -15,11 +15,14 @@
> > > #ifndef __TEE_DRV_H
> > > #define __TEE_DRV_H
> > >
> > > -#include <linux/types.h>
> > > +#include <linux/device.h>
> > > #include <linux/idr.h>
> > > #include <linux/kref.h>
> > > #include <linux/list.h>
> > > +#include <linux/mod_devicetable.h>
> > > #include <linux/tee.h>
> > > +#include <linux/types.h>
> > > +#include <linux/uuid.h>
> > >
> > > /*
> > > * The file describes the API provided by the generic TEE driver to the
> > > @@ -538,4 +541,31 @@ static inline bool tee_param_is_memref(struct tee_param *param)
> > > }
> > > }
> > >
> > > +extern struct bus_type tee_bus_type;
> > > +
> > > +/**
> > > + * struct tee_client_device - tee based device
> > > + * @id: device identifier
> > > + * @dev: device structure
> > > + */
> > > +struct tee_client_device {
> > > + struct tee_client_device_id id;
> > > + struct device dev;
> > > +};
> > > +
> > > +#define to_tee_client_device(d) container_of(d, struct tee_client_device, dev)
> > > +
> > > +/**
> > > + * struct tee_client_driver - tee client driver
> > > + * @id_table: device id table supported by this driver
> > > + * @driver: driver structure
> > > + */
> > > +struct tee_client_driver {
> > > + const struct tee_client_device_id *id_table;
> > > + struct device_driver driver;
> > > +};
> > > +
> > > +#define to_tee_client_driver(d) \
> > > + container_of(d, struct tee_client_driver, driver)
> > > +
> > > #endif /*__TEE_DRV_H*/
> > > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> > > index 2930044..1607183 100644
> > > --- a/scripts/mod/devicetable-offsets.c
> > > +++ b/scripts/mod/devicetable-offsets.c
> > > @@ -225,5 +225,8 @@ int main(void)
> > > DEVID_FIELD(typec_device_id, svid);
> > > DEVID_FIELD(typec_device_id, mode);
> > >
> > > + DEVID(tee_client_device_id);
> > > + DEVID_FIELD(tee_client_device_id, uuid);
> > > +
> > > return 0;
> > > }
> > > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> > > index a37af7d..d0e4172 100644
> > > --- a/scripts/mod/file2alias.c
> > > +++ b/scripts/mod/file2alias.c
> > > @@ -37,6 +37,9 @@ typedef unsigned char __u8;
> > > typedef struct {
> > > __u8 b[16];
> > > } uuid_le;
> > > +typedef struct {
> > > + __u8 b[16];
> > > +} uuid_t;
> > >
> > > /* Big exception to the "don't include kernel headers into userspace, which
> > > * even potentially has different endianness and word sizes, since
> > > @@ -1287,6 +1290,21 @@ static int do_typec_entry(const char *filename, void *symval, char *alias)
> > > return 1;
> > > }
> > >
> > > +/* Looks like: tee:uuid */
> > > +static int do_tee_entry(const char *filename, void *symval, char *alias)
> > > +{
> > > + DEF_FIELD(symval, tee_client_device_id, uuid);
> > > +
> > > + sprintf(alias, "tee:%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> > > + uuid.b[0], uuid.b[1], uuid.b[2], uuid.b[3], uuid.b[4],
> > > + uuid.b[5], uuid.b[6], uuid.b[7], uuid.b[8], uuid.b[9],
> > > + uuid.b[10], uuid.b[11], uuid.b[12], uuid.b[13], uuid.b[14],
> > > + uuid.b[15]);
> > > +
> > > + add_wildcard(alias);
> > > + return 1;
> > > +}
> > > +
> > > /* Does namelen bytes of name exactly match the symbol? */
> > > static bool sym_is(const char *name, unsigned namelen, const char *symbol)
> > > {
> > > @@ -1357,6 +1375,7 @@ static const struct devtable devtable[] = {
> > > {"fslmc", SIZE_fsl_mc_device_id, do_fsl_mc_entry},
> > > {"tbsvc", SIZE_tb_service_id, do_tbsvc_entry},
> > > {"typec", SIZE_typec_device_id, do_typec_entry},
> > > + {"tee", SIZE_tee_client_device_id, do_tee_entry},
> > > };
> > >
> > > /* Create MODULE_ALIAS() statements.
> > > --
> > > 2.7.4
> > >
With Daniel's inputs addressed, for this patch:
Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
Thanks
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support
2019-01-24 10:15 ` Daniel Thompson
@ 2019-01-24 11:28 ` Sumit Garg
0 siblings, 0 replies; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 11:28 UTC (permalink / raw)
To: Daniel Thompson
Cc: Jens Wiklander, Herbert Xu, Ard Biesheuvel, linux-arm-kernel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, yamada.masahiro, michal.lkml, mpm,
Rob Herring, Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman,
Bhupesh Sharma, tee-dev
On Thu, 24 Jan 2019 at 15:45, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Thu, Jan 24, 2019 at 11:24:38AM +0530, Sumit Garg wrote:
> > OP-TEE provides a pseudo TA to enumerate TAs which can act as devices/
> > services for TEE bus. So implement device enumeration using invoke
> > function: PTA_CMD_GET_DEVICES provided by pseudo TA to fetch array of
> > device UUIDs. Also register these enumerated devices with TEE bus as
> > "optee-clntX" device.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > drivers/tee/optee/Makefile | 1 +
> > drivers/tee/optee/core.c | 4 +
> > drivers/tee/optee/device.c | 153 ++++++++++++++++++++++++++++++++++++++
> > drivers/tee/optee/optee_private.h | 3 +
> > 4 files changed, 161 insertions(+)
> > create mode 100644 drivers/tee/optee/device.c
> >
> > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > index 48d262a..56263ae 100644
> > --- a/drivers/tee/optee/Makefile
> > +++ b/drivers/tee/optee/Makefile
> > @@ -5,3 +5,4 @@ optee-objs += call.o
> > optee-objs += rpc.o
> > optee-objs += supp.o
> > optee-objs += shm_pool.o
> > +optee-objs += device.o
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > index e5efce3..ac59c77 100644
> > --- a/drivers/tee/optee/core.c
> > +++ b/drivers/tee/optee/core.c
> > @@ -634,6 +634,10 @@ static struct optee *optee_probe(struct device_node *np)
> > if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
> > pr_info("dynamic shared memory is enabled\n");
> >
> > + rc = optee_enumerate_devices();
> > + if (rc)
> > + goto err;
> > +
> > pr_info("initialized driver\n");
> > return optee;
> > err:
> > diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
> > new file mode 100644
> > index 0000000..987e552
> > --- /dev/null
> > +++ b/drivers/tee/optee/device.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 Linaro Ltd.
> > + */
> > +
>
> Does there need to be a pr_fmt() here (to make the pr_*() messages from
> this file match those from the optee/core.c):
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
I think for consistency with optee/core.c file prints, will add this here.
> Alternatively you could get the probe function to pass the optee device
> and use dev_*()...
>
> TBH this is pretty severe nitpicking so feel free to add my reviewed
> by the next time this patch comes around:
>
No worries :).
-Sumit
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>
>
> Daniel.
>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/tee_drv.h>
> > +#include <linux/uuid.h>
> > +#include "optee_private.h"
> > +
> > +/*
> > + * Get device UUIDs
> > + *
> > + * [out] memref[0] Array of device UUIDs
> > + *
> > + * Return codes:
> > + * TEE_SUCCESS - Invoke command success
> > + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> > + * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required
> > + */
> > +#define PTA_CMD_GET_DEVICES 0x0
> > +
> > +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> > +{
> > + if (ver->impl_id == TEE_IMPL_ID_OPTEE)
> > + return 1;
> > + else
> > + return 0;
> > +}
> > +
> > +static int get_devices(struct tee_context *ctx, u32 session,
> > + struct tee_shm *device_shm, u32 *shm_size)
> > +{
> > + u32 ret = 0;
> > + struct tee_ioctl_invoke_arg inv_arg = {0};
> > + struct tee_param param[4] = {0};
> > +
> > + /* Invoke PTA_CMD_GET_DEVICES function */
> > + inv_arg.func = PTA_CMD_GET_DEVICES;
> > + inv_arg.session = session;
> > + inv_arg.num_params = 4;
> > +
> > + /* Fill invoke cmd params */
> > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
> > + param[0].u.memref.shm = device_shm;
> > + param[0].u.memref.size = *shm_size;
> > + param[0].u.memref.shm_offs = 0;
> > +
> > + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> > + if ((ret < 0) || ((inv_arg.ret != TEEC_SUCCESS) &&
> > + (inv_arg.ret != TEEC_ERROR_SHORT_BUFFER))) {
> > + pr_err("PTA_CMD_GET_DEVICES invoke function err: %x\n",
> > + inv_arg.ret);
> > + return -EINVAL;
> > + }
> > +
> > + *shm_size = param[0].u.memref.size;
> > +
> > + return 0;
> > +}
> > +
> > +static int optee_register_device(const uuid_t *device_uuid, u32 device_id)
> > +{
> > + struct tee_client_device *optee_device = NULL;
> > + int rc;
> > +
> > + optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
> > + if (!optee_device)
> > + return -ENOMEM;
> > +
> > + optee_device->dev.bus = &tee_bus_type;
> > + dev_set_name(&optee_device->dev, "optee-clnt%u", device_id);
> > + uuid_copy(&optee_device->id.uuid, device_uuid);
> > +
> > + rc = device_register(&optee_device->dev);
> > + if (rc) {
> > + pr_err("device registration failed, err: %d\n", rc);
> > + kfree(optee_device);
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +int optee_enumerate_devices(void)
> > +{
> > + const uuid_t pta_uuid =
> > + UUID_INIT(0x7011a688, 0xddde, 0x4053,
> > + 0xa5, 0xa9, 0x7b, 0x3c, 0x4d, 0xdf, 0x13, 0xb8);
> > + struct tee_ioctl_open_session_arg sess_arg = {0};
> > + struct tee_shm *device_shm = NULL;
> > + const uuid_t *device_uuid = NULL;
> > + struct tee_context *ctx = NULL;
> > + u32 shm_size = 0, idx, num_devices = 0;
> > + int rc;
> > +
> > + /* Open context with OP-TEE driver */
> > + ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> > + if (IS_ERR(ctx))
> > + return -ENODEV;
> > +
> > + /* Open session with device enumeration pseudo TA */
> > + memcpy(sess_arg.uuid, pta_uuid.b, TEE_IOCTL_UUID_LEN);
> > + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> > + sess_arg.num_params = 0;
> > +
> > + rc = tee_client_open_session(ctx, &sess_arg, NULL);
> > + if ((rc < 0) || (sess_arg.ret != TEEC_SUCCESS)) {
> > + /* Device enumeration pseudo TA not found */
> > + rc = 0;
> > + goto out_ctx;
> > + }
> > +
> > + rc = get_devices(ctx, sess_arg.session, NULL, &shm_size);
> > + if (rc < 0)
> > + goto out_sess;
> > +
> > + device_shm = tee_shm_alloc(ctx, shm_size,
> > + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> > + if (IS_ERR(device_shm)) {
> > + pr_err("tee_shm_alloc failed\n");
> > + rc = PTR_ERR(device_shm);
> > + goto out_sess;
> > + }
> > +
> > + rc = get_devices(ctx, sess_arg.session, device_shm, &shm_size);
> > + if (rc < 0)
> > + goto out_shm;
> > +
> > + device_uuid = tee_shm_get_va(device_shm, 0);
> > + if (IS_ERR(device_uuid)) {
> > + pr_err("tee_shm_get_va failed\n");
> > + rc = PTR_ERR(device_uuid);
> > + goto out_shm;
> > + }
> > +
> > + num_devices = shm_size / sizeof(uuid_t);
> > +
> > + for (idx = 0; idx < num_devices; idx++) {
> > + rc = optee_register_device(&device_uuid[idx], idx);
> > + if (rc)
> > + goto out_shm;
> > + }
> > +
> > +out_shm:
> > + tee_shm_free(device_shm);
> > +out_sess:
> > + tee_client_close_session(ctx, sess_arg.session);
> > +out_ctx:
> > + tee_client_close_context(ctx);
> > +
> > + return rc;
> > +}
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index 35e7938..a5e84af 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -28,6 +28,7 @@
> > #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
> > #define TEEC_ERROR_COMMUNICATION 0xFFFF000E
> > #define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
> > +#define TEEC_ERROR_SHORT_BUFFER 0xFFFF0010
> >
> > #define TEEC_ORIGIN_COMMS 0x00000002
> >
> > @@ -181,6 +182,8 @@ void optee_free_pages_list(void *array, size_t num_entries);
> > void optee_fill_pages_list(u64 *dst, struct page **pages, int num_pages,
> > size_t page_offset);
> >
> > +int optee_enumerate_devices(void);
> > +
> > /*
> > * Small helpers
> > */
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 4/4] hwrng: add OP-TEE based rng driver
2019-01-24 5:54 ` [PATCH v5 4/4] hwrng: add OP-TEE based rng driver Sumit Garg
@ 2019-01-24 12:30 ` Daniel Thompson
2019-01-24 14:02 ` Sumit Garg
2019-01-28 4:30 ` Sumit Garg
1 sibling, 1 reply; 14+ messages in thread
From: Daniel Thompson @ 2019-01-24 12:30 UTC (permalink / raw)
To: Sumit Garg
Cc: jens.wiklander, herbert, ard.biesheuvel, linux-arm-kernel,
linux-crypto, linux-kernel, yamada.masahiro, michal.lkml, mpm,
robh+dt, mark.rutland, arnd, gregkh, bhsharma, tee-dev
On Thu, Jan 24, 2019 at 11:24:39AM +0530, Sumit Garg wrote:
> On ARM SoC's with TrustZone enabled, peripherals like entropy sources
> might not be accessible to normal world (linux in this case) and rather
> accessible to secure world (OP-TEE in this case) only. So this driver
> aims to provides a generic interface to OP-TEE based random number
> generator service.
>
> This driver registers on TEE bus to interact with OP-TEE based rng
> device/service.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> MAINTAINERS | 5 +
> drivers/char/hw_random/Kconfig | 15 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/optee-rng.c | 274 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 295 insertions(+)
> create mode 100644 drivers/char/hw_random/optee-rng.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 51029a4..dcef7e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11262,6 +11262,11 @@ M: Jens Wiklander <jens.wiklander@linaro.org>
> S: Maintained
> F: drivers/tee/optee/
>
> +OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
> +M: Sumit Garg <sumit.garg@linaro.org>
> +S: Maintained
> +F: drivers/char/hw_random/optee-rng.c
> +
> OPA-VNIC DRIVER
> M: Dennis Dalessandro <dennis.dalessandro@intel.com>
> M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index dac895d..25a7d8f 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -424,6 +424,21 @@ config HW_RANDOM_EXYNOS
> will be called exynos-trng.
>
> If unsure, say Y.
> +
> +config HW_RANDOM_OPTEE
> + tristate "OP-TEE based Random Number Generator support"
> + depends on OPTEE
> + default HW_RANDOM
> + help
> + This driver provides support for OP-TEE based Random Number
> + Generator on ARM SoCs where hardware entropy sources are not
> + accessible to normal world (Linux).
> +
> + To compile this driver as a module, choose M here: the module
> + will be called optee-rng.
> +
> + If unsure, say Y.
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index e35ec3c..7c9ef4a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -38,3 +38,4 @@ obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
> obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
> obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
> +obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
> new file mode 100644
> index 0000000..4ad0eca
> --- /dev/null
> +++ b/drivers/char/hw_random/optee-rng.c
> @@ -0,0 +1,274 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018-2019 Linaro Ltd.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/hw_random.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/uuid.h>
> +
> +#define DRIVER_NAME "optee-rng"
> +
> +#define TEE_ERROR_HEALTH_TEST_FAIL 0x00000001
> +
> +/*
> + * TA_CMD_GET_ENTROPY - Get Entropy from RNG
> + *
> + * param[0] (inout memref) - Entropy buffer memory reference
> + * param[1] unused
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool
> + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
> + */
> +#define TA_CMD_GET_ENTROPY 0x0
> +
> +/*
> + * TA_CMD_GET_RNG_INFO - Get RNG information
> + *
> + * param[0] (out value) - value.a: RNG data-rate in bytes per second
> + * value.b: Quality/Entropy per 1024 bit of data
> + * param[1] unused
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + */
> +#define TA_CMD_GET_RNG_INFO 0x1
> +
> +#define MAX_ENTROPY_REQ_SZ (4 * 1024)
> +
> +static struct tee_context *ctx;
> +static struct tee_shm *entropy_shm_pool;
> +static u32 ta_rng_data_rate;
> +static u32 ta_rng_session_id;
> +
> +static size_t get_optee_rng_data(void *buf, size_t req_size)
> +{
> + u32 ret = 0;
> + u8 *rng_data = NULL;
> + size_t rng_size = 0;
> + struct tee_ioctl_invoke_arg inv_arg = {0};
> + struct tee_param param[4] = {0};
> +
> + /* Invoke TA_CMD_GET_ENTROPY function of Trusted App */
> + inv_arg.func = TA_CMD_GET_ENTROPY;
> + inv_arg.session = ta_rng_session_id;
> + inv_arg.num_params = 4;
> +
> + /* Fill invoke cmd params */
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> + param[0].u.memref.shm = entropy_shm_pool;
> + param[0].u.memref.size = req_size;
> + param[0].u.memref.shm_offs = 0;
> +
> + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> + if ((ret < 0) || (inv_arg.ret != 0)) {
> + pr_err("TA_CMD_GET_ENTROPY invoke function error: %x\n",
> + inv_arg.ret);
Why can't these be dev_err()?
> + return 0;
> + }
> +
> + rng_data = tee_shm_get_va(entropy_shm_pool, 0);
> + if (IS_ERR(rng_data)) {
> + pr_err("tee_shm_get_va failed\n");
> + return 0;
> + }
> +
> + rng_size = param[0].u.memref.size;
> + memcpy(buf, rng_data, rng_size);
> +
> + return rng_size;
> +}
> +
> +static int optee_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + u8 *data = buf;
> + size_t read = 0, rng_size = 0;
> + int timeout = 1;
> +
> + if (max > MAX_ENTROPY_REQ_SZ)
> + max = MAX_ENTROPY_REQ_SZ;
> +
> + while (read == 0) {
> + rng_size = get_optee_rng_data(data, (max - read));
> +
> + data += rng_size;
> + read += rng_size;
> +
> + if (wait) {
> + if (timeout-- == 0)
> + return read;
> + msleep((1000 * (max - read)) / ta_rng_data_rate);
> + } else {
> + return read;
> + }
> + }
> +
> + return read;
> +}
> +
> +static int optee_rng_init(struct hwrng *rng)
> +{
> + entropy_shm_pool = tee_shm_alloc(ctx, MAX_ENTROPY_REQ_SZ,
> + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> + if (IS_ERR(entropy_shm_pool)) {
> + pr_err("tee_shm_alloc failed\n");
This one is slightly annoying. It could be a dev_err() but needs quite
a bit of machinary hooked up (optee_rng_private) to allow you to look up
the device from the hwrng.
It would allow you to move all those statics (like entropy_shm_pool)
into a proper context structure though...
Daniel.
> + return PTR_ERR(entropy_shm_pool);
> + }
> +
> + return 0;
> +}
> +
> +static void optee_rng_cleanup(struct hwrng *rng)
> +{
> + tee_shm_free(entropy_shm_pool);
> +}
> +
> +static struct hwrng optee_rng = {
> + .name = DRIVER_NAME,
> + .init = optee_rng_init,
> + .cleanup = optee_rng_cleanup,
> + .read = optee_rng_read,
> +};
> +
> +static int get_optee_rng_info(struct device *dev)
> +{
> + u32 ret = 0;
> + struct tee_ioctl_invoke_arg inv_arg = {0};
> + struct tee_param param[4] = {0};
> +
> + /* Invoke TA_CMD_GET_RNG_INFO function of Trusted App */
> + inv_arg.func = TA_CMD_GET_RNG_INFO;
> + inv_arg.session = ta_rng_session_id;
> + inv_arg.num_params = 4;
> +
> + /* Fill invoke cmd params */
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> +
> + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> + if ((ret < 0) || (inv_arg.ret != 0)) {
> + dev_err(dev, "TA_CMD_GET_RNG_INFO invoke func error: %x\n",
> + inv_arg.ret);
> + return -EINVAL;
> + }
> +
> + ta_rng_data_rate = param[0].u.value.a;
> + optee_rng.quality = param[0].u.value.b;
> +
> + return 0;
> +}
> +
> +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> + if (ver->impl_id == TEE_IMPL_ID_OPTEE)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static int optee_rng_probe(struct device *dev)
> +{
> + struct tee_client_device *rng_device = to_tee_client_device(dev);
> + int ret = 0, err = -ENODEV;
> + struct tee_ioctl_open_session_arg sess_arg = {0};
> +
> + /* Open context with TEE driver */
> + ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> + if (IS_ERR(ctx))
> + return -ENODEV;
> +
> + /* Open session with hwrng Trusted App */
> + memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> + sess_arg.num_params = 0;
> +
> + ret = tee_client_open_session(ctx, &sess_arg, NULL);
> + if ((ret < 0) || (sess_arg.ret != 0)) {
> + dev_err(dev, "tee_client_open_session failed, error: %x\n",
> + sess_arg.ret);
> + err = -EINVAL;
> + goto out_ctx;
> + }
> + ta_rng_session_id = sess_arg.session;
> +
> + err = get_optee_rng_info(dev);
> + if (err)
> + goto out_sess;
> +
> + err = hwrng_register(&optee_rng);
> + if (err) {
> + dev_err(dev, "hwrng registration failed (%d)\n", err);
> + goto out_sess;
> + }
> +
> + return 0;
> +
> +out_sess:
> + tee_client_close_session(ctx, ta_rng_session_id);
> +out_ctx:
> + tee_client_close_context(ctx);
> +
> + return err;
> +}
> +
> +static int optee_rng_remove(struct device *dev)
> +{
> + hwrng_unregister(&optee_rng);
> + tee_client_close_session(ctx, ta_rng_session_id);
> + tee_client_close_context(ctx);
> +
> + return 0;
> +}
> +
> +const struct tee_client_device_id optee_rng_id_table[] = {
> + {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
> + 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(tee, optee_rng_id_table);
> +
> +static struct tee_client_driver optee_rng_driver = {
> + .id_table = optee_rng_id_table,
> + .driver = {
> + .name = DRIVER_NAME,
> + .bus = &tee_bus_type,
> + .probe = optee_rng_probe,
> + .remove = optee_rng_remove,
> + },
> +};
> +
> +static int __init optee_rng_mod_init(void)
> +{
> + int rc;
> +
> + rc = driver_register(&optee_rng_driver.driver);
> + if (rc)
> + pr_warn("driver registration failed, err: %d", rc);
> +
> + return rc;
> +}
> +
> +static void __exit optee_rng_mod_exit(void)
> +{
> + driver_unregister(&optee_rng_driver.driver);
> +}
> +
> +module_init(optee_rng_mod_init);
> +module_exit(optee_rng_mod_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Sumit Garg <sumit.garg@linaro.org>");
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 4/4] hwrng: add OP-TEE based rng driver
2019-01-24 12:30 ` Daniel Thompson
@ 2019-01-24 14:02 ` Sumit Garg
0 siblings, 0 replies; 14+ messages in thread
From: Sumit Garg @ 2019-01-24 14:02 UTC (permalink / raw)
To: Daniel Thompson
Cc: Jens Wiklander, Herbert Xu, Ard Biesheuvel, linux-arm-kernel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, yamada.masahiro, michal.lkml, mpm,
Rob Herring, Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman,
Bhupesh Sharma, tee-dev
On Thu, 24 Jan 2019 at 18:01, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Thu, Jan 24, 2019 at 11:24:39AM +0530, Sumit Garg wrote:
> > On ARM SoC's with TrustZone enabled, peripherals like entropy sources
> > might not be accessible to normal world (linux in this case) and rather
> > accessible to secure world (OP-TEE in this case) only. So this driver
> > aims to provides a generic interface to OP-TEE based random number
> > generator service.
> >
> > This driver registers on TEE bus to interact with OP-TEE based rng
> > device/service.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > MAINTAINERS | 5 +
> > drivers/char/hw_random/Kconfig | 15 ++
> > drivers/char/hw_random/Makefile | 1 +
> > drivers/char/hw_random/optee-rng.c | 274 +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 295 insertions(+)
> > create mode 100644 drivers/char/hw_random/optee-rng.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 51029a4..dcef7e9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11262,6 +11262,11 @@ M: Jens Wiklander <jens.wiklander@linaro.org>
> > S: Maintained
> > F: drivers/tee/optee/
> >
> > +OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
> > +M: Sumit Garg <sumit.garg@linaro.org>
> > +S: Maintained
> > +F: drivers/char/hw_random/optee-rng.c
> > +
> > OPA-VNIC DRIVER
> > M: Dennis Dalessandro <dennis.dalessandro@intel.com>
> > M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> > index dac895d..25a7d8f 100644
> > --- a/drivers/char/hw_random/Kconfig
> > +++ b/drivers/char/hw_random/Kconfig
> > @@ -424,6 +424,21 @@ config HW_RANDOM_EXYNOS
> > will be called exynos-trng.
> >
> > If unsure, say Y.
> > +
> > +config HW_RANDOM_OPTEE
> > + tristate "OP-TEE based Random Number Generator support"
> > + depends on OPTEE
> > + default HW_RANDOM
> > + help
> > + This driver provides support for OP-TEE based Random Number
> > + Generator on ARM SoCs where hardware entropy sources are not
> > + accessible to normal world (Linux).
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called optee-rng.
> > +
> > + If unsure, say Y.
> > +
> > endif # HW_RANDOM
> >
> > config UML_RANDOM
> > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> > index e35ec3c..7c9ef4a 100644
> > --- a/drivers/char/hw_random/Makefile
> > +++ b/drivers/char/hw_random/Makefile
> > @@ -38,3 +38,4 @@ obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> > obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
> > obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
> > obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
> > +obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> > diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
> > new file mode 100644
> > index 0000000..4ad0eca
> > --- /dev/null
> > +++ b/drivers/char/hw_random/optee-rng.c
> > @@ -0,0 +1,274 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018-2019 Linaro Ltd.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/of.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/tee_drv.h>
> > +#include <linux/uuid.h>
> > +
> > +#define DRIVER_NAME "optee-rng"
> > +
> > +#define TEE_ERROR_HEALTH_TEST_FAIL 0x00000001
> > +
> > +/*
> > + * TA_CMD_GET_ENTROPY - Get Entropy from RNG
> > + *
> > + * param[0] (inout memref) - Entropy buffer memory reference
> > + * param[1] unused
> > + * param[2] unused
> > + * param[3] unused
> > + *
> > + * Result:
> > + * TEE_SUCCESS - Invoke command success
> > + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> > + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool
> > + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
> > + */
> > +#define TA_CMD_GET_ENTROPY 0x0
> > +
> > +/*
> > + * TA_CMD_GET_RNG_INFO - Get RNG information
> > + *
> > + * param[0] (out value) - value.a: RNG data-rate in bytes per second
> > + * value.b: Quality/Entropy per 1024 bit of data
> > + * param[1] unused
> > + * param[2] unused
> > + * param[3] unused
> > + *
> > + * Result:
> > + * TEE_SUCCESS - Invoke command success
> > + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> > + */
> > +#define TA_CMD_GET_RNG_INFO 0x1
> > +
> > +#define MAX_ENTROPY_REQ_SZ (4 * 1024)
> > +
> > +static struct tee_context *ctx;
> > +static struct tee_shm *entropy_shm_pool;
> > +static u32 ta_rng_data_rate;
> > +static u32 ta_rng_session_id;
> > +
> > +static size_t get_optee_rng_data(void *buf, size_t req_size)
> > +{
> > + u32 ret = 0;
> > + u8 *rng_data = NULL;
> > + size_t rng_size = 0;
> > + struct tee_ioctl_invoke_arg inv_arg = {0};
> > + struct tee_param param[4] = {0};
> > +
> > + /* Invoke TA_CMD_GET_ENTROPY function of Trusted App */
> > + inv_arg.func = TA_CMD_GET_ENTROPY;
> > + inv_arg.session = ta_rng_session_id;
> > + inv_arg.num_params = 4;
> > +
> > + /* Fill invoke cmd params */
> > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> > + param[0].u.memref.shm = entropy_shm_pool;
> > + param[0].u.memref.size = req_size;
> > + param[0].u.memref.shm_offs = 0;
> > +
> > + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> > + if ((ret < 0) || (inv_arg.ret != 0)) {
> > + pr_err("TA_CMD_GET_ENTROPY invoke function error: %x\n",
> > + inv_arg.ret);
>
> Why can't these be dev_err()?
>
See my response below.
>
> > + return 0;
> > + }
> > +
> > + rng_data = tee_shm_get_va(entropy_shm_pool, 0);
> > + if (IS_ERR(rng_data)) {
> > + pr_err("tee_shm_get_va failed\n");
> > + return 0;
> > + }
> > +
> > + rng_size = param[0].u.memref.size;
> > + memcpy(buf, rng_data, rng_size);
> > +
> > + return rng_size;
> > +}
> > +
> > +static int optee_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> > +{
> > + u8 *data = buf;
> > + size_t read = 0, rng_size = 0;
> > + int timeout = 1;
> > +
> > + if (max > MAX_ENTROPY_REQ_SZ)
> > + max = MAX_ENTROPY_REQ_SZ;
> > +
> > + while (read == 0) {
> > + rng_size = get_optee_rng_data(data, (max - read));
> > +
> > + data += rng_size;
> > + read += rng_size;
> > +
> > + if (wait) {
> > + if (timeout-- == 0)
> > + return read;
> > + msleep((1000 * (max - read)) / ta_rng_data_rate);
> > + } else {
> > + return read;
> > + }
> > + }
> > +
> > + return read;
> > +}
> > +
> > +static int optee_rng_init(struct hwrng *rng)
> > +{
> > + entropy_shm_pool = tee_shm_alloc(ctx, MAX_ENTROPY_REQ_SZ,
> > + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> > + if (IS_ERR(entropy_shm_pool)) {
> > + pr_err("tee_shm_alloc failed\n");
>
> This one is slightly annoying. It could be a dev_err() but needs quite
> a bit of machinary hooked up (optee_rng_private) to allow you to look up
> the device from the hwrng.
>
> It would allow you to move all those statics (like entropy_shm_pool)
> into a proper context structure though...
>
Ok, will create "static struct optee_rng_private" to bundle all the
statics along with device pointer and hwrng struct. And use dev_err()
instead.
-Sumit
>
> Daniel.
>
>
>
> > + return PTR_ERR(entropy_shm_pool);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void optee_rng_cleanup(struct hwrng *rng)
> > +{
> > + tee_shm_free(entropy_shm_pool);
> > +}
> > +
> > +static struct hwrng optee_rng = {
> > + .name = DRIVER_NAME,
> > + .init = optee_rng_init,
> > + .cleanup = optee_rng_cleanup,
> > + .read = optee_rng_read,
> > +};
> > +
> > +static int get_optee_rng_info(struct device *dev)
> > +{
> > + u32 ret = 0;
> > + struct tee_ioctl_invoke_arg inv_arg = {0};
> > + struct tee_param param[4] = {0};
> > +
> > + /* Invoke TA_CMD_GET_RNG_INFO function of Trusted App */
> > + inv_arg.func = TA_CMD_GET_RNG_INFO;
> > + inv_arg.session = ta_rng_session_id;
> > + inv_arg.num_params = 4;
> > +
> > + /* Fill invoke cmd params */
> > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> > +
> > + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> > + if ((ret < 0) || (inv_arg.ret != 0)) {
> > + dev_err(dev, "TA_CMD_GET_RNG_INFO invoke func error: %x\n",
> > + inv_arg.ret);
> > + return -EINVAL;
> > + }
> > +
> > + ta_rng_data_rate = param[0].u.value.a;
> > + optee_rng.quality = param[0].u.value.b;
> > +
> > + return 0;
> > +}
> > +
> > +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> > +{
> > + if (ver->impl_id == TEE_IMPL_ID_OPTEE)
> > + return 1;
> > + else
> > + return 0;
> > +}
> > +
> > +static int optee_rng_probe(struct device *dev)
> > +{
> > + struct tee_client_device *rng_device = to_tee_client_device(dev);
> > + int ret = 0, err = -ENODEV;
> > + struct tee_ioctl_open_session_arg sess_arg = {0};
> > +
> > + /* Open context with TEE driver */
> > + ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> > + if (IS_ERR(ctx))
> > + return -ENODEV;
> > +
> > + /* Open session with hwrng Trusted App */
> > + memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> > + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> > + sess_arg.num_params = 0;
> > +
> > + ret = tee_client_open_session(ctx, &sess_arg, NULL);
> > + if ((ret < 0) || (sess_arg.ret != 0)) {
> > + dev_err(dev, "tee_client_open_session failed, error: %x\n",
> > + sess_arg.ret);
> > + err = -EINVAL;
> > + goto out_ctx;
> > + }
> > + ta_rng_session_id = sess_arg.session;
> > +
> > + err = get_optee_rng_info(dev);
> > + if (err)
> > + goto out_sess;
> > +
> > + err = hwrng_register(&optee_rng);
> > + if (err) {
> > + dev_err(dev, "hwrng registration failed (%d)\n", err);
> > + goto out_sess;
> > + }
> > +
> > + return 0;
> > +
> > +out_sess:
> > + tee_client_close_session(ctx, ta_rng_session_id);
> > +out_ctx:
> > + tee_client_close_context(ctx);
> > +
> > + return err;
> > +}
> > +
> > +static int optee_rng_remove(struct device *dev)
> > +{
> > + hwrng_unregister(&optee_rng);
> > + tee_client_close_session(ctx, ta_rng_session_id);
> > + tee_client_close_context(ctx);
> > +
> > + return 0;
> > +}
> > +
> > +const struct tee_client_device_id optee_rng_id_table[] = {
> > + {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
> > + 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(tee, optee_rng_id_table);
> > +
> > +static struct tee_client_driver optee_rng_driver = {
> > + .id_table = optee_rng_id_table,
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .bus = &tee_bus_type,
> > + .probe = optee_rng_probe,
> > + .remove = optee_rng_remove,
> > + },
> > +};
> > +
> > +static int __init optee_rng_mod_init(void)
> > +{
> > + int rc;
> > +
> > + rc = driver_register(&optee_rng_driver.driver);
> > + if (rc)
> > + pr_warn("driver registration failed, err: %d", rc);
> > +
> > + return rc;
> > +}
> > +
> > +static void __exit optee_rng_mod_exit(void)
> > +{
> > + driver_unregister(&optee_rng_driver.driver);
> > +}
> > +
> > +module_init(optee_rng_mod_init);
> > +module_exit(optee_rng_mod_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Sumit Garg <sumit.garg@linaro.org>");
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 4/4] hwrng: add OP-TEE based rng driver
2019-01-24 5:54 ` [PATCH v5 4/4] hwrng: add OP-TEE based rng driver Sumit Garg
2019-01-24 12:30 ` Daniel Thompson
@ 2019-01-28 4:30 ` Sumit Garg
1 sibling, 0 replies; 14+ messages in thread
From: Sumit Garg @ 2019-01-28 4:30 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Wiklander, Ard Biesheuvel, linux-arm-kernel,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, yamada.masahiro, michal.lkml, mpm,
Rob Herring, Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman,
Daniel Thompson, Bhupesh Sharma, tee-dev
Hi Herbert,
On Thu, 24 Jan 2019 at 11:25, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> On ARM SoC's with TrustZone enabled, peripherals like entropy sources
> might not be accessible to normal world (linux in this case) and rather
> accessible to secure world (OP-TEE in this case) only. So this driver
> aims to provides a generic interface to OP-TEE based random number
> generator service.
>
> This driver registers on TEE bus to interact with OP-TEE based rng
> device/service.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Do you have any comments/feedback on this patch before I send next version (v6)?
-Sumit
> ---
> MAINTAINERS | 5 +
> drivers/char/hw_random/Kconfig | 15 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/optee-rng.c | 274 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 295 insertions(+)
> create mode 100644 drivers/char/hw_random/optee-rng.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 51029a4..dcef7e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11262,6 +11262,11 @@ M: Jens Wiklander <jens.wiklander@linaro.org>
> S: Maintained
> F: drivers/tee/optee/
>
> +OP-TEE RANDOM NUMBER GENERATOR (RNG) DRIVER
> +M: Sumit Garg <sumit.garg@linaro.org>
> +S: Maintained
> +F: drivers/char/hw_random/optee-rng.c
> +
> OPA-VNIC DRIVER
> M: Dennis Dalessandro <dennis.dalessandro@intel.com>
> M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index dac895d..25a7d8f 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -424,6 +424,21 @@ config HW_RANDOM_EXYNOS
> will be called exynos-trng.
>
> If unsure, say Y.
> +
> +config HW_RANDOM_OPTEE
> + tristate "OP-TEE based Random Number Generator support"
> + depends on OPTEE
> + default HW_RANDOM
> + help
> + This driver provides support for OP-TEE based Random Number
> + Generator on ARM SoCs where hardware entropy sources are not
> + accessible to normal world (Linux).
> +
> + To compile this driver as a module, choose M here: the module
> + will be called optee-rng.
> +
> + If unsure, say Y.
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index e35ec3c..7c9ef4a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -38,3 +38,4 @@ obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> obj-$(CONFIG_HW_RANDOM_MTK) += mtk-rng.o
> obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
> obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
> +obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
> new file mode 100644
> index 0000000..4ad0eca
> --- /dev/null
> +++ b/drivers/char/hw_random/optee-rng.c
> @@ -0,0 +1,274 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018-2019 Linaro Ltd.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/hw_random.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/uuid.h>
> +
> +#define DRIVER_NAME "optee-rng"
> +
> +#define TEE_ERROR_HEALTH_TEST_FAIL 0x00000001
> +
> +/*
> + * TA_CMD_GET_ENTROPY - Get Entropy from RNG
> + *
> + * param[0] (inout memref) - Entropy buffer memory reference
> + * param[1] unused
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool
> + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
> + */
> +#define TA_CMD_GET_ENTROPY 0x0
> +
> +/*
> + * TA_CMD_GET_RNG_INFO - Get RNG information
> + *
> + * param[0] (out value) - value.a: RNG data-rate in bytes per second
> + * value.b: Quality/Entropy per 1024 bit of data
> + * param[1] unused
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + */
> +#define TA_CMD_GET_RNG_INFO 0x1
> +
> +#define MAX_ENTROPY_REQ_SZ (4 * 1024)
> +
> +static struct tee_context *ctx;
> +static struct tee_shm *entropy_shm_pool;
> +static u32 ta_rng_data_rate;
> +static u32 ta_rng_session_id;
> +
> +static size_t get_optee_rng_data(void *buf, size_t req_size)
> +{
> + u32 ret = 0;
> + u8 *rng_data = NULL;
> + size_t rng_size = 0;
> + struct tee_ioctl_invoke_arg inv_arg = {0};
> + struct tee_param param[4] = {0};
> +
> + /* Invoke TA_CMD_GET_ENTROPY function of Trusted App */
> + inv_arg.func = TA_CMD_GET_ENTROPY;
> + inv_arg.session = ta_rng_session_id;
> + inv_arg.num_params = 4;
> +
> + /* Fill invoke cmd params */
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> + param[0].u.memref.shm = entropy_shm_pool;
> + param[0].u.memref.size = req_size;
> + param[0].u.memref.shm_offs = 0;
> +
> + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> + if ((ret < 0) || (inv_arg.ret != 0)) {
> + pr_err("TA_CMD_GET_ENTROPY invoke function error: %x\n",
> + inv_arg.ret);
> + return 0;
> + }
> +
> + rng_data = tee_shm_get_va(entropy_shm_pool, 0);
> + if (IS_ERR(rng_data)) {
> + pr_err("tee_shm_get_va failed\n");
> + return 0;
> + }
> +
> + rng_size = param[0].u.memref.size;
> + memcpy(buf, rng_data, rng_size);
> +
> + return rng_size;
> +}
> +
> +static int optee_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + u8 *data = buf;
> + size_t read = 0, rng_size = 0;
> + int timeout = 1;
> +
> + if (max > MAX_ENTROPY_REQ_SZ)
> + max = MAX_ENTROPY_REQ_SZ;
> +
> + while (read == 0) {
> + rng_size = get_optee_rng_data(data, (max - read));
> +
> + data += rng_size;
> + read += rng_size;
> +
> + if (wait) {
> + if (timeout-- == 0)
> + return read;
> + msleep((1000 * (max - read)) / ta_rng_data_rate);
> + } else {
> + return read;
> + }
> + }
> +
> + return read;
> +}
> +
> +static int optee_rng_init(struct hwrng *rng)
> +{
> + entropy_shm_pool = tee_shm_alloc(ctx, MAX_ENTROPY_REQ_SZ,
> + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> + if (IS_ERR(entropy_shm_pool)) {
> + pr_err("tee_shm_alloc failed\n");
> + return PTR_ERR(entropy_shm_pool);
> + }
> +
> + return 0;
> +}
> +
> +static void optee_rng_cleanup(struct hwrng *rng)
> +{
> + tee_shm_free(entropy_shm_pool);
> +}
> +
> +static struct hwrng optee_rng = {
> + .name = DRIVER_NAME,
> + .init = optee_rng_init,
> + .cleanup = optee_rng_cleanup,
> + .read = optee_rng_read,
> +};
> +
> +static int get_optee_rng_info(struct device *dev)
> +{
> + u32 ret = 0;
> + struct tee_ioctl_invoke_arg inv_arg = {0};
> + struct tee_param param[4] = {0};
> +
> + /* Invoke TA_CMD_GET_RNG_INFO function of Trusted App */
> + inv_arg.func = TA_CMD_GET_RNG_INFO;
> + inv_arg.session = ta_rng_session_id;
> + inv_arg.num_params = 4;
> +
> + /* Fill invoke cmd params */
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> +
> + ret = tee_client_invoke_func(ctx, &inv_arg, param);
> + if ((ret < 0) || (inv_arg.ret != 0)) {
> + dev_err(dev, "TA_CMD_GET_RNG_INFO invoke func error: %x\n",
> + inv_arg.ret);
> + return -EINVAL;
> + }
> +
> + ta_rng_data_rate = param[0].u.value.a;
> + optee_rng.quality = param[0].u.value.b;
> +
> + return 0;
> +}
> +
> +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> + if (ver->impl_id == TEE_IMPL_ID_OPTEE)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static int optee_rng_probe(struct device *dev)
> +{
> + struct tee_client_device *rng_device = to_tee_client_device(dev);
> + int ret = 0, err = -ENODEV;
> + struct tee_ioctl_open_session_arg sess_arg = {0};
> +
> + /* Open context with TEE driver */
> + ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> + if (IS_ERR(ctx))
> + return -ENODEV;
> +
> + /* Open session with hwrng Trusted App */
> + memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> + sess_arg.num_params = 0;
> +
> + ret = tee_client_open_session(ctx, &sess_arg, NULL);
> + if ((ret < 0) || (sess_arg.ret != 0)) {
> + dev_err(dev, "tee_client_open_session failed, error: %x\n",
> + sess_arg.ret);
> + err = -EINVAL;
> + goto out_ctx;
> + }
> + ta_rng_session_id = sess_arg.session;
> +
> + err = get_optee_rng_info(dev);
> + if (err)
> + goto out_sess;
> +
> + err = hwrng_register(&optee_rng);
> + if (err) {
> + dev_err(dev, "hwrng registration failed (%d)\n", err);
> + goto out_sess;
> + }
> +
> + return 0;
> +
> +out_sess:
> + tee_client_close_session(ctx, ta_rng_session_id);
> +out_ctx:
> + tee_client_close_context(ctx);
> +
> + return err;
> +}
> +
> +static int optee_rng_remove(struct device *dev)
> +{
> + hwrng_unregister(&optee_rng);
> + tee_client_close_session(ctx, ta_rng_session_id);
> + tee_client_close_context(ctx);
> +
> + return 0;
> +}
> +
> +const struct tee_client_device_id optee_rng_id_table[] = {
> + {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
> + 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(tee, optee_rng_id_table);
> +
> +static struct tee_client_driver optee_rng_driver = {
> + .id_table = optee_rng_id_table,
> + .driver = {
> + .name = DRIVER_NAME,
> + .bus = &tee_bus_type,
> + .probe = optee_rng_probe,
> + .remove = optee_rng_remove,
> + },
> +};
> +
> +static int __init optee_rng_mod_init(void)
> +{
> + int rc;
> +
> + rc = driver_register(&optee_rng_driver.driver);
> + if (rc)
> + pr_warn("driver registration failed, err: %d", rc);
> +
> + return rc;
> +}
> +
> +static void __exit optee_rng_mod_exit(void)
> +{
> + driver_unregister(&optee_rng_driver.driver);
> +}
> +
> +module_init(optee_rng_mod_init);
> +module_exit(optee_rng_mod_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Sumit Garg <sumit.garg@linaro.org>");
> +MODULE_DESCRIPTION("OP-TEE based random number generator driver");
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-01-28 4:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 5:54 [PATCH v5 0/4] Introduce TEE bus driver framework Sumit Garg
2019-01-24 5:54 ` [PATCH v5 1/4] tee: add bus driver framework for TEE based devices Sumit Garg
2019-01-24 9:45 ` Daniel Thompson
2019-01-24 10:19 ` Sumit Garg
2019-01-24 10:49 ` Bhupesh Sharma
2019-01-24 5:54 ` [PATCH v5 2/4] tee: add supp_nowait flag in tee_context struct Sumit Garg
2019-01-24 9:48 ` Daniel Thompson
2019-01-24 5:54 ` [PATCH v5 3/4] tee: optee: add TEE bus device enumeration support Sumit Garg
2019-01-24 10:15 ` Daniel Thompson
2019-01-24 11:28 ` Sumit Garg
2019-01-24 5:54 ` [PATCH v5 4/4] hwrng: add OP-TEE based rng driver Sumit Garg
2019-01-24 12:30 ` Daniel Thompson
2019-01-24 14:02 ` Sumit Garg
2019-01-28 4:30 ` Sumit Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).