Re: [PATCH v4 1/2] tee: add revision sysfs attribute

[Sumit Garg via OP-TEE <op-tee@lists.trustedfirmware.org>]

On Tue, Dec 30, 2025 at 01:17:58PM +0800, Aristo Chen wrote:
> Add a generic TEE revision sysfs attribute backed by a new optional
> get_tee_revision() callback. The revision string is diagnostic-only and
> must not be used to infer feature support.
> 
> Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
> ---
>  Documentation/ABI/testing/sysfs-class-tee | 10 +++++
>  drivers/tee/tee_core.c                    | 51 ++++++++++++++++++++++-
>  include/linux/tee_core.h                  |  8 ++++
>  3 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-tee b/Documentation/ABI/testing/sysfs-class-tee
> index c9144d16003e..6e783210104e 100644
> --- a/Documentation/ABI/testing/sysfs-class-tee
> +++ b/Documentation/ABI/testing/sysfs-class-tee
> @@ -13,3 +13,13 @@ Description:
>  		space if the variable is absent. The primary purpose
>  		of this variable is to let systemd know whether
>  		tee-supplicant is needed in the early boot with initramfs.
> +
> +What:		/sys/class/tee/tee{,priv}X/revision
> +Date:		Dec 2025
> +KernelVersion:	6.18
> +Contact:	op-tee@lists.trustedfirmware.org
> +Description:
> +		Read-only revision string reported by the TEE driver. This is
> +		for diagnostics only and must not be used to infer feature
> +		support. Use TEE_IOC_VERSION for capability and compatibility
> +		checks.
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index d65d47cc154e..0a00499811c1 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -1146,7 +1146,56 @@ static struct attribute *tee_dev_attrs[] = {
>  	NULL
>  };
>  
> -ATTRIBUTE_GROUPS(tee_dev);
> +static const struct attribute_group tee_dev_group = {
> +	.attrs = tee_dev_attrs,
> +};
> +
> +static ssize_t revision_show(struct device *dev,
> +			     struct device_attribute *attr, char *buf)
> +{
> +	struct tee_device *teedev = container_of(dev, struct tee_device, dev);
> +	char version[TEE_REVISION_STR_SIZE];
> +	int ret;
> +
> +	if (!teedev->desc->ops->get_tee_revision)
> +		return -ENODEV;
> +
> +	ret = teedev->desc->ops->get_tee_revision(teedev, version,
> +						  sizeof(version));
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%s\n", version);
> +}
> +static DEVICE_ATTR_RO(revision);
> +
> +static struct attribute *tee_revision_attrs[] = {
> +	&dev_attr_revision.attr,
> +	NULL
> +};
> +
> +static umode_t tee_revision_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 (teedev->desc->ops->get_tee_revision)
> +		return attr->mode;
> +
> +	return 0;
> +}
> +
> +static const struct attribute_group tee_revision_group = {
> +	.attrs = tee_revision_attrs,
> +	.is_visible = tee_revision_attr_is_visible,
> +};
> +
> +static const struct attribute_group *tee_dev_groups[] = {
> +	&tee_dev_group,
> +	&tee_revision_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..c214c38898ca 100644
> --- a/include/linux/tee_core.h
> +++ b/include/linux/tee_core.h
> @@ -76,6 +76,9 @@ struct tee_device {
>  /**
>   * struct tee_driver_ops - driver operations vtable
>   * @get_version:	returns version of driver
> + * @get_tee_revision:	returns revision string (diagnostic only);
> + *			do not infer feature support from this, use
> + *			TEE_IOC_VERSION instead
>   * @open:		called for a context when the device file is opened
>   * @close_context:	called when the device file is closed
>   * @release:		called to release the context
> @@ -98,6 +101,8 @@ struct tee_device {
>  struct tee_driver_ops {
>  	void (*get_version)(struct tee_device *teedev,
>  			    struct tee_ioctl_version_data *vers);
> +	int (*get_tee_revision)(struct tee_device *teedev,
> +				char *buf, size_t len);
>  	int (*open)(struct tee_context *ctx);
>  	void (*close_context)(struct tee_context *ctx);
>  	void (*release)(struct tee_context *ctx);
> @@ -292,6 +297,9 @@ struct tee_shm_pool *tee_shm_pool_alloc_res_mem(unsigned long vaddr,
>  						phys_addr_t paddr, size_t size,
>  						int min_alloc_order);
>  
> +/* Size for TEE revision string buffer used by get_tee_revision(). */
> +#define TEE_REVISION_STR_SIZE	128
> +

Move this macro alongside get_tee_revision() callback. With that feel
free to add:

Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>

-Sumit

>  /**
>   * tee_shm_pool_free() - Free a shared memory pool
>   * @pool:	The shared memory pool to free
> -- 
> 2.43.0
>