From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 86BB31DF254 for ; Thu, 8 Jan 2026 03:01:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767841302; cv=none; b=WwIwqIwN+6abkUYNiPnY1lkbugEqK18uoZ3yRZvUDmzi6ImfpmwC6CqO0nhaY3vPYo4lVWA9Z60MMNlQG2t2D2CgDzCnBgRHEVsFvYGNlEWIJ8hUI291DxteIosEHvq+Aof4RUKT19E6NYuYsS//ft8CEhbxzBcoLX9V5QAP5Ys= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767841302; c=relaxed/simple; bh=h6fiBmn3kgiXJASgqnUpEHC6/S3AHMHkcpmkj0+Ht5w=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=kbe0ipgI+IJLIs1mokvh4Ird2kstWc6TaHj1F9hssPFkMAjhHXZwJSf2973gEbB2DeT1sDK/9yQbbx3rmzs2atxsVFfcTDHJ1Zwy+Df68k32ohjL4N3+BIZqFEom1ogxvJldRE2jDq4CZK994umQUzCFHm34CZOxRelrjzeK7gE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=cLqCrsBS; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="cLqCrsBS" Received: by smtp.kernel.org (Postfix) with ESMTPSA id F1EE4C4CEF1; Thu, 8 Jan 2026 03:01:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1767841302; bh=h6fiBmn3kgiXJASgqnUpEHC6/S3AHMHkcpmkj0+Ht5w=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=cLqCrsBSuhIBNhnxJHC2AQXc84sLS0NPuvTXVj9eDsvWdBPr95Qj4wTohB7ETwjiD WuZ+3ikfAgzu9p0pgOWE1v3YEpMfIERKSsDEgoj0spb8+qHbANUUoTg5/PBSjJ4b3E AUx2AtJF0N7Us2mqZom+pNGX3CtCVUTGlYwwtLLg2s5Vi42UMfesc2srdXtwQiw2pP VEZKY0k9gha3xyeMsE98X8CVG1H5EsIW6SIX3tRNc3KJchUGIoG7d/tAHIG7z9Q9oG exvoljrTtdWpsrUhKDx71ks5TnnzEzFf3XY8C3emkvFJsLWtl1UTg/BYMHUBUTX5B1 gD739gR9bxkKw== Message-ID: Date: Wed, 7 Jan 2026 21:01:39 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/2] tee: add revision sysfs attribute To: Aristo Chen Cc: linux-kernel@vger.kernel.org, jens.wiklander@linaro.org, sumit.garg@kernel.org, op-tee@lists.trustedfirmware.org, harshal.dev@oss.qualcomm.com, Rijo-john.Thomas@amd.com, amirreza.zarrabi@oss.qualcomm.com, Aristo Chen References: <20251230051804.6230-1-aristo.chen@canonical.com> <20260107152607.902735-1-aristo.chen@canonical.com> <2f4b30fd-8e0a-4482-9bab-a90e32e69839@kernel.org> Content-Language: en-US From: "Mario Limonciello (AMD) (kernel.org)" In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 1/7/2026 8:55 PM, Aristo Chen wrote: > Hi Mario, > > Mario Limonciello 於 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 >>> --- >>> 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? Ah I didn't realize there was previous discussion that lead to this, I saw some earlier versions in my holiday mailbox glut but ignored them when I saw the new one. Leave it as is then. > >> >>> + * 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