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

Hi Mario,

Mario Limonciello superm1@kernel.org 於 2026年1月7日週三 下午11:28寫道：

On 1/7/26 9:26 AM, 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 | 9 ++++ 3 files changed, 69 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

This needs to be bumped up and dates pushed out.

I will fix this in the v6 patch, thanks!

...

+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..ee5f0bd41f43 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);

Why is this comment here about it being for diagnostics only? I feel it's up to the implementation how it would be used.

According to the previous discussion, we would like to prevent user thinking about optee os version x.y means z feature, and we should always use TEE_IOC_VERSION for capability and compatibility check.

Is there any other specific use case that makes you think removing the wording is required?

...

• •               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

@@ -95,9 +98,12 @@ struct tee_device {

• client closes the device file, even if there are existing references to the
• context. The TEE driver can use @close_context to start cleaning up.

*/

• 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);
  

@@ -123,6 +129,9 @@ struct tee_driver_ops { int (*shm_unregister)(struct tee_context *ctx, struct tee_shm *shm); };

+/* Size for TEE revision string buffer used by get_tee_revision(). */ +#define TEE_REVISION_STR_SIZE 128

• /**
  • struct tee_desc - Describes the TEE driver to the subsystem
  • @name: name of driver

Best regards, Aristo