From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Add a minimal optee_version_info (major/minor/build) and get_optee_revision hook, collect the OS revision in both SMC and FF-A ABIs, and publish /sys/class/tee/tee*/optee_os_revision for a stable userspace readout.
Signed-off-by: Aristo Chen aristo.chen@canonical.com --- drivers/tee/optee/ffa_abi.c | 27 +++++++++++- drivers/tee/optee/optee_private.h | 1 + drivers/tee/optee/smc_abi.c | 27 +++++++++++- drivers/tee/tee_core.c | 73 ++++++++++++++++++++++++++++++- include/linux/tee_core.h | 2 + include/linux/tee_drv.h | 12 +++++ 6 files changed, 137 insertions(+), 5 deletions(-)
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..291ba3bfde7f 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, - const struct ffa_ops *ops) + const struct ffa_ops *ops, + struct optee_version_info *version_info) { const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; } + if (version_info) { + version_info->os_major = data.data0; + version_info->os_minor = data.data1; + version_info->os_build_id = data.data2; + } + if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2); @@ -893,6 +900,18 @@ static void optee_ffa_get_version(struct tee_device *teedev, *vers = v; }
+static int optee_ffa_get_optee_revision(struct tee_device *teedev, + struct optee_version_info *vers) +{ + struct optee *optee = tee_get_drvdata(teedev); + + if (!optee) + return -ENODEV; + + *vers = optee->version_info; + return 0; +} + static int optee_ffa_open(struct tee_context *ctx) { return optee_open(ctx, true); @@ -900,6 +919,7 @@ static int optee_ffa_open(struct tee_context *ctx)
static const struct tee_driver_ops optee_ffa_clnt_ops = { .get_version = optee_ffa_get_version, + .get_optee_revision = optee_ffa_get_optee_revision, .open = optee_ffa_open, .release = optee_release, .open_session = optee_open_session, @@ -918,6 +938,7 @@ static const struct tee_desc optee_ffa_clnt_desc = {
static const struct tee_driver_ops optee_ffa_supp_ops = { .get_version = optee_ffa_get_version, + .get_optee_revision = optee_ffa_get_optee_revision, .open = optee_ffa_open, .release = optee_release_supp, .supp_recv = optee_supp_recv, @@ -1034,6 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops; + struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool; @@ -1047,7 +1069,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
- if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops)) + if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL;
if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps, @@ -1059,6 +1081,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM; + optee->version_info = version_info;
pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) { diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..967c015e8ffb 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -249,6 +249,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work; + struct optee_version_info version_info; };
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..1e412949898f 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1232,6 +1232,18 @@ static void optee_get_version(struct tee_device *teedev, *vers = v; }
+static int optee_get_optee_revision(struct tee_device *teedev, + struct optee_version_info *vers) +{ + struct optee *optee = tee_get_drvdata(teedev); + + if (!optee) + return -ENODEV; + + *vers = optee->version_info; + return 0; +} + static int optee_smc_open(struct tee_context *ctx) { struct optee *optee = tee_get_drvdata(ctx->teedev); @@ -1242,6 +1254,7 @@ static int optee_smc_open(struct tee_context *ctx)
static const struct tee_driver_ops optee_clnt_ops = { .get_version = optee_get_version, + .get_optee_revision = optee_get_optee_revision, .open = optee_smc_open, .release = optee_release, .open_session = optee_open_session, @@ -1261,6 +1274,7 @@ static const struct tee_desc optee_clnt_desc = {
static const struct tee_driver_ops optee_supp_ops = { .get_version = optee_get_version, + .get_optee_revision = optee_get_optee_revision, .open = optee_smc_open, .release = optee_release_supp, .supp_recv = optee_supp_recv, @@ -1323,7 +1337,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn, + struct optee_version_info *version_info) { union { struct arm_smccc_res smccc; @@ -1337,6 +1352,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
+ if (version_info) { + version_info->os_major = res.result.major; + version_info->os_minor = res.result.minor; + version_info->os_build_id = res.result.build_id; + } + if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2, @@ -1727,6 +1748,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx; + struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps; @@ -1745,7 +1767,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
- optee_msg_get_os_revision(invoke_fn); + optee_msg_get_os_revision(invoke_fn, &version_info);
if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n"); @@ -1814,6 +1836,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; } + optee->version_info = version_info;
optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn; diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index d65d47cc154e..dc23058e7be6 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -1141,12 +1141,83 @@ static ssize_t implementation_id_show(struct device *dev, } static DEVICE_ATTR_RO(implementation_id);
+static int tee_get_optee_revision(struct tee_device *teedev, + struct optee_version_info *ver_info) +{ + if (!teedev->desc->ops->get_optee_revision) + return -ENODEV; + + return teedev->desc->ops->get_optee_revision(teedev, ver_info); +} + +static bool tee_is_optee(struct tee_device *teedev) +{ + struct tee_ioctl_version_data vers; + + teedev->desc->ops->get_version(teedev, &vers); + + return vers.impl_id == TEE_IMPL_ID_OPTEE; +} + +static ssize_t optee_os_revision_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct tee_device *teedev = container_of(dev, struct tee_device, dev); + struct optee_version_info ver_info; + int ret; + + if (!tee_is_optee(teedev)) + return -ENODEV; + + ret = tee_get_optee_revision(teedev, &ver_info); + if (ret) + return ret; + + if (ver_info.os_build_id) + return sysfs_emit(buf, "%u.%u (%08x)\n", ver_info.os_major, + ver_info.os_minor, ver_info.os_build_id); + + return sysfs_emit(buf, "%u.%u\n", ver_info.os_major, + ver_info.os_minor); +} +static DEVICE_ATTR_RO(optee_os_revision); + static struct attribute *tee_dev_attrs[] = { &dev_attr_implementation_id.attr, NULL };
-ATTRIBUTE_GROUPS(tee_dev); +static struct attribute *tee_optee_attrs[] = { + &dev_attr_optee_os_revision.attr, + NULL +}; + +static umode_t tee_optee_attr_is_visible(struct kobject *kobj, + struct attribute *attr, int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct tee_device *teedev = container_of(dev, struct tee_device, dev); + + if (tee_is_optee(teedev) && teedev->desc->ops->get_optee_revision) + return attr->mode; + + return 0; +} + +static const struct attribute_group tee_dev_group = { + .attrs = tee_dev_attrs, +}; + +static const struct attribute_group tee_optee_group = { + .attrs = tee_optee_attrs, + .is_visible = tee_optee_attr_is_visible, +}; + +static const struct attribute_group *tee_dev_groups[] = { + &tee_dev_group, + &tee_optee_group, + NULL +};
static const struct class tee_class = { .name = "tee", diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h index 1f3e5dad6d0d..4bd9b6b191c9 100644 --- a/include/linux/tee_core.h +++ b/include/linux/tee_core.h @@ -98,6 +98,8 @@ struct tee_device { struct tee_driver_ops { void (*get_version)(struct tee_device *teedev, struct tee_ioctl_version_data *vers); + int (*get_optee_revision)(struct tee_device *teedev, + struct optee_version_info *vers); int (*open)(struct tee_context *ctx); void (*close_context)(struct tee_context *ctx); void (*release)(struct tee_context *ctx); diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 88a6f9697c89..64a2fea11cb9 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -20,6 +20,18 @@
struct tee_device;
+/** + * struct optee_version_info - OP-TEE version information + * @os_major: OS major version + * @os_minor: OS minor version + * @os_build_id: OS build identifier (0 if unspecified) + */ +struct optee_version_info { + u32 os_major; + u32 os_minor; + u32 os_build_id; +}; + /** * struct tee_context - driver specific context on file pointer data * @teedev: pointer to this drivers struct tee_device
Hi Aristo,
On 11/21/2025 7:42 PM, Wei Ming Chen wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Add a minimal optee_version_info (major/minor/build) and get_optee_revision hook, collect the OS revision in both SMC and FF-A ABIs, and publish /sys/class/tee/tee*/optee_os_revision for a stable userspace readout.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/ffa_abi.c | 27 +++++++++++- drivers/tee/optee/optee_private.h | 1 + drivers/tee/optee/smc_abi.c | 27 +++++++++++- drivers/tee/tee_core.c | 73 ++++++++++++++++++++++++++++++- include/linux/tee_core.h | 2 + include/linux/tee_drv.h | 12 +++++ 6 files changed, 137 insertions(+), 5 deletions(-)
[..]
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index d65d47cc154e..dc23058e7be6 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -1141,12 +1141,83 @@ static ssize_t implementation_id_show(struct device *dev, } static DEVICE_ATTR_RO(implementation_id); +static int tee_get_optee_revision(struct tee_device *teedev,
struct optee_version_info *ver_info)+{
- if (!teedev->desc->ops->get_optee_revision)
return -ENODEV;- return teedev->desc->ops->get_optee_revision(teedev, ver_info);
+}
I don't think implementing functions with a TEE back-end driver specific name and implementation is a good idea in TEE core.
+static bool tee_is_optee(struct tee_device *teedev) +{
- struct tee_ioctl_version_data vers;
- teedev->desc->ops->get_version(teedev, &vers);
- return vers.impl_id == TEE_IMPL_ID_OPTEE;
+}
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
- struct tee_device *teedev = container_of(dev, struct tee_device, dev);
- struct optee_version_info ver_info;
- int ret;
- if (!tee_is_optee(teedev))
return -ENODEV;- ret = tee_get_optee_revision(teedev, &ver_info);
- if (ret)
return ret;- if (ver_info.os_build_id)
return sysfs_emit(buf, "%u.%u (%08x)\n", ver_info.os_major,ver_info.os_minor, ver_info.os_build_id);- return sysfs_emit(buf, "%u.%u\n", ver_info.os_major,
ver_info.os_minor);+}
[..]
diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h index 1f3e5dad6d0d..4bd9b6b191c9 100644 --- a/include/linux/tee_core.h +++ b/include/linux/tee_core.h @@ -98,6 +98,8 @@ struct tee_device { struct tee_driver_ops { void (*get_version)(struct tee_device *teedev, struct tee_ioctl_version_data *vers);
- int (*get_optee_revision)(struct tee_device *teedev,
struct optee_version_info *vers);
I think it would be better if this patchset could be made OPTEE agnostic. After-all, the callbacks defined in tee_driver_ops are supposed to be implemented by each TEE back-end driver as per their implementation.
int (*open)(struct tee_context *ctx); void (*close_context)(struct tee_context *ctx); void (*release)(struct tee_context *ctx); diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 88a6f9697c89..64a2fea11cb9 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -20,6 +20,18 @@ struct tee_device; +/**
- struct optee_version_info - OP-TEE version information
- @os_major: OS major version
- @os_minor: OS minor version
- @os_build_id: OS build identifier (0 if unspecified)
- */
+struct optee_version_info {
- u32 os_major;
- u32 os_minor;
- u32 os_build_id;
+};
It is not entirely clear from the structure what kind of version information for OPTEE is being provided. I see from the implementation that when FFA is enabled, this provides FFA version implemented by OPTEE but when FFA is not used, it provides OPTEE OS version. This can be confusing for a user trying to report an OPTEE 'version'.
/**
- struct tee_context - driver specific context on file pointer data
- @teedev: pointer to this drivers struct tee_device
Thanks Harshal
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com --- drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct optee *optee = dev_get_drvdata(dev); + struct optee_version_info *v; + + if (!optee) + return -ENODEV; + + v = &optee->version_info; + if (v->os_build_id) + return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major, + v->os_minor, (unsigned long long)v->os_build_id); + + return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor); +} +static DEVICE_ATTR_RO(optee_os_revision); + static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr, + &dev_attr_optee_os_revision.attr, NULL };
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, - const struct ffa_ops *ops) + const struct ffa_ops *ops, + struct optee_version_info *version_info) { const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; } + if (version_info) { + version_info->os_major = data.data0; + version_info->os_minor = data.data1; + version_info->os_build_id = data.data2; + } + if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2); @@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops; + struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool; @@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
- if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops)) + if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL;
if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps, @@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM; + optee->version_info = version_info;
pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) { diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/** + * struct optee_version_info - OP-TEE OS revision reported by secure world + * @os_major: OP-TEE OS major version + * @os_minor: OP-TEE OS minor version + * @os_build_id: OP-TEE OS build identifier (0 if unspecified) + * + * Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or + * OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an + * FF-A ABI version. + */ +struct optee_version_info { + u32 os_major; + u32 os_minor; + u64 os_build_id; +}; + /* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work; + struct optee_version_info version_info; };
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn, + struct optee_version_info *version_info) { union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
+ if (version_info) { + version_info->os_major = res.result.major; + version_info->os_minor = res.result.minor; + version_info->os_build_id = res.result.build_id; + } + if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2, @@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx; + struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps; @@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
- optee_msg_get_os_revision(invoke_fn); + optee_msg_get_os_revision(invoke_fn, &version_info);
if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n"); @@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; } + optee->version_info = version_info;
optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs. If so, let's take a step back to see what's needed. We can add the sysfs attribute from drivers/tee/tee_core.c and add another optional callback to struct tee_driver_ops, such as get_tee_revision(). For OP-TEE, the specific information we want to share is clear, but other TEEs might have additional or different data. What do others think?
Cheers, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
struct optee *optee = dev_get_drvdata(dev);struct optee_version_info *v;if (!optee)return -ENODEV;v = &optee->version_info;if (v->os_build_id)return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major,v->os_minor, (unsigned long long)v->os_build_id);return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor);+} +static DEVICE_ATTR_RO(optee_os_revision);
static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr,
&dev_attr_optee_os_revision.attr, NULL};
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
const struct ffa_ops *ops)
const struct ffa_ops *ops,struct optee_version_info *version_info){ const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; }
if (version_info) {version_info->os_major = data.data0;version_info->os_minor = data.data1;version_info->os_build_id = data.data2;}if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2);@@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops;
struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool;@@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops))
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL; if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps,@@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM;
optee->version_info = version_info; pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) {diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/**
- struct optee_version_info - OP-TEE OS revision reported by secure world
- @os_major: OP-TEE OS major version
- @os_minor: OP-TEE OS minor version
- @os_build_id: OP-TEE OS build identifier (0 if unspecified)
- Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- FF-A ABI version.
- */
+struct optee_version_info {
u32 os_major;u32 os_minor;u64 os_build_id;+};
/* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work;
struct optee_version_info version_info;};
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
struct optee_version_info *version_info){ union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
if (version_info) {version_info->os_major = res.result.major;version_info->os_minor = res.result.minor;version_info->os_build_id = res.result.build_id;}if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2,@@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx;
struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps;@@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
optee_msg_get_os_revision(invoke_fn);
optee_msg_get_os_revision(invoke_fn, &version_info); if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n");@@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; }
optee->version_info = version_info; optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;-- 2.43.0
[AMD Official Use Only - AMD Internal Distribution Only]
+Deva,
I am on sick leave at present.
Thanks, Rijo
Get Outlook for iOShttps://aka.ms/o0ukef ________________________________ From: Jens Wiklander jens.wiklander@linaro.org Sent: Monday, November 24, 2025 12:45 PM To: Wei Ming Chen jj251510319013@gmail.com Cc: linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org; sumit.garg@kernel.org sumit.garg@kernel.org; op-tee@lists.trustedfirmware.org op-tee@lists.trustedfirmware.org; harshal.dev@oss.qualcomm.com harshal.dev@oss.qualcomm.com; Aristo Chen aristo.chen@canonical.com; Limonciello, Mario Mario.Limonciello@amd.com; Balint Dobszay balint.dobszay@arm.com; Thomas, Rijo-john Rijo-john.Thomas@amd.com; Amirreza Zarrabi amirreza.zarrabi@oss.qualcomm.com Subject: Re: [PATCH v2 1/1] tee: optee: expose OS revision via sysfs
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs. If so, let's take a step back to see what's needed. We can add the sysfs attribute from drivers/tee/tee_core.c and add another optional callback to struct tee_driver_ops, such as get_tee_revision(). For OP-TEE, the specific information we want to share is clear, but other TEEs might have additional or different data. What do others think?
Cheers, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
struct optee *optee = dev_get_drvdata(dev);struct optee_version_info *v;if (!optee)return -ENODEV;v = &optee->version_info;if (v->os_build_id)return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major,v->os_minor, (unsigned long long)v->os_build_id);return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor);+} +static DEVICE_ATTR_RO(optee_os_revision);
static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr,
&dev_attr_optee_os_revision.attr, NULL};
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
const struct ffa_ops *ops)
const struct ffa_ops *ops,struct optee_version_info *version_info){ const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; }
if (version_info) {version_info->os_major = data.data0;version_info->os_minor = data.data1;version_info->os_build_id = data.data2;}if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2);@@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops;
struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool;@@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops))
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL; if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps,@@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM;
optee->version_info = version_info; pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) {diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/**
- struct optee_version_info - OP-TEE OS revision reported by secure world
- @os_major: OP-TEE OS major version
- @os_minor: OP-TEE OS minor version
- @os_build_id: OP-TEE OS build identifier (0 if unspecified)
- Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- FF-A ABI version.
- */
+struct optee_version_info {
u32 os_major;u32 os_minor;u64 os_build_id;+};
/* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work;
struct optee_version_info version_info;};
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
struct optee_version_info *version_info){ union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
if (version_info) {version_info->os_major = res.result.major;version_info->os_minor = res.result.minor;version_info->os_build_id = res.result.build_id;}if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2,@@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx;
struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps;@@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
optee_msg_get_os_revision(invoke_fn);
optee_msg_get_os_revision(invoke_fn, &version_info); if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n");@@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; }
optee->version_info = version_info; optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;-- 2.43.0
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
If so, let's take a step back to see what's needed. We can add the sysfs attribute from drivers/tee/tee_core.c and add another optional callback to struct tee_driver_ops, such as get_tee_revision(). For OP-TEE, the specific information we want to share is clear, but other TEEs might have additional or different data. What do others think?
Implementation wise, this sounds like a reasonable approach.
-Sumit
Cheers, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
struct optee *optee = dev_get_drvdata(dev);struct optee_version_info *v;if (!optee)return -ENODEV;v = &optee->version_info;if (v->os_build_id)return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major,v->os_minor, (unsigned long long)v->os_build_id);return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor);+} +static DEVICE_ATTR_RO(optee_os_revision);
static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr,
&dev_attr_optee_os_revision.attr, NULL};
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
const struct ffa_ops *ops)
const struct ffa_ops *ops,struct optee_version_info *version_info){ const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; }
if (version_info) {version_info->os_major = data.data0;version_info->os_minor = data.data1;version_info->os_build_id = data.data2;}if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2);@@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops;
struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool;@@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops))
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL; if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps,@@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM;
optee->version_info = version_info; pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) {diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/**
- struct optee_version_info - OP-TEE OS revision reported by secure world
- @os_major: OP-TEE OS major version
- @os_minor: OP-TEE OS minor version
- @os_build_id: OP-TEE OS build identifier (0 if unspecified)
- Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- FF-A ABI version.
- */
+struct optee_version_info {
u32 os_major;u32 os_minor;u64 os_build_id;+};
/* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work;
struct optee_version_info version_info;};
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
struct optee_version_info *version_info){ union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
if (version_info) {version_info->os_major = res.result.major;version_info->os_minor = res.result.minor;version_info->os_build_id = res.result.build_id;}if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2,@@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx;
struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps;@@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
optee_msg_get_os_revision(invoke_fn);
optee_msg_get_os_revision(invoke_fn, &version_info); if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n");@@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; }
optee->version_info = version_info; optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;-- 2.43.0
On Tue, Nov 25, 2025 at 01:23:22PM +0530, Sumit Garg via OP-TEE wrote:
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
Ah, typo here:
s/TEE_IOC_REVERSION/TEE_IOC_REVISION/
-Sumit
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
If so, let's take a step back to see what's needed. We can add the sysfs attribute from drivers/tee/tee_core.c and add another optional callback to struct tee_driver_ops, such as get_tee_revision(). For OP-TEE, the specific information we want to share is clear, but other TEEs might have additional or different data. What do others think?
Implementation wise, this sounds like a reasonable approach.
-Sumit
Cheers, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
struct optee *optee = dev_get_drvdata(dev);struct optee_version_info *v;if (!optee)return -ENODEV;v = &optee->version_info;if (v->os_build_id)return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major,v->os_minor, (unsigned long long)v->os_build_id);return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor);+} +static DEVICE_ATTR_RO(optee_os_revision);
static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr,
&dev_attr_optee_os_revision.attr, NULL};
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
const struct ffa_ops *ops)
const struct ffa_ops *ops,struct optee_version_info *version_info){ const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; }
if (version_info) {version_info->os_major = data.data0;version_info->os_minor = data.data1;version_info->os_build_id = data.data2;}if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2);@@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops;
struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool;@@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops))
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL; if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps,@@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM;
optee->version_info = version_info; pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) {diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/**
- struct optee_version_info - OP-TEE OS revision reported by secure world
- @os_major: OP-TEE OS major version
- @os_minor: OP-TEE OS minor version
- @os_build_id: OP-TEE OS build identifier (0 if unspecified)
- Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- FF-A ABI version.
- */
+struct optee_version_info {
u32 os_major;u32 os_minor;u64 os_build_id;+};
/* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work;
struct optee_version_info version_info;};
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
struct optee_version_info *version_info){ union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
if (version_info) {version_info->os_major = res.result.major;version_info->os_minor = res.result.minor;version_info->os_build_id = res.result.build_id;}if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2,@@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx;
struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps;@@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
optee_msg_get_os_revision(invoke_fn);
optee_msg_get_os_revision(invoke_fn, &version_info); if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n");@@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; }
optee->version_info = version_info; optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;-- 2.43.0
Hi,
Sumit Garg sumit.garg@kernel.org 於 2025年11月25日週二 下午3:55寫道：
On Tue, Nov 25, 2025 at 01:23:22PM +0530, Sumit Garg via OP-TEE wrote:
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
Ah, typo here:
s/TEE_IOC_REVERSION/TEE_IOC_REVISION/
-Sumit
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
My current thinking is that if the TEE revision is exposed, users can write a script to capture the platform state and record the exact secure OS revision even after the dmesg/journalctl logs have rolled over. This would significantly improve bug triage and regression tracking.
In my case, I have a package with precompiled xtest binaries for multiple releases (from 3.14 to 4.6), and I work with different platforms that run different OP-TEE OS versions. Having a reliable way to obtain the TEE revision would help a lot, as it would allow me to select the correct xtest version when running tests.
If so, let's take a step back to see what's needed. We can add the sysfs attribute from drivers/tee/tee_core.c and add another optional callback to struct tee_driver_ops, such as get_tee_revision(). For OP-TEE, the specific information we want to share is clear, but other TEEs might have additional or different data. What do others think?
Implementation wise, this sounds like a reasonable approach.
-Sumit
Cheers, Jens
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 5b62139714ce..66409cf5da1c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -83,8 +83,27 @@ static ssize_t rpmb_routing_model_show(struct device *dev, } static DEVICE_ATTR_RO(rpmb_routing_model);
+static ssize_t optee_os_revision_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
struct optee *optee = dev_get_drvdata(dev);struct optee_version_info *v;if (!optee)return -ENODEV;v = &optee->version_info;if (v->os_build_id)return sysfs_emit(buf, "%u.%u (%016llx)\n", v->os_major,v->os_minor, (unsigned long long)v->os_build_id);return sysfs_emit(buf, "%u.%u\n", v->os_major, v->os_minor);+} +static DEVICE_ATTR_RO(optee_os_revision);
static struct attribute *optee_dev_attrs[] = { &dev_attr_rpmb_routing_model.attr,
&dev_attr_optee_os_revision.attr, NULL};
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c index bf8390789ecf..3d4f35599dd1 100644 --- a/drivers/tee/optee/ffa_abi.c +++ b/drivers/tee/optee/ffa_abi.c @@ -776,7 +776,8 @@ static int optee_ffa_reclaim_protmem(struct optee *optee, */
static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev,
const struct ffa_ops *ops)
const struct ffa_ops *ops,struct optee_version_info *version_info){ const struct ffa_msg_ops *msg_ops = ops->msg_ops; struct ffa_send_direct_data data = { @@ -806,6 +807,12 @@ static bool optee_ffa_api_is_compatible(struct ffa_device *ffa_dev, pr_err("Unexpected error %d\n", rc); return false; }
if (version_info) {version_info->os_major = data.data0;version_info->os_minor = data.data1;version_info->os_build_id = data.data2;}if (data.data2) pr_info("revision %lu.%lu (%08lx)", data.data0, data.data1, data.data2);@@ -1034,6 +1041,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) { const struct ffa_notifier_ops *notif_ops; const struct ffa_ops *ffa_ops;
struct optee_version_info version_info = { }; unsigned int max_notif_value; unsigned int rpc_param_count; struct tee_shm_pool *pool;@@ -1047,7 +1055,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) ffa_ops = ffa_dev->ops; notif_ops = ffa_ops->notifier_ops;
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops))
if (!optee_ffa_api_is_compatible(ffa_dev, ffa_ops, &version_info)) return -EINVAL; if (!optee_ffa_exchange_caps(ffa_dev, ffa_ops, &sec_caps,@@ -1059,6 +1067,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) optee = kzalloc(sizeof(*optee), GFP_KERNEL); if (!optee) return -ENOMEM;
optee->version_info = version_info; pool = optee_ffa_shm_pool_alloc_pages(); if (IS_ERR(pool)) {diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index db9ea673fbca..3e7bcd44976b 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -19,6 +19,22 @@
#define OPTEE_MAX_ARG_SIZE 1024
+/**
- struct optee_version_info - OP-TEE OS revision reported by secure world
- @os_major: OP-TEE OS major version
- @os_minor: OP-TEE OS minor version
- @os_build_id: OP-TEE OS build identifier (0 if unspecified)
- Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- FF-A ABI version.
- */
+struct optee_version_info {
u32 os_major;u32 os_minor;u64 os_build_id;+};
/* Some Global Platform error codes used in this driver */ #define TEEC_SUCCESS 0x00000000 #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006 @@ -249,6 +265,7 @@ struct optee { bool in_kernel_rpmb_routing; struct work_struct scan_bus_work; struct work_struct rpmb_scan_bus_work;
struct optee_version_info version_info;};
struct optee_session { diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 0be663fcd52b..07c703609320 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1323,7 +1323,8 @@ static bool optee_msg_api_uid_is_optee_image_load(optee_invoke_fn *invoke_fn) } #endif
-static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) +static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn,
struct optee_version_info *version_info){ union { struct arm_smccc_res smccc; @@ -1337,6 +1338,12 @@ static void optee_msg_get_os_revision(optee_invoke_fn *invoke_fn) invoke_fn(OPTEE_SMC_CALL_GET_OS_REVISION, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
if (version_info) {version_info->os_major = res.result.major;version_info->os_minor = res.result.minor;version_info->os_build_id = res.result.build_id;}if (res.result.build_id) pr_info("revision %lu.%lu (%0*lx)", res.result.major, res.result.minor, (int)sizeof(res.result.build_id) * 2,@@ -1727,6 +1734,7 @@ static int optee_probe(struct platform_device *pdev) unsigned int thread_count; struct tee_device *teedev; struct tee_context *ctx;
struct optee_version_info version_info = { }; u32 max_notif_value; u32 arg_cache_flags; u32 sec_caps;@@ -1745,7 +1753,7 @@ static int optee_probe(struct platform_device *pdev) return -EINVAL; }
optee_msg_get_os_revision(invoke_fn);
optee_msg_get_os_revision(invoke_fn, &version_info); if (!optee_msg_api_revision_is_compatible(invoke_fn)) { pr_warn("api revision mismatch\n");@@ -1814,6 +1822,7 @@ static int optee_probe(struct platform_device *pdev) rc = -ENOMEM; goto err_free_shm_pool; }
optee->version_info = version_info; optee->ops = &optee_ops; optee->smc.invoke_fn = invoke_fn;-- 2.43.0
Hi,
On Mon, Dec 1, 2025 at 12:48 PM Aristo Chen jj251510319013@gmail.com wrote:
Hi,
Sumit Garg sumit.garg@kernel.org 於 2025年11月25日週二 下午3:55寫道：
On Tue, Nov 25, 2025 at 01:23:22PM +0530, Sumit Garg via OP-TEE wrote:
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
Ah, typo here:
s/TEE_IOC_REVERSION/TEE_IOC_REVISION/
-Sumit
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
My current thinking is that if the TEE revision is exposed, users can write a script to capture the platform state and record the exact secure OS revision even after the dmesg/journalctl logs have rolled over. This would significantly improve bug triage and regression tracking.
In my case, I have a package with precompiled xtest binaries for multiple releases (from 3.14 to 4.6), and I work with different platforms that run different OP-TEE OS versions. Having a reliable way to obtain the TEE revision would help a lot, as it would allow me to select the correct xtest version when running tests.
I'm concerned that the ABI might be misused to be part of what the client expects from the TEE. You even express that as a use case. I'd rather fix the problem with xtest.
Cheers, Jens
Hi Jens
Jens Wiklander jens.wiklander@linaro.org 於 2025年12月1日週一 下午9:06寫道：
Hi,
On Mon, Dec 1, 2025 at 12:48 PM Aristo Chen jj251510319013@gmail.com wrote:
Hi,
Sumit Garg sumit.garg@kernel.org 於 2025年11月25日週二 下午3:55寫道：
On Tue, Nov 25, 2025 at 01:23:22PM +0530, Sumit Garg via OP-TEE wrote:
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote:
From: Aristo Chen aristo.chen@canonical.com
Today the only way to read the OP-TEE OS version is from dmesg/journal logs, which can be lost as buffers roll over. Capture the OS revision (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store it in the OP-TEE driver, and expose a stable userspace readout via /sys/class/tee/tee*/optee_os_revision.
Signed-off-by: Aristo Chen aristo.chen@canonical.com
drivers/tee/optee/core.c | 19 +++++++++++++++++++ drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ drivers/tee/optee/smc_abi.c | 13 +++++++++++-- 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
Ah, typo here:
s/TEE_IOC_REVERSION/TEE_IOC_REVISION/
-Sumit
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
My current thinking is that if the TEE revision is exposed, users can write a script to capture the platform state and record the exact secure OS revision even after the dmesg/journalctl logs have rolled over. This would significantly improve bug triage and regression tracking.
In my case, I have a package with precompiled xtest binaries for multiple releases (from 3.14 to 4.6), and I work with different platforms that run different OP-TEE OS versions. Having a reliable way to obtain the TEE revision would help a lot, as it would allow me to select the correct xtest version when running tests.
I'm concerned that the ABI might be misused to be part of what the client expects from the TEE. You even express that as a use case. I'd rather fix the problem with xtest.
Thanks for the feedback! To clarify: currently, the OP-TEE OS revision I expose in sysfs is the same value already printed in dmesg at boot (e.g., “optee: revision 4.8 (XXXXXX)”).
Are your concerns specifically about clients inferring capabilities from a revision string (“rev X.Y implies feature Z”)? If so, I agree that’s fragile and not the intent. I’m happy to add a short note in the doc that this is informational only and that feature detection must use proper capability queries.
Please let me know if that addresses the worry, or if there’s another concern I’m missing.
Thanks, Aristo
Cheers, Jens
Hi,
On Tue, Dec 2, 2025 at 10:54 AM Aristo Chen jj251510319013@gmail.com wrote:
Hi Jens
Jens Wiklander jens.wiklander@linaro.org 於 2025年12月1日週一 下午9:06寫道：
Hi,
On Mon, Dec 1, 2025 at 12:48 PM Aristo Chen jj251510319013@gmail.com wrote:
Hi,
Sumit Garg sumit.garg@kernel.org 於 2025年11月25日週二 下午3:55寫道：
On Tue, Nov 25, 2025 at 01:23:22PM +0530, Sumit Garg via OP-TEE wrote:
On Mon, Nov 24, 2025 at 08:15:04AM +0100, Jens Wiklander wrote:
Hi,
On Sat, Nov 22, 2025 at 4:00 PM Wei Ming Chen jj251510319013@gmail.com wrote: > > From: Aristo Chen aristo.chen@canonical.com > > Today the only way to read the OP-TEE OS version is from dmesg/journal > logs, which can be lost as buffers roll over. Capture the OS revision > (major/minor/build_id) from secure world for both SMC and FF-A ABIs, store > it in the OP-TEE driver, and expose a stable userspace readout via > /sys/class/tee/tee*/optee_os_revision. > > Signed-off-by: Aristo Chen aristo.chen@canonical.com > --- > drivers/tee/optee/core.c | 19 +++++++++++++++++++ > drivers/tee/optee/ffa_abi.c | 13 +++++++++++-- > drivers/tee/optee/optee_private.h | 17 +++++++++++++++++ > drivers/tee/optee/smc_abi.c | 13 +++++++++++-- > 4 files changed, 58 insertions(+), 4 deletions(-)
This appears to be a feature that could be useful for all TEEs.
True, it is something that TEE core should support. Although I would have preferred to extend TEE_IOC_VERSION since that's the common way the user-space library get's TEE implementation specific information. But since it being already a user-space ABI which doesn't offer extension. Maybe we can consider adding TEE_IOC_REVERSION instead of sysfs.
Ah, typo here:
s/TEE_IOC_REVERSION/TEE_IOC_REVISION/
-Sumit
But before doing that we need to know who is the actual consumer here from user-space perspective? Will the client applications also depend on the TEE implementation revision?
My current thinking is that if the TEE revision is exposed, users can write a script to capture the platform state and record the exact secure OS revision even after the dmesg/journalctl logs have rolled over. This would significantly improve bug triage and regression tracking.
In my case, I have a package with precompiled xtest binaries for multiple releases (from 3.14 to 4.6), and I work with different platforms that run different OP-TEE OS versions. Having a reliable way to obtain the TEE revision would help a lot, as it would allow me to select the correct xtest version when running tests.
I'm concerned that the ABI might be misused to be part of what the client expects from the TEE. You even express that as a use case. I'd rather fix the problem with xtest.
Thanks for the feedback! To clarify: currently, the OP-TEE OS revision I expose in sysfs is the same value already printed in dmesg at boot (e.g., “optee: revision 4.8 (XXXXXX)”).
Are your concerns specifically about clients inferring capabilities from a revision string (“rev X.Y implies feature Z”)? If so, I agree that’s fragile and not the intent.
Yes
I’m happy to add a short note in the doc that this is informational only and that feature detection must use proper capability queries.
Please let me know if that addresses the worry, or if there’s another concern I’m missing.
Adding APIs that aren't supposed to be used seems odd. Do you know if there are examples in the kernel for this kind of thing?
Cheers, Jens
op-tee@lists.trustedfirmware.org
-
Aristo Chen -
Harshal Dev -
Jens Wiklander -
Sumit Garg -
Thomas, Rijo-john -
Wei Ming Chen