Re: [PATCH v2 1/1] tee: optee: expose OS revision via sysfs

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

On Mon, Dec 22, 2025 at 09:34:24AM +0100, Jens Wiklander wrote:
> Hi Aristo,
> 
> On Fri, Dec 19, 2025 at 4:39 PM Aristo Chen <jj251510319013@gmail.com> wrote:
> >
> > Hi Jens
> >
> > Jens Wiklander <jens.wiklander@linaro.org> 於 2025年12月9日週二 下午4:31寫道：
> > >
> > > Hi Aristo,
> > >
> > > On Sun, Dec 7, 2025 at 3:01 PM Aristo Chen <jj251510319013@gmail.com> wrote:
> > > >
> > > > Hi Jens,
> > > >
> > > > Jens Wiklander <jens.wiklander@linaro.org> 於 2025年12月3日週三 下午3:51寫道：
> > > > >
> > > > > 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
> > > >
> > > > Thanks for clarifying the concern!
> > > >
> > > > >
> > > > > > 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?
> > > > I’ve done some research and, as far as I can tell, the TEE core already
> > > > provides an informational-only file at /sys/class/tee/tee0/implementation_id.
> > > > My understanding is that it was introduced roughly 10 years ago. However,
> > > > I haven’t found any documentation clarifying its intended purpose, so I’m
> > > > assuming this may be a similar situation. I’d appreciate it if you could correct
> > > > me if I’m wrong.
> > >
> > > That one is for a quick way for a client to tell the different TEE
> > > device implementations apart. You're right, it resembles your case,
> > > except that this is part of a well-defined ABI.
> > >
> > > >
> > > > Also, here are some files with similiar purposes:
> > > > - /sys/devices/system/cpu/cpu*/microcode/version
> > > > - /sys/devices/virtual/dmi/id/{bios_date,bios_release,bios_vendor,bios_version}
> > > > - /sys/kernel/security/apparmor/revision
> > >
> > > Thanks, this is good background information.
> > >
> > > We have a BoF session on the TEE subsystem at LPC on Friday [1]. We'll
> > > take the opportunity to discuss it there.
> >
> > Just a quick follow-up on this patch, has there been any decision or
> > direction from that discussion?
> > I’m happy to rework the patch in whatever direction the group prefers.
> 
> No one was against it. We didn't discuss the details, but here's what
> I'd like to see:
> - The callback name should be get_tee_revision() or get_tee_fw_version()
> - The FW version is returned as a string for flexibility
> - A note that the FW version is not intended to be used to determine
> what features, etc, are available, TEE_IOC_VERSION should cover that.
> 
> Sumit, what do you think?

That sounds reasonable to me.

-Sumit