[PATCH v2 0/6] Add support for a TEE agnostic Inline Crypto Engine service

[PATCH v2 0/6] Add support for a TEE agnostic Inline Crypto Engine service

[Harshal Dev]

Qualcomm platforms such as the IQ-9075 EVK now boot with both Qualcomm
Trusted Execution Environment (QTEE) and OP-TEE [1]. However, the Qualcomm
Inline Crypto Engine (ICE) driver currently only makes SCM calls into QTEE.
With the arrival of ICE support on OPTEE [2,3] for Qualcomm platforms,
there is a need for TrustZone (TZ) agnostic APIs which re-direct the call
to the correct TZ at runtime to request programming of the secure interfaces
on the ICE hardware block.
    
This patch series adds such TZ agnostic APIs to the ICE driver for
requesting secure services from either QTEE or OP-TEE depending on whether
an SCM driver back-end or a TEE client driver back-end is registered with
it at runtime. This series is logically similar to the one introduced for
Peripheral Authentication Service (PAS) [3] which was recently merged to
provide a generic PAS service for both QTEE and OP-TEE.

Apart from introducing a TEE bus based client driver for ICE, this series
also adds support for optionally registering a TEE device which represents
a particular TEE (QTEE or OPTEE) on the platform bus to enable drivers such
as ICE which poll on the availability of a TEE interface to be notified.

The ICE driver is an independent platform driver which polls on the
availability of a TEE interface (SCM driver) before checking if the ICE
secure service is supported by QTEE by querying qcom_scm_is_available().
The TEE subsystem currently does not provide any mechanism to notify the
availability of a TEE interface, i.e. whether QCOMTEE or OPTEE have probed.
Without the introduction of such a notification mechanism the ICE driver
would indefinitely defer probe until the OPTEE ICE Pseudo TA becomes available
on the TEE bus implying that for OPTEE builds without the ICE PTA enabled,
the dependent UFS/SDHC storage driver will fail to come online.

The TEE bus based client driver introduced in this series which registers
with the ICE driver has an ABI with OP-TEE that is 1-1 identical with the
SCM driver's ICE ABI with QTEE. This is because both TEEs implement logically
similar drivers with the exception that OP-TEE currently does not have
Hardware Key Manager v2 support and so the inline encryption functionality
with OP-TEE is currently limited to Qualcomm platforms with only Hardware
Key Manager v1.

Validation:
This series has been validated on a LeMans IQ-9075 EVK device by attempting
file system encryption via the fscrypt tool.

Merge Strategy:
All patches in this series except one touch Qualcomm firmware and soc drivers.
Hence, it should go via the QCOM-SoC tree. Request the TEE subsystem maintainers
to provide an Acknowledgement for this.

[1] https://ldts.github.io/qcom-buildroot/index.html
[2] https://github.com/OP-TEE/optee_os/pull/7906 (In review)
[3] https://github.com/OP-TEE/optee_os/pull/8137 (Draft, depends on PR#7906)
[4] https://lore.kernel.org/all/20260702115835.167602-1-sumit.garg@kernel.org/

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
Changes in v2:
- Merged the furnctionality of the middleware ICE service into the ICE
  driver as requested by Eric.
- Introduced tee_platform_device_register() to fix the issue of indefinite
  probe defer in the ICE driver when ICE PTA is not enabled in OPTEE as
  pointed out by Eric.
- Fixed type "Psuedo" in ICE TEE back-end commit.
- Fixed space based indentation with tab in ICE back-end Kconfig.
- Ensure tee_shm_get_va() failure is checked and ERR_PTR() returned.
- Validate out_shm size everywhere in qcom_ice_tee.c
- Zero the memory holding the key when TEE invoke call fails.
- Check failure from qcom_ice_tz_ops_register() during probe.
- Cross-check out_size and lt_key_size in qcom_ice_tee_generate_key().
- Always return 0 when qcom_ice_tee_probe() is success.
- Did not collect tags from Abel since significant rework undertaken.
- Link to v1: https://lore.kernel.org/r/20261021-ice-migrate-tee-svc-v1-0-b035be7aab0a@oss.qualcomm.com

---
Harshal Dev (6):
      soc: qcom: ice: Add TZ agnostic re-direction functions
      firmware: qcom_scm: Transition to ICE TZ agnostic operations
      firmware: qcom: Add a TEE bus based ICE back-end
      tee: optee: Register TEE devices on the platform bus
      soc: qcom: ice: Migrate to the TZ agnostic ICE APIs
      firmware: qcom: scm: Drop exported ICE wrapper functions

 MAINTAINERS                                |   8 +
 drivers/firmware/qcom/Kconfig              |   9 +
 drivers/firmware/qcom/Makefile             |   1 +
 drivers/firmware/qcom/qcom_ice_tee.c       | 443 +++++++++++++++++++++++++++++
 drivers/firmware/qcom/qcom_scm.c           |  90 +++---
 drivers/soc/qcom/Kconfig                   |   1 +
 drivers/soc/qcom/ice.c                     | 173 +++++++++--
 drivers/tee/optee/ffa_abi.c                |   8 +
 drivers/tee/optee/smc_abi.c                |   8 +
 drivers/tee/tee_core.c                     |  21 ++
 include/linux/firmware/qcom/qcom_ice_tee.h |  10 +
 include/linux/firmware/qcom/qcom_scm.h     |  20 --
 include/linux/tee_core.h                   |   9 +
 include/soc/qcom/ice.h                     |  56 ++++
 14 files changed, 784 insertions(+), 73 deletions(-)
---
base-commit: 8f7df005a7564bd7ffa75da9c3085d5b996f1e16
change-id: 20261021-ice-migrate-tee-svc-a5cd6959ae36

Best regards,
-- 
Harshal Dev <harshal.dev@oss.qualcomm.com>

[PATCH v2 1/6] soc: qcom: ice: Add TZ agnostic re-direction functions

[Harshal Dev]

Qualcomm platforms such as the IQ-9075 EVK now boot with both QTEE and
OPTEE. However, the Qualcomm Inline Crypto Engine (ICE) driver currently
only makes SCM calls into QTEE. With the arrival of ICE support on OPTEE
for Qualcomm platforms the ICE driver should call into the correct Trust
Zone (TZ) at runtime to request programming of the secure interfaces on
the hardware block.

Add TZ agnostic re-direction functions for all the current QTEE SCM calls
which can route the calls to either an SCM or TEE bus-based secure back
end when registered with ICE.

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 drivers/soc/qcom/ice.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/soc/qcom/ice.h |  56 ++++++++++++++++++++++
 2 files changed, 180 insertions(+)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index e256595b9826..452896b1858f 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -118,6 +118,130 @@ struct qcom_ice {
 static DEFINE_XARRAY(ice_handles);
 static DEFINE_MUTEX(ice_mutex);
 
+static struct qcom_ice_tz_ops *ice_tz_ops;
+
+static bool qcom_ice_tz_ops_registered(void)
+{
+	/*
+	 * The barrier is intended to synchronize the data stores for the
+	 * ice_tz_ops data structure when client drivers (UFS/SDHC) are in
+	 * parallel checking for ICE service availability.
+	 */
+	return !!smp_load_acquire(&ice_tz_ops);
+}
+
+static bool qcom_ice_tz_is_available(void)
+{
+	return qcom_scm_is_available();
+}
+
+static int qcom_ice_tz_invalidate_key(u32 index)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_invalidate_key(ops->dev, index);
+}
+
+static int qcom_ice_tz_set_key(u32 index, const u8 *key, u32 key_size,
+			       enum qcom_ice_cipher cipher,
+			       u32 data_unit_size)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_set_key(ops->dev, index, key, key_size, cipher,
+				data_unit_size);
+}
+
+static bool qcom_ice_tz_has_wrapped_key_support(void)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_has_wrapped_key_support(ops->dev);
+}
+
+static int qcom_ice_tz_derive_sw_secret(const u8 *eph_key,
+					size_t eph_key_size, u8 *sw_secret,
+					size_t sw_secret_size)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_derive_sw_secret(ops->dev, eph_key, eph_key_size,
+					 sw_secret, sw_secret_size);
+}
+
+static int qcom_ice_tz_generate_key(u8 *lt_key, size_t lt_key_size)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_generate_key(ops->dev, lt_key, lt_key_size);
+}
+
+static int qcom_ice_tz_prepare_key(const u8 *lt_key, size_t lt_key_size,
+				   u8 *eph_key, size_t eph_key_size)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_prepare_key(ops->dev, lt_key, lt_key_size, eph_key,
+				    eph_key_size);
+}
+
+static int qcom_ice_tz_import_key(const u8 *raw_key, size_t raw_key_size,
+				  u8 *lt_key, size_t lt_key_size)
+{
+	struct qcom_ice_tz_ops *ops;
+
+	ops = ice_tz_ops;
+	if (!ops)
+		return -ENODEV;
+
+	return ops->ice_import_key(ops->dev, raw_key, raw_key_size, lt_key,
+				   lt_key_size);
+}
+
+int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops)
+{
+	if (!qcom_ice_tz_ops_registered())
+		/* Paired with smp_load_acquire() in qcom_ice_tz_ops_registered() */
+		smp_store_release(&ice_tz_ops, ops);
+	else
+		return -EBUSY;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_ice_tz_ops_register);
+
+void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops)
+{
+	if (READ_ONCE(ice_tz_ops) == ops)
+		/* Paired with smp_load_acquire() in qcom_ice_tz_ops_registered() */
+		smp_store_release(&ice_tz_ops, NULL);
+}
+EXPORT_SYMBOL_GPL(qcom_ice_tz_ops_unregister);
+
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
 {
 	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 4bee553f0a59..89e62e151b85 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -10,6 +10,47 @@
 #include <linux/types.h>
 
 struct qcom_ice;
+struct device;
+
+enum qcom_ice_cipher {
+	QCOM_ICE_CIPHER_AES_128_XTS = 0,
+	QCOM_ICE_CIPHER_AES_128_CBC = 1,
+	QCOM_ICE_CIPHER_AES_256_XTS = 3,
+	QCOM_ICE_CIPHER_AES_256_CBC = 4,
+};
+
+/**
+ * struct qcom_ice_tz_ops - Qcom ICE TZ operations
+ * @drv_name:                    ICE TZ backend driver name.
+ * @dev:                         ICE TZ backend device.
+ * @ice_invalidate_key:          Invalidate keyslot callback.
+ * @ice_set_key:                 Program keyslot callback.
+ * @ice_has_wrapped_key_support: Wrapped-key support callback.
+ * @ice_derive_sw_secret:        Derive software secret callback.
+ * @ice_generate_key:            Generate wrapped key callback.
+ * @ice_prepare_key:             Prepare wrapped key callback.
+ * @ice_import_key:              Import wrapped key callback.
+ */
+struct qcom_ice_tz_ops {
+	const char *drv_name;
+	struct device *dev;
+	int (*ice_invalidate_key)(struct device *dev, u32 index);
+	int (*ice_set_key)(struct device *dev, u32 index, const u8 *key,
+			   u32 key_size, enum qcom_ice_cipher cipher,
+			   u32 data_unit_size);
+	bool (*ice_has_wrapped_key_support)(struct device *dev);
+	int (*ice_derive_sw_secret)(struct device *dev, const u8 *eph_key,
+				    size_t eph_key_size, u8 *sw_secret,
+				    size_t sw_secret_size);
+	int (*ice_generate_key)(struct device *dev, u8 *lt_key,
+				size_t lt_key_size);
+	int (*ice_prepare_key)(struct device *dev, const u8 *lt_key,
+			       size_t lt_key_size, u8 *eph_key,
+			       size_t eph_key_size);
+	int (*ice_import_key)(struct device *dev, const u8 *raw_key,
+			      size_t raw_key_size, u8 *lt_key,
+			      size_t lt_key_size);
+};
 
 int qcom_ice_enable(struct qcom_ice *ice);
 int qcom_ice_resume(struct qcom_ice *ice);
@@ -31,4 +72,19 @@ int qcom_ice_import_key(struct qcom_ice *ice,
 			u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
 struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
 
+#ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
+int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops);
+void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops);
+#else
+static inline int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *__unused)
+{
+	return 0;
+}
+
+static inline
+void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *__unused)
+{
+}
+#endif /* CONFIG_QCOM_INLINE_CRYPTO_ENGINE */
+
 #endif /* __QCOM_ICE_H__ */

-- 
2.34.1

[PATCH v2 2/6] firmware: qcom_scm: Transition to ICE TZ agnostic operations

[Harshal Dev]

Transition the ICE related SCM calls to the generic ICE TZ operations by
registering them with the ICE driver and wrapping them in exported APIs
with the same signature.

This prepares the ICE driver for eventual migration to the TZ agnostic
secure programming interfaces once the TEE bus based back-end becomes
available.

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 131 +++++++++++++++++++++++++++++++--------
 1 file changed, 106 insertions(+), 25 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 43e250faea75..6b66bcedc84f 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -32,6 +32,8 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 
+#include <soc/qcom/ice.h>
+
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
 #include "qcom_pas.h"
@@ -1397,6 +1399,14 @@ int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_ocmem_unlock);
 
+bool __qcom_scm_ice_available(struct device *dev)
+{
+	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
+					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
+		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
+					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
+}
+
 /**
  * qcom_scm_ice_available() - Is the ICE key programming interface available?
  *
@@ -1405,10 +1415,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_ocmem_unlock);
  */
 bool qcom_scm_ice_available(void)
 {
-	return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
-					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
-		__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
-					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
+	return __qcom_scm_ice_available(__scm->dev);
 }
 EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
 
@@ -1424,7 +1431,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_ice_invalidate_key(u32 index)
+static int __qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1434,7 +1441,12 @@ int qcom_scm_ice_invalidate_key(u32 index)
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call(dev, &desc, NULL);
+}
+
+int qcom_scm_ice_invalidate_key(u32 index)
+{
+	return __qcom_scm_ice_invalidate_key(__scm->dev, index);
 }
 EXPORT_SYMBOL_GPL(qcom_scm_ice_invalidate_key);
 
@@ -1459,8 +1471,10 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_invalidate_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
-			 enum qcom_scm_ice_cipher cipher, u32 data_unit_size)
+static int __qcom_scm_ice_set_key(struct device *dev, u32 index,
+				  const u8 *key, u32 key_size,
+				  enum qcom_ice_cipher cipher,
+				  u32 data_unit_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1485,25 +1499,38 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
 	memcpy(keybuf, key, key_size);
 	desc.args[1] = qcom_tzmem_to_phys(keybuf);
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
+	ret = qcom_scm_call(dev, &desc, NULL);
 
 	memzero_explicit(keybuf, key_size);
 
 	return ret;
 }
+
+int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
+			 enum qcom_scm_ice_cipher cipher, u32 data_unit_size)
+{
+	return __qcom_scm_ice_set_key(__scm->dev, index, key, key_size,
+				      (enum qcom_ice_cipher)cipher,
+				      data_unit_size);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
 
-bool qcom_scm_has_wrapped_key_support(void)
+static bool __qcom_scm_has_wrapped_key_support(struct device *dev)
 {
-	return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
+	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
 					    QCOM_SCM_ES_DERIVE_SW_SECRET) &&
-	       __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
+	       __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
 					    QCOM_SCM_ES_GENERATE_ICE_KEY) &&
-	       __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
+	       __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
 					    QCOM_SCM_ES_PREPARE_ICE_KEY) &&
-	       __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_ES,
+	       __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
 					    QCOM_SCM_ES_IMPORT_ICE_KEY);
 }
+
+bool qcom_scm_has_wrapped_key_support(void)
+{
+	return __qcom_scm_has_wrapped_key_support(__scm->dev);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_has_wrapped_key_support);
 
 /**
@@ -1522,8 +1549,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_has_wrapped_key_support);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
-			      u8 *sw_secret, size_t sw_secret_size)
+static int __qcom_scm_derive_sw_secret(struct device *dev,
+				       const u8 *eph_key, size_t eph_key_size,
+				       u8 *sw_secret, size_t sw_secret_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1552,7 +1580,7 @@ int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
 	desc.args[2] = qcom_tzmem_to_phys(sw_secret_buf);
 	desc.args[3] = sw_secret_size;
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
+	ret = qcom_scm_call(dev, &desc, NULL);
 	if (!ret)
 		memcpy(sw_secret, sw_secret_buf, sw_secret_size);
 
@@ -1560,6 +1588,13 @@ int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
 	memzero_explicit(sw_secret_buf, sw_secret_size);
 	return ret;
 }
+
+int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
+			      u8 *sw_secret, size_t sw_secret_size)
+{
+	return __qcom_scm_derive_sw_secret(__scm->dev, eph_key, eph_key_size,
+				   sw_secret, sw_secret_size);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret);
 
 /**
@@ -1573,7 +1608,8 @@ EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
+static int __qcom_scm_generate_ice_key(struct device *dev,
+				       u8 *lt_key, size_t lt_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1592,13 +1628,18 @@ int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
 	desc.args[0] = qcom_tzmem_to_phys(lt_key_buf);
 	desc.args[1] = lt_key_size;
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
+	ret = qcom_scm_call(dev, &desc, NULL);
 	if (!ret)
 		memcpy(lt_key, lt_key_buf, lt_key_size);
 
 	memzero_explicit(lt_key_buf, lt_key_size);
 	return ret;
 }
+
+int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
+{
+	return __qcom_scm_generate_ice_key(__scm->dev, lt_key, lt_key_size);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
 
 /**
@@ -1614,8 +1655,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
-			     u8 *eph_key, size_t eph_key_size)
+static int __qcom_scm_prepare_ice_key(struct device *dev,
+				      const u8 *lt_key, size_t lt_key_size,
+				      u8 *eph_key, size_t eph_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1644,7 +1686,7 @@ int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
 	desc.args[2] = qcom_tzmem_to_phys(eph_key_buf);
 	desc.args[3] = eph_key_size;
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
+	ret = qcom_scm_call(dev, &desc, NULL);
 	if (!ret)
 		memcpy(eph_key, eph_key_buf, eph_key_size);
 
@@ -1652,6 +1694,13 @@ int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
 	memzero_explicit(eph_key_buf, eph_key_size);
 	return ret;
 }
+
+int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
+			     u8 *eph_key, size_t eph_key_size)
+{
+	return __qcom_scm_prepare_ice_key(__scm->dev, lt_key, lt_key_size,
+				  eph_key, eph_key_size);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
 
 /**
@@ -1667,8 +1716,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
-			    u8 *lt_key, size_t lt_key_size)
+static int __qcom_scm_import_ice_key(struct device *dev,
+				     const u8 *raw_key, size_t raw_key_size,
+				     u8 *lt_key, size_t lt_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1697,7 +1747,7 @@ int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
 	desc.args[2] = qcom_tzmem_to_phys(lt_key_buf);
 	desc.args[3] = lt_key_size;
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
+	ret = qcom_scm_call(dev, &desc, NULL);
 	if (!ret)
 		memcpy(lt_key, lt_key_buf, lt_key_size);
 
@@ -1705,8 +1755,26 @@ int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
 	memzero_explicit(lt_key_buf, lt_key_size);
 	return ret;
 }
+
+int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
+			    u8 *lt_key, size_t lt_key_size)
+{
+	return __qcom_scm_import_ice_key(__scm->dev, raw_key, raw_key_size,
+				 lt_key, lt_key_size);
+}
 EXPORT_SYMBOL_GPL(qcom_scm_import_ice_key);
 
+static struct qcom_ice_tz_ops qcom_ice_scm_ops = {
+	.drv_name = "qcom_scm",
+	.ice_invalidate_key = __qcom_scm_ice_invalidate_key,
+	.ice_set_key = __qcom_scm_ice_set_key,
+	.ice_has_wrapped_key_support = __qcom_scm_has_wrapped_key_support,
+	.ice_derive_sw_secret = __qcom_scm_derive_sw_secret,
+	.ice_generate_key = __qcom_scm_generate_ice_key,
+	.ice_prepare_key = __qcom_scm_prepare_ice_key,
+	.ice_import_key	= __qcom_scm_import_ice_key,
+};
+
 /**
  * qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
  *
@@ -2820,6 +2888,18 @@ static int qcom_scm_probe(struct platform_device *pdev)
 					     "Failed to request qcom-scm irq\n");
 	}
 
+	/*
+	 * If the ICE SCM interface is available, the operations MUST be
+	 * registered before the SCM API is published below, otherwise
+	 * ICE driver could attempt to call the ICE SCM pointers before
+	 * they are registered and assume that they aren't available.
+	 */
+	if (__qcom_scm_ice_available(scm->dev)) {
+		qcom_ice_scm_ops.dev = scm->dev;
+		ret = qcom_ice_tz_ops_register(&qcom_ice_scm_ops);
+		WARN(ret < 0, "failed to register ice SCM ops: %d\n", ret);
+	}
+
 	/*
 	 * Paired with smp_load_acquire() in qcom_scm_is_available().
 	 *
@@ -2876,6 +2956,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev)
 	/* Clean shutdown, disable download mode to allow normal restart */
 	qcom_scm_set_download_mode(__scm, QCOM_DLOAD_NODUMP);
 	qcom_pas_ops_unregister();
+	qcom_ice_tz_ops_unregister(&qcom_ice_scm_ops);
 }
 
 static const struct of_device_id qcom_scm_dt_match[] = {

-- 
2.34.1

[PATCH v2 3/6] firmware: qcom: Add a TEE bus based ICE back-end

[Harshal Dev]

Add support for a TEE client driver which forwards calls from the Inline
Crypto Engine (ICE) driver to the OPTEE ICE Pseudo-TA.

The ICE PTA supports both ICE Legacy and Standard modes. In Standard mode
Hardware Key Manager v1 is supported.

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 MAINTAINERS                          |   7 +
 drivers/firmware/qcom/Kconfig        |   9 +
 drivers/firmware/qcom/Makefile       |   1 +
 drivers/firmware/qcom/qcom_ice_tee.c | 402 +++++++++++++++++++++++++++++++++++
 4 files changed, 419 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f58a305de664..1132e456488b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23007,6 +23007,13 @@ F:	Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
 F:	drivers/soc/qcom/ice.c
 F:	include/soc/qcom/ice.h
 
+QUALCOMM INLINE CRYPTO ENGINE TEE SERVICE
+M:	Harshal Dev <harshal.dev@oss.qualcomm.com>
+L:	linux-arm-msm@vger.kernel.org
+L:	linux-crypto@vger.kernel.org
+S:	Maintained
+F:	drivers/firmware/qcom/qcom_ice_tee.c
+
 QUALCOMM INTERCONNECT BWMON DRIVER
 M:	Krzysztof Kozlowski <krzk@kernel.org>
 L:	linux-arm-msm@vger.kernel.org
diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
index 3ad22f8fc3e5..189ada7fdcd6 100644
--- a/drivers/firmware/qcom/Kconfig
+++ b/drivers/firmware/qcom/Kconfig
@@ -25,6 +25,15 @@ config QCOM_PAS_TEE
 	  Enable the generic Peripheral Authentication Service (PAS) provided
 	  by the firmware TEE implementation as the backend.
 
+config QCOM_ICE_TEE
+	tristate
+	depends on TEE
+	depends on !CPU_BIG_ENDIAN
+	default m if ARCH_QCOM
+	help
+	  Enable the generic Inline Crypto Engine (ICE) firmware service TEE
+	  implementation as a backend.
+
 config QCOM_SCM
 	tristate "Qualcomm PAS SCM interface driver" if ARCH_QCOM || COMPILE_TEST
 	select QCOM_PAS
diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
index 88ce74d74c3e..5ef64f17e6e5 100644
--- a/drivers/firmware/qcom/Makefile
+++ b/drivers/firmware/qcom/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_QCOM_QSEECOM)	+= qcom_qseecom.o
 obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
 obj-$(CONFIG_QCOM_PAS)		+= qcom_pas.o
 obj-$(CONFIG_QCOM_PAS_TEE)	+= qcom_pas_tee.o
+obj-$(CONFIG_QCOM_ICE_TEE)     += qcom_ice_tee.o
diff --git a/drivers/firmware/qcom/qcom_ice_tee.c b/drivers/firmware/qcom/qcom_ice_tee.c
new file mode 100644
index 000000000000..694aa05847f0
--- /dev/null
+++ b/drivers/firmware/qcom/qcom_ice_tee.c
@@ -0,0 +1,402 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include <linux/firmware/qcom/qcom_ice_tee.h>
+
+#include <soc/qcom/ice.h>
+
+#define PTA_QCOM_ICE_INVALIDATE_KEY		0
+#define PTA_QCOM_ICE_SET_CONFIG_KEY		1
+#define PTA_QCOM_ICE_GENERATE_KEY		2
+#define PTA_QCOM_ICE_IMPORT_KEY			3
+#define PTA_QCOM_ICE_EXPORT_KEY			4
+#define PTA_QCOM_ICE_GET_RAW_SECRET		5
+#define PTA_QCOM_ICE_HAS_WRAPPED_KEY_SUPPORT	6
+
+#define TEE_NUM_PARAMS				4
+
+struct qcom_ice_tee_private {
+	struct device *dev;
+	struct tee_context *ctx;
+	u32 session_id;
+};
+
+static int qcom_ice_tee_invalidate_key(struct device *dev, u32 index)
+{
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+	struct tee_ioctl_invoke_arg inv_arg = {
+		.func = PTA_QCOM_ICE_INVALIDATE_KEY,
+		.session = data->session_id,
+		.num_params = TEE_NUM_PARAMS,
+	};
+	struct tee_param param[TEE_NUM_PARAMS] = {
+		[0] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
+			.u.value.a = index,
+			.u.value.b = 0,
+		}
+	};
+	int ret;
+
+	ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
+	if (ret < 0 || inv_arg.ret != 0)
+		ret = ret ?: -EINVAL;
+
+	return ret;
+}
+
+static int qcom_ice_tee_set_key(struct device *dev, u32 index,
+				const u8 *key, u32 key_size,
+				enum qcom_ice_cipher cipher,
+				u32 data_unit_size)
+{
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+	struct tee_ioctl_invoke_arg inv_arg = {
+		.func = PTA_QCOM_ICE_SET_CONFIG_KEY,
+		.session = data->session_id,
+		.num_params = TEE_NUM_PARAMS,
+	};
+	struct tee_param param[TEE_NUM_PARAMS] = {
+		[0] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
+			.u.value.a = index,
+			.u.value.b = cipher,
+		},
+		[1] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
+			.u.value.a = data_unit_size,
+		},
+		[2] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
+		}
+	};
+	struct tee_shm *key_shm;
+	u8 *key_buf;
+	int ret;
+
+	key_shm = tee_shm_alloc_kernel_buf(data->ctx, key_size);
+	if (IS_ERR(key_shm))
+		return PTR_ERR(key_shm);
+
+	key_buf = tee_shm_get_va(key_shm, 0);
+	if (IS_ERR(key_buf)) {
+		ret = PTR_ERR(key_buf);
+		goto exit;
+	}
+
+	memcpy(key_buf, key, key_size);
+	param[2].u.memref.shm = key_shm;
+	param[2].u.memref.size = key_size;
+
+	ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
+	memzero_explicit(key_buf, key_size);
+	if (ret < 0 || inv_arg.ret != 0) {
+		dev_err(dev,
+			"ICE set_key failed, idx=%u, ret=%d, err=0x%x\n",
+			index, ret, inv_arg.ret);
+		ret = ret ?: -EINVAL;
+	}
+exit:
+	tee_shm_free(key_shm);
+	return ret;
+}
+
+static int qcom_ice_tee_wrapped_key_call(struct device *dev, u32 cmd,
+					 const u8 *in, size_t in_size,
+					 u8 *out, size_t out_size)
+{
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+	struct tee_ioctl_invoke_arg inv_arg = {
+		.func = cmd,
+		.session = data->session_id,
+		.num_params = TEE_NUM_PARAMS,
+	};
+	struct tee_param param[TEE_NUM_PARAMS] = { };
+	struct tee_shm *in_shm = NULL;
+	struct tee_shm *out_shm = NULL;
+	size_t out_buf_size;
+	u8 *buf;
+	int ret;
+
+	if (in && in_size) {
+		in_shm = tee_shm_alloc_kernel_buf(data->ctx, in_size);
+		if (IS_ERR(in_shm))
+			return PTR_ERR(in_shm);
+
+		buf = tee_shm_get_va(in_shm, 0);
+		if (IS_ERR(buf)) {
+			ret = PTR_ERR(buf);
+			goto exit;
+		}
+		memcpy(buf, in, in_size);
+
+		param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
+		param[0].u.memref.shm = in_shm;
+		param[0].u.memref.size = in_size;
+	}
+
+	if (out && out_size) {
+		out_shm = tee_shm_alloc_kernel_buf(data->ctx, out_size);
+		if (IS_ERR(out_shm)) {
+			ret = PTR_ERR(out_shm);
+			goto exit;
+		}
+
+		param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+		param[1].u.memref.shm = out_shm;
+		param[1].u.memref.size = out_size;
+	}
+
+	ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
+	if (ret < 0 || inv_arg.ret != 0) {
+		dev_err(dev,
+			"ICE cmd %u failed, ret=%d, err=0x%x\n",
+			cmd, ret, inv_arg.ret);
+		ret = ret ?: -EINVAL;
+		goto exit;
+	}
+
+	if (out && out_size) {
+		out_buf_size = param[1].u.memref.size;
+		if (out_size != out_buf_size) {
+			ret = -EINVAL;
+			goto exit;
+		}
+
+		buf = tee_shm_get_va(out_shm, 0);
+		if (IS_ERR(buf)) {
+			ret = PTR_ERR(buf);
+			goto exit;
+		}
+		memcpy(out, buf, out_buf_size);
+		memzero_explicit(buf, out_buf_size);
+	}
+exit:
+	if (in_shm && !IS_ERR(in_shm)) {
+		buf = tee_shm_get_va(in_shm, 0);
+		if (!IS_ERR(buf))
+			memzero_explicit(buf, in_size);
+		tee_shm_free(in_shm);
+	}
+	if (out_shm && !IS_ERR(out_shm)) {
+		buf = tee_shm_get_va(out_shm, 0);
+		if (!IS_ERR(buf))
+			memzero_explicit(buf, out_size);
+		tee_shm_free(out_shm);
+	}
+
+	return ret;
+}
+
+static int qcom_ice_tee_derive_sw_secret(struct device *dev,
+					 const u8 *eph_key,
+					 size_t eph_key_size,
+					 u8 *sw_secret,
+					 size_t sw_secret_size)
+{
+	return qcom_ice_tee_wrapped_key_call(dev, PTA_QCOM_ICE_GET_RAW_SECRET,
+					     eph_key, eph_key_size,
+					     sw_secret, sw_secret_size);
+}
+
+static int qcom_ice_tee_generate_key(struct device *dev,
+				     u8 *lt_key, size_t lt_key_size)
+{
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+	struct tee_ioctl_invoke_arg inv_arg = {
+		.func = PTA_QCOM_ICE_GENERATE_KEY,
+		.session = data->session_id,
+		.num_params = TEE_NUM_PARAMS,
+	};
+	struct tee_param param[TEE_NUM_PARAMS] = { };
+	struct tee_shm *key_shm;
+	u8 *buf;
+	size_t out_size;
+	int ret;
+
+	key_shm = tee_shm_alloc_kernel_buf(data->ctx, lt_key_size);
+	if (IS_ERR(key_shm))
+		return PTR_ERR(key_shm);
+
+	param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+	param[0].u.memref.shm = key_shm;
+	param[0].u.memref.shm_offs = 0;
+	param[0].u.memref.size = lt_key_size;
+
+	ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
+	if (ret < 0 || inv_arg.ret != 0) {
+		dev_err(dev,
+			"ICE generate_key failed, ret=%d, err=0x%x\n",
+			ret, inv_arg.ret);
+		ret = ret ?: -EINVAL;
+		goto exit;
+	}
+
+	buf = tee_shm_get_va(key_shm, 0);
+	if (IS_ERR(buf)) {
+		ret = PTR_ERR(buf);
+		goto exit;
+	}
+
+	out_size = param[0].u.memref.size;
+	if (out_size != lt_key_size) {
+		ret = -EINVAL;
+		goto clear;
+	}
+
+	memcpy(lt_key, buf, out_size);
+clear:
+	memzero_explicit(buf, lt_key_size);
+exit:
+	tee_shm_free(key_shm);
+	return ret;
+}
+
+static int qcom_ice_tee_prepare_key(struct device *dev,
+				    const u8 *lt_key,
+				    size_t lt_key_size,
+				    u8 *eph_key,
+				    size_t eph_key_size)
+{
+	return qcom_ice_tee_wrapped_key_call(dev, PTA_QCOM_ICE_EXPORT_KEY,
+					     lt_key, lt_key_size,
+					     eph_key, eph_key_size);
+}
+
+static int qcom_ice_tee_import_key(struct device *dev,
+				   const u8 *raw_key,
+				   size_t raw_key_size,
+				   u8 *lt_key,
+				   size_t lt_key_size)
+{
+	return qcom_ice_tee_wrapped_key_call(dev, PTA_QCOM_ICE_IMPORT_KEY,
+					     raw_key, raw_key_size,
+					     lt_key, lt_key_size);
+}
+
+static bool qcom_ice_tee_has_wrapped_key_support(struct device *dev)
+{
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+	struct tee_ioctl_invoke_arg inv_arg = {
+		.func = PTA_QCOM_ICE_HAS_WRAPPED_KEY_SUPPORT,
+		.session = data->session_id,
+		.num_params = TEE_NUM_PARAMS,
+	};
+	struct tee_param param[TEE_NUM_PARAMS] = {
+		[0] = {
+			.attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
+			.u.value.a = 0,
+		}
+	};
+	int ret;
+	bool has_support = false;
+
+	ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
+	if (ret < 0 || inv_arg.ret != 0) {
+		dev_err(dev,
+			"ICE wrapped_key_support failed, ret=%d, err=0x%x\n",
+			ret, inv_arg.ret);
+		goto exit;
+	}
+
+	has_support = param[0].u.value.a;
+exit:
+	return has_support;
+}
+
+static struct qcom_ice_tz_ops qcom_ice_tee_ops = {
+	.drv_name = "qcom-ice-tee",
+	.ice_invalidate_key = qcom_ice_tee_invalidate_key,
+	.ice_set_key = qcom_ice_tee_set_key,
+	.ice_has_wrapped_key_support = qcom_ice_tee_has_wrapped_key_support,
+	.ice_derive_sw_secret = qcom_ice_tee_derive_sw_secret,
+	.ice_generate_key = qcom_ice_tee_generate_key,
+	.ice_prepare_key = qcom_ice_tee_prepare_key,
+	.ice_import_key = qcom_ice_tee_import_key,
+};
+
+static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+	return ver->impl_id == TEE_IMPL_ID_OPTEE;
+}
+
+static int qcom_ice_tee_probe(struct tee_client_device *ice_dev)
+{
+	struct device *dev = &ice_dev->dev;
+	struct qcom_ice_tee_private *data;
+	struct tee_ioctl_open_session_arg sess_arg = {
+		.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL,
+	};
+	int ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
+	if (IS_ERR(data->ctx))
+		return -ENODEV;
+
+	export_uuid(sess_arg.uuid, &ice_dev->id.uuid);
+	ret = tee_client_open_session(data->ctx, &sess_arg, NULL);
+	if (ret < 0 || sess_arg.ret != 0) {
+		dev_err(dev,
+			"tee_client_open_session failed, ret=%d, err=0x%x\n",
+			ret, sess_arg.ret);
+		ret = ret ?: -EINVAL;
+		goto err_sess;
+	}
+
+	data->session_id = sess_arg.session;
+	dev_set_drvdata(dev, data);
+
+	qcom_ice_tee_ops.dev = dev;
+	ret = qcom_ice_tz_ops_register(&qcom_ice_tee_ops);
+	if (ret)
+		goto err_ops;
+
+	return 0;
+err_ops:
+	tee_client_close_session(data->ctx, data->session_id);
+err_sess:
+	tee_client_close_context(data->ctx);
+	return ret;
+}
+
+static void qcom_ice_tee_remove(struct tee_client_device *ice_dev)
+{
+	struct device *dev = &ice_dev->dev;
+	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
+
+	qcom_ice_tz_ops_unregister(&qcom_ice_tee_ops);
+	tee_client_close_session(data->ctx, data->session_id);
+	tee_client_close_context(data->ctx);
+}
+
+static const struct tee_client_device_id qcom_ice_tee_id_table[] = {
+	{UUID_INIT(0x29e87b9e, 0x012a, 0x4878,
+		   0xa1, 0xe1, 0xa1, 0xb9, 0x0a, 0x21, 0x5b, 0x16)},
+	{}
+};
+MODULE_DEVICE_TABLE(tee, qcom_ice_tee_id_table);
+
+static struct tee_client_driver qcom_ice_tee_driver = {
+	.probe		= qcom_ice_tee_probe,
+	.remove		= qcom_ice_tee_remove,
+	.id_table	= qcom_ice_tee_id_table,
+	.driver		= {
+		.name = "qcom-ice-tee",
+	},
+};
+
+module_tee_client_driver(qcom_ice_tee_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Qualcomm ICE TEE driver");

-- 
2.34.1

[PATCH v2 4/6] tee: optee: Register TEE devices on the platform bus

[Harshal Dev]

The Qualcomm Inline Crypto Engine (ICE) driver is an independent platform
driver which polls on availability of a TEE interface (SCM driver) before
checking if the ICE secure service is supported by QTEE by querying the
qcom_scm_ice_available() API.

The TEE subsystem currently does not provide any way to notify regarding
the availability of a TEE interface, i.e. whether QCOMTEE or OPTEE have
probed. And so when migrating to the TEE subsystem via an ICE TEE client
driver, ICE needs to defer probe until the OPTEE ICE Pseudo TA becomes
available on the TEE bus. This means that for OPTEE builds without the
ICE PTA, the ICE driver would indefintely defer probe and the dependent
UFS/SDHC storage driver will fail to come online.

Add support for optionally registering the back-end TEE devices which
represent a particular TEE (say OPTEE) on the platform bus via the
tee_platform_device_register() API. This allows drivers which poll on
TEE secure interface availability to be notified even if the TEE service
itself is not supported.

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 MAINTAINERS                                |  1 +
 drivers/firmware/qcom/qcom_ice_tee.c       | 41 ++++++++++++++++++++++++++++++
 drivers/soc/qcom/Kconfig                   |  1 +
 drivers/soc/qcom/ice.c                     |  9 ++++++-
 drivers/tee/optee/ffa_abi.c                |  8 ++++++
 drivers/tee/optee/smc_abi.c                |  8 ++++++
 drivers/tee/tee_core.c                     | 21 +++++++++++++++
 include/linux/firmware/qcom/qcom_ice_tee.h | 10 ++++++++
 include/linux/tee_core.h                   |  9 +++++++
 9 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1132e456488b..06dba76996b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23013,6 +23013,7 @@ L:	linux-arm-msm@vger.kernel.org
 L:	linux-crypto@vger.kernel.org
 S:	Maintained
 F:	drivers/firmware/qcom/qcom_ice_tee.c
+F:	include/linux/firmware/qcom/qcom_ice_tee.h
 
 QUALCOMM INTERCONNECT BWMON DRIVER
 M:	Krzysztof Kozlowski <krzk@kernel.org>
diff --git a/drivers/firmware/qcom/qcom_ice_tee.c b/drivers/firmware/qcom/qcom_ice_tee.c
index 694aa05847f0..39f7094439d9 100644
--- a/drivers/firmware/qcom/qcom_ice_tee.c
+++ b/drivers/firmware/qcom/qcom_ice_tee.c
@@ -5,6 +5,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/tee_drv.h>
 #include <linux/uuid.h>
@@ -22,6 +23,9 @@
 
 #define TEE_NUM_PARAMS				4
 
+/* This represents the back-end OPTEE device */
+static struct device *__teedev;
+
 struct qcom_ice_tee_private {
 	struct device *dev;
 	struct tee_context *ctx;
@@ -281,6 +285,18 @@ static int qcom_ice_tee_import_key(struct device *dev,
 					     lt_key, lt_key_size);
 }
 
+bool qcom_ice_tee_is_available(void)
+{
+	/*
+	 * Paired with smp_load_acquire() in qcom_ice_plat_probe().
+	 *
+	 * This marks the availability of the OPTEE interface informing the
+	 * polling ICE driver that the platform supports a TEE.
+	 */
+	return !!smp_load_acquire(&__teedev);
+}
+EXPORT_SYMBOL_GPL(qcom_ice_tee_is_available);
+
 static bool qcom_ice_tee_has_wrapped_key_support(struct device *dev)
 {
 	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
@@ -398,5 +414,30 @@ static struct tee_client_driver qcom_ice_tee_driver = {
 
 module_tee_client_driver(qcom_ice_tee_driver);
 
+static int qcom_ice_plat_probe(struct platform_device *pdev)
+{
+	/* Paired with smp_load_acquire() in qcom_ice_tee_is_available() */
+	smp_store_release(&__teedev, &pdev->dev);
+	return 0;
+}
+
+static const struct platform_device_id qcom_ice_plat_id_table[] = {
+	{
+		.name = "optee",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_ice_plat_id_table);
+
+static struct platform_driver qcom_ice_plat_driver = {
+	.probe = qcom_ice_plat_probe,
+	.id_table = qcom_ice_plat_id_table,
+	.driver = {
+		.name = "qcom-ice-plat",
+	},
+};
+
+module_platform_driver(qcom_ice_plat_driver);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Qualcomm ICE TEE driver");
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 535c8619197b..8dc75c252f52 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -296,6 +296,7 @@ endif
 config QCOM_INLINE_CRYPTO_ENGINE
 	tristate
 	select QCOM_SCM
+	select QCOM_ICE_TEE
 
 config QCOM_KRYO_L2_ACCESSORS
 	bool
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index 452896b1858f..4844621b7497 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/xarray.h>
 
+#include <linux/firmware/qcom/qcom_ice_tee.h>
 #include <linux/firmware/qcom/qcom_scm.h>
 
 #include <soc/qcom/ice.h>
@@ -132,7 +133,13 @@ static bool qcom_ice_tz_ops_registered(void)
 
 static bool qcom_ice_tz_is_available(void)
 {
-	return qcom_scm_is_available();
+	/*
+	 * Both back-ends should be available before we check for
+	 * qcom_ice_tz_ops_registered(), otherwise we could pre-maturely
+	 * assume that TZ does not provide any ICE secure interface.
+	 */
+	return (qcom_scm_is_available() & qcom_ice_tee_is_available());
+
 }
 
 static int qcom_ice_tz_invalidate_key(u32 index)
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index 702da7d98fbc..2c0065f3d9e4 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -1177,6 +1177,14 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
 	if (rc)
 		goto err_unregister_devices;
 
+	/*
+	 * Device should only be visible on the platform bus once the
+	 * child devices have already been enumerated on the TEE bus.
+	 */
+	rc = tee_platform_device_register(optee->teedev, "optee");
+	if (rc)
+		goto err_unregister_devices;
+
 	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
 	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
 	blocking_notifier_chain_register(&optee_rpmb_intf_added,
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index 51624443359b..85ee76173982 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -1920,6 +1920,14 @@ static int optee_probe(struct platform_device *pdev)
 	if (rc)
 		goto err_disable_shm_cache;
 
+	/*
+	 * Device should only be visible on the platform bus once the
+	 * child devices have already been enumerated on the TEE bus.
+	 */
+	rc = tee_platform_device_register(optee->teedev, "optee");
+	if (rc)
+		goto err_disable_shm_cache;
+
 	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
 	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
 	blocking_notifier_chain_register(&optee_rpmb_intf_added,
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 1aac50c7c1de..0d3c266b29c7 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -10,6 +10,7 @@
 #include <linux/fs.h>
 #include <linux/idr.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/overflow.h>
 #include <linux/slab.h>
 #include <linux/tee_core.h>
@@ -1225,6 +1226,26 @@ int tee_device_register(struct tee_device *teedev)
 }
 EXPORT_SYMBOL_GPL(tee_device_register);
 
+static void plat_tee_free(void *data)
+{
+	struct platform_device *plat_dev = data;
+
+	platform_device_unregister(plat_dev);
+}
+
+int tee_platform_device_register(struct tee_device *teedev, const char *teename)
+{
+	struct platform_device *plat_dev;
+
+	plat_dev = platform_device_register_data(&teedev->dev, teename,
+						 PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(plat_dev))
+		return PTR_ERR(plat_dev);
+
+	devm_add_action_or_reset(&teedev->dev, plat_tee_free, plat_dev);
+	return 0;
+}
+
 void tee_device_put(struct tee_device *teedev)
 {
 	mutex_lock(&teedev->mutex);
diff --git a/include/linux/firmware/qcom/qcom_ice_tee.h b/include/linux/firmware/qcom/qcom_ice_tee.h
new file mode 100644
index 000000000000..a5906c5d4e39
--- /dev/null
+++ b/include/linux/firmware/qcom/qcom_ice_tee.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#ifndef __QCOM_ICE_TEE
+#define __QCOM_ICE_TEE
+
+bool qcom_ice_tee_is_available(void);
+
+#endif /* __QCOM_ICE_TEE */
diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h
index f993d5118edd..d5310b404c1e 100644
--- a/include/linux/tee_core.h
+++ b/include/linux/tee_core.h
@@ -205,6 +205,15 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
  */
 int tee_device_register(struct tee_device *teedev);
 
+/**
+ * tee_platform_device_register() - Registers a TEE platform device
+ * @teedev:	Device to register
+ * @teename:	Name of the TEE the device represents
+ *
+ * @returns: < 0 on failure
+ */
+int tee_platform_device_register(struct tee_device *teedev, const char *teename);
+
 /**
  * tee_device_unregister() - Removes a TEE device
  * @teedev:	Device to unregister

-- 
2.34.1

[PATCH v2 5/6] soc: qcom: ice: Migrate to the TZ agnostic ICE APIs

[Harshal Dev]

Migrate the Inline Crypto Engine (ICE) driver to the TZ agnostic ICE APIs
which re-directs calls from the driver to either the SCM or the TEE bus
based back-end driver at runtime.

This makes the ICE driver agnostic of the TEE firmware (QTEE or OPTEE)
running on the Qualcomm platform.

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 drivers/soc/qcom/ice.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index 4844621b7497..9bab177f7ce5 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -307,7 +307,7 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
 	 * advertise support for raw keys or wrapped keys, HWKM cannot be used
 	 * unconditionally.  A module parameter is used to opt into using it.
 	 */
-	if (ice->hwkm_version && qcom_scm_has_wrapped_key_support()) {
+	if (ice->hwkm_version && qcom_ice_tz_has_wrapped_key_support()) {
 		if (qcom_ice_use_wrapped_keys) {
 			dev_info(dev, "Using HWKM. Supporting wrapped keys only.\n");
 			ice->use_hwkm = true;
@@ -483,7 +483,7 @@ static int qcom_ice_program_wrapped_key(struct qcom_ice *ice, unsigned int slot,
 	struct device *dev = ice->dev;
 	union crypto_cfg cfg = {
 		.dusize = bkey->crypto_cfg.data_unit_size / 512,
-		.capidx = QCOM_SCM_ICE_CIPHER_AES_256_XTS,
+		.capidx = QCOM_ICE_CIPHER_AES_256_XTS,
 		.cfge = QCOM_ICE_HWKM_CFG_ENABLE_VAL,
 	};
 	int err;
@@ -501,11 +501,11 @@ static int qcom_ice_program_wrapped_key(struct qcom_ice *ice, unsigned int slot,
 	qcom_ice_writel(ice, 0x0, QCOM_ICE_REG_CRYPTOCFG(slot));
 
 	/* Call into TrustZone to program the wrapped key using HWKM. */
-	err = qcom_scm_ice_set_key(translate_hwkm_slot(ice, slot), bkey->bytes,
-				   bkey->size, cfg.capidx, cfg.dusize);
+	err = qcom_ice_tz_set_key(translate_hwkm_slot(ice, slot), bkey->bytes,
+				  bkey->size, cfg.capidx, cfg.dusize);
 	if (err) {
 		dev_err_ratelimited(dev,
-				    "qcom_scm_ice_set_key failed; err=%d, slot=%u\n",
+				    "qcom_ice_tz_set_key failed; err=%d, slot=%u\n",
 				    err, slot);
 		return err;
 	}
@@ -553,9 +553,9 @@ int qcom_ice_program_key(struct qcom_ice *ice, unsigned int slot,
 	for (i = 0; i < ARRAY_SIZE(key.words); i++)
 		__cpu_to_be32s(&key.words[i]);
 
-	err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
-				   QCOM_SCM_ICE_CIPHER_AES_256_XTS,
-				   blk_key->crypto_cfg.data_unit_size / 512);
+	err = qcom_ice_tz_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
+				  QCOM_ICE_CIPHER_AES_256_XTS,
+				  blk_key->crypto_cfg.data_unit_size / 512);
 
 	memzero_explicit(&key, sizeof(key));
 
@@ -567,7 +567,7 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
 {
 	if (ice->hwkm_init_complete)
 		slot = translate_hwkm_slot(ice, slot);
-	return qcom_scm_ice_invalidate_key(slot);
+	return qcom_ice_tz_invalidate_key(slot);
 }
 EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
 
@@ -604,9 +604,9 @@ int qcom_ice_derive_sw_secret(struct qcom_ice *ice,
 			      const u8 *eph_key, size_t eph_key_size,
 			      u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
 {
-	int err = qcom_scm_derive_sw_secret(eph_key, eph_key_size,
-					    sw_secret,
-					    BLK_CRYPTO_SW_SECRET_SIZE);
+	int err = qcom_ice_tz_derive_sw_secret(eph_key, eph_key_size,
+					       sw_secret,
+					       BLK_CRYPTO_SW_SECRET_SIZE);
 	if (err == -EIO || err == -EINVAL)
 		err = -EBADMSG; /* probably invalid key */
 	return err;
@@ -627,8 +627,8 @@ int qcom_ice_generate_key(struct qcom_ice *ice,
 {
 	int err;
 
-	err = qcom_scm_generate_ice_key(lt_key,
-					QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+	err = qcom_ice_tz_generate_key(lt_key,
+				       QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
 	if (err)
 		return err;
 
@@ -654,8 +654,8 @@ int qcom_ice_prepare_key(struct qcom_ice *ice,
 {
 	int err;
 
-	err = qcom_scm_prepare_ice_key(lt_key, lt_key_size,
-				       eph_key, QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+	err = qcom_ice_tz_prepare_key(lt_key, lt_key_size,
+				      eph_key, QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
 	if (err == -EIO || err == -EINVAL)
 		err = -EBADMSG; /* probably invalid key */
 	if (err)
@@ -682,8 +682,8 @@ int qcom_ice_import_key(struct qcom_ice *ice,
 {
 	int err;
 
-	err = qcom_scm_import_ice_key(raw_key, raw_key_size,
-				      lt_key, QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+	err = qcom_ice_tz_import_key(raw_key, raw_key_size,
+				     lt_key, QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
 	if (err)
 		return err;
 
@@ -696,11 +696,11 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 {
 	struct qcom_ice *engine;
 
-	if (!qcom_scm_is_available())
+	if (!qcom_ice_tz_is_available())
 		return ERR_PTR(-EPROBE_DEFER);
 
-	if (!qcom_scm_ice_available()) {
-		dev_warn(dev, "ICE SCM interface not found\n");
+	if (!qcom_ice_tz_ops_registered()) {
+		dev_warn(dev, "ICE firmware service interface not found\n");
 		return ERR_PTR(-EOPNOTSUPP);
 	}
 

-- 
2.34.1

[PATCH v2 6/6] firmware: qcom: scm: Drop exported ICE wrapper functions

[Harshal Dev]

Drop the exported ICE wrapper functions from the SCM driver since the ICE
driver has now migrated to the TZ agnostic ICE APIs which allow runtime
routing of ICE secure calls to the appropriate TEE (QTEE or OPTEE).

Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       | 117 +++++++++------------------------
 include/linux/firmware/qcom/qcom_scm.h |  20 ------
 2 files changed, 31 insertions(+), 106 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 6b66bcedc84f..277334f71178 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1399,25 +1399,20 @@ int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_ocmem_unlock);
 
-bool __qcom_scm_ice_available(struct device *dev)
-{
-	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
-					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
-		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
-					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
-}
-
 /**
  * qcom_scm_ice_available() - Is the ICE key programming interface available?
+ * @dev: The SCM device
  *
  * Return: true iff the SCM calls wrapped by qcom_scm_ice_invalidate_key() and
  *	   qcom_scm_ice_set_key() are available.
  */
-bool qcom_scm_ice_available(void)
+static bool qcom_scm_ice_available(struct device *dev)
 {
-	return __qcom_scm_ice_available(__scm->dev);
+	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
+					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
+		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
+					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
 }
-EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
 
 /**
  * qcom_scm_ice_invalidate_key() - Invalidate an inline encryption key
@@ -1431,7 +1426,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
+static int qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1444,12 +1439,6 @@ static int __qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
 	return qcom_scm_call(dev, &desc, NULL);
 }
 
-int qcom_scm_ice_invalidate_key(u32 index)
-{
-	return __qcom_scm_ice_invalidate_key(__scm->dev, index);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_ice_invalidate_key);
-
 /**
  * qcom_scm_ice_set_key() - Set an inline encryption key
  * @index: the keyslot into which to set the key
@@ -1471,10 +1460,10 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_invalidate_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_ice_set_key(struct device *dev, u32 index,
-				  const u8 *key, u32 key_size,
-				  enum qcom_ice_cipher cipher,
-				  u32 data_unit_size)
+static int qcom_scm_ice_set_key(struct device *dev, u32 index,
+				const u8 *key, u32 key_size,
+				enum qcom_ice_cipher cipher,
+				u32 data_unit_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1506,16 +1495,7 @@ static int __qcom_scm_ice_set_key(struct device *dev, u32 index,
 	return ret;
 }
 
-int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
-			 enum qcom_scm_ice_cipher cipher, u32 data_unit_size)
-{
-	return __qcom_scm_ice_set_key(__scm->dev, index, key, key_size,
-				      (enum qcom_ice_cipher)cipher,
-				      data_unit_size);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
-
-static bool __qcom_scm_has_wrapped_key_support(struct device *dev)
+static bool qcom_scm_has_wrapped_key_support(struct device *dev)
 {
 	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
 					    QCOM_SCM_ES_DERIVE_SW_SECRET) &&
@@ -1527,12 +1507,6 @@ static bool __qcom_scm_has_wrapped_key_support(struct device *dev)
 					    QCOM_SCM_ES_IMPORT_ICE_KEY);
 }
 
-bool qcom_scm_has_wrapped_key_support(void)
-{
-	return __qcom_scm_has_wrapped_key_support(__scm->dev);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_has_wrapped_key_support);
-
 /**
  * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key
  * @eph_key: an ephemerally-wrapped key
@@ -1549,9 +1523,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_has_wrapped_key_support);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_derive_sw_secret(struct device *dev,
-				       const u8 *eph_key, size_t eph_key_size,
-				       u8 *sw_secret, size_t sw_secret_size)
+static int qcom_scm_derive_sw_secret(struct device *dev,
+				     const u8 *eph_key, size_t eph_key_size,
+				     u8 *sw_secret, size_t sw_secret_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1589,13 +1563,6 @@ static int __qcom_scm_derive_sw_secret(struct device *dev,
 	return ret;
 }
 
-int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
-			      u8 *sw_secret, size_t sw_secret_size)
-{
-	return __qcom_scm_derive_sw_secret(__scm->dev, eph_key, eph_key_size,
-				   sw_secret, sw_secret_size);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret);
 
 /**
  * qcom_scm_generate_ice_key() - Generate a wrapped key for storage encryption
@@ -1608,8 +1575,8 @@ EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_generate_ice_key(struct device *dev,
-				       u8 *lt_key, size_t lt_key_size)
+static int qcom_scm_generate_ice_key(struct device *dev,
+				     u8 *lt_key, size_t lt_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1636,12 +1603,6 @@ static int __qcom_scm_generate_ice_key(struct device *dev,
 	return ret;
 }
 
-int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
-{
-	return __qcom_scm_generate_ice_key(__scm->dev, lt_key, lt_key_size);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
-
 /**
  * qcom_scm_prepare_ice_key() - Re-wrap a key with the per-boot ephemeral key
  * @lt_key: a long-term wrapped key
@@ -1655,9 +1616,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_prepare_ice_key(struct device *dev,
-				      const u8 *lt_key, size_t lt_key_size,
-				      u8 *eph_key, size_t eph_key_size)
+static int qcom_scm_prepare_ice_key(struct device *dev,
+				    const u8 *lt_key, size_t lt_key_size,
+				    u8 *eph_key, size_t eph_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1695,14 +1656,6 @@ static int __qcom_scm_prepare_ice_key(struct device *dev,
 	return ret;
 }
 
-int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
-			     u8 *eph_key, size_t eph_key_size)
-{
-	return __qcom_scm_prepare_ice_key(__scm->dev, lt_key, lt_key_size,
-				  eph_key, eph_key_size);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
-
 /**
  * qcom_scm_import_ice_key() - Import key for storage encryption
  * @raw_key: the raw key to import
@@ -1716,9 +1669,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
  *
  * Return: 0 on success; -errno on failure.
  */
-static int __qcom_scm_import_ice_key(struct device *dev,
-				     const u8 *raw_key, size_t raw_key_size,
-				     u8 *lt_key, size_t lt_key_size)
+static int qcom_scm_import_ice_key(struct device *dev,
+				   const u8 *raw_key, size_t raw_key_size,
+				   u8 *lt_key, size_t lt_key_size)
 {
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_ES,
@@ -1756,23 +1709,15 @@ static int __qcom_scm_import_ice_key(struct device *dev,
 	return ret;
 }
 
-int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
-			    u8 *lt_key, size_t lt_key_size)
-{
-	return __qcom_scm_import_ice_key(__scm->dev, raw_key, raw_key_size,
-				 lt_key, lt_key_size);
-}
-EXPORT_SYMBOL_GPL(qcom_scm_import_ice_key);
-
 static struct qcom_ice_tz_ops qcom_ice_scm_ops = {
 	.drv_name = "qcom_scm",
-	.ice_invalidate_key = __qcom_scm_ice_invalidate_key,
-	.ice_set_key = __qcom_scm_ice_set_key,
-	.ice_has_wrapped_key_support = __qcom_scm_has_wrapped_key_support,
-	.ice_derive_sw_secret = __qcom_scm_derive_sw_secret,
-	.ice_generate_key = __qcom_scm_generate_ice_key,
-	.ice_prepare_key = __qcom_scm_prepare_ice_key,
-	.ice_import_key	= __qcom_scm_import_ice_key,
+	.ice_invalidate_key = qcom_scm_ice_invalidate_key,
+	.ice_set_key = qcom_scm_ice_set_key,
+	.ice_has_wrapped_key_support = qcom_scm_has_wrapped_key_support,
+	.ice_derive_sw_secret = qcom_scm_derive_sw_secret,
+	.ice_generate_key = qcom_scm_generate_ice_key,
+	.ice_prepare_key = qcom_scm_prepare_ice_key,
+	.ice_import_key = qcom_scm_import_ice_key,
 };
 
 /**
@@ -2894,7 +2839,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	 * ICE driver could attempt to call the ICE SCM pointers before
 	 * they are registered and assume that they aren't available.
 	 */
-	if (__qcom_scm_ice_available(scm->dev)) {
+	if (qcom_scm_ice_available(scm->dev)) {
 		qcom_ice_scm_ops.dev = scm->dev;
 		ret = qcom_ice_tz_ops_register(&qcom_ice_scm_ops);
 		WARN(ret < 0, "failed to register ice SCM ops: %d\n", ret);
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index a0a6bc0229c4..36b87be82804 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -46,13 +46,6 @@ enum qcom_scm_sec_dev_id {
 	QCOM_SCM_ICE_DEV_ID     = 20,
 };
 
-enum qcom_scm_ice_cipher {
-	QCOM_SCM_ICE_CIPHER_AES_128_XTS = 0,
-	QCOM_SCM_ICE_CIPHER_AES_128_CBC = 1,
-	QCOM_SCM_ICE_CIPHER_AES_256_XTS = 3,
-	QCOM_SCM_ICE_CIPHER_AES_256_CBC = 4,
-};
-
 #define QCOM_SCM_PERM_READ       0x4
 #define QCOM_SCM_PERM_WRITE      0x2
 #define QCOM_SCM_PERM_EXEC       0x1
@@ -86,19 +79,6 @@ int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset, u32 size,
 			u32 mode);
 int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size);
 
-bool qcom_scm_ice_available(void);
-int qcom_scm_ice_invalidate_key(u32 index);
-int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
-			 enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
-bool qcom_scm_has_wrapped_key_support(void);
-int qcom_scm_derive_sw_secret(const u8 *eph_key, size_t eph_key_size,
-			      u8 *sw_secret, size_t sw_secret_size);
-int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size);
-int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
-			     u8 *eph_key, size_t eph_key_size);
-int qcom_scm_import_ice_key(const u8 *raw_key, size_t raw_key_size,
-			    u8 *lt_key, size_t lt_key_size);
-
 bool qcom_scm_hdcp_available(void);
 int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
 

-- 
2.34.1

Re: [PATCH v2 0/6] Add support for a TEE agnostic Inline Crypto Engine service

[Harshal Dev]

+ op-tee@lists.trustedfirmware.org, Jens Wiklander <jens.wiklander@oss.qualcomm.com>

On 11-11-2026 07:47 pm, Harshal Dev wrote:
> Qualcomm platforms such as the IQ-9075 EVK now boot with both Qualcomm
> Trusted Execution Environment (QTEE) and OP-TEE [1]. However, the Qualcomm
> Inline Crypto Engine (ICE) driver currently only makes SCM calls into QTEE.
> With the arrival of ICE support on OPTEE [2,3] for Qualcomm platforms,
> there is a need for TrustZone (TZ) agnostic APIs which re-direct the call
> to the correct TZ at runtime to request programming of the secure interfaces
> on the ICE hardware block.
>     
> This patch series adds such TZ agnostic APIs to the ICE driver for
> requesting secure services from either QTEE or OP-TEE depending on whether
> an SCM driver back-end or a TEE client driver back-end is registered with
> it at runtime. This series is logically similar to the one introduced for
> Peripheral Authentication Service (PAS) [3] which was recently merged to
> provide a generic PAS service for both QTEE and OP-TEE.
> 
> Apart from introducing a TEE bus based client driver for ICE, this series
> also adds support for optionally registering a TEE device which represents
> a particular TEE (QTEE or OPTEE) on the platform bus to enable drivers such
> as ICE which poll on the availability of a TEE interface to be notified.
> 
> The ICE driver is an independent platform driver which polls on the
> availability of a TEE interface (SCM driver) before checking if the ICE
> secure service is supported by QTEE by querying qcom_scm_is_available().
> The TEE subsystem currently does not provide any mechanism to notify the
> availability of a TEE interface, i.e. whether QCOMTEE or OPTEE have probed.
> Without the introduction of such a notification mechanism the ICE driver
> would indefinitely defer probe until the OPTEE ICE Pseudo TA becomes available
> on the TEE bus implying that for OPTEE builds without the ICE PTA enabled,
> the dependent UFS/SDHC storage driver will fail to come online.
> 
> The TEE bus based client driver introduced in this series which registers
> with the ICE driver has an ABI with OP-TEE that is 1-1 identical with the
> SCM driver's ICE ABI with QTEE. This is because both TEEs implement logically
> similar drivers with the exception that OP-TEE currently does not have
> Hardware Key Manager v2 support and so the inline encryption functionality
> with OP-TEE is currently limited to Qualcomm platforms with only Hardware
> Key Manager v1.
> 
> Validation:
> This series has been validated on a LeMans IQ-9075 EVK device by attempting
> file system encryption via the fscrypt tool.
> 
> Merge Strategy:
> All patches in this series except one touch Qualcomm firmware and soc drivers.
> Hence, it should go via the QCOM-SoC tree. Request the TEE subsystem maintainers
> to provide an Acknowledgement for this.
> 
> [1] https://ldts.github.io/qcom-buildroot/index.html
> [2] https://github.com/OP-TEE/optee_os/pull/7906 (In review)
> [3] https://github.com/OP-TEE/optee_os/pull/8137 (Draft, depends on PR#7906)
> [4] https://lore.kernel.org/all/20260702115835.167602-1-sumit.garg@kernel.org/
> 
> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
> ---
> Changes in v2:
> - Merged the furnctionality of the middleware ICE service into the ICE
>   driver as requested by Eric.
> - Introduced tee_platform_device_register() to fix the issue of indefinite
>   probe defer in the ICE driver when ICE PTA is not enabled in OPTEE as
>   pointed out by Eric.
> - Fixed type "Psuedo" in ICE TEE back-end commit.
> - Fixed space based indentation with tab in ICE back-end Kconfig.
> - Ensure tee_shm_get_va() failure is checked and ERR_PTR() returned.
> - Validate out_shm size everywhere in qcom_ice_tee.c
> - Zero the memory holding the key when TEE invoke call fails.
> - Check failure from qcom_ice_tz_ops_register() during probe.
> - Cross-check out_size and lt_key_size in qcom_ice_tee_generate_key().
> - Always return 0 when qcom_ice_tee_probe() is success.
> - Did not collect tags from Abel since significant rework undertaken.
> - Link to v1: https://lore.kernel.org/r/20261021-ice-migrate-tee-svc-v1-0-b035be7aab0a@oss.qualcomm.com
> 
> ---
> Harshal Dev (6):
>       soc: qcom: ice: Add TZ agnostic re-direction functions
>       firmware: qcom_scm: Transition to ICE TZ agnostic operations
>       firmware: qcom: Add a TEE bus based ICE back-end
>       tee: optee: Register TEE devices on the platform bus
>       soc: qcom: ice: Migrate to the TZ agnostic ICE APIs
>       firmware: qcom: scm: Drop exported ICE wrapper functions
> 
>  MAINTAINERS                                |   8 +
>  drivers/firmware/qcom/Kconfig              |   9 +
>  drivers/firmware/qcom/Makefile             |   1 +
>  drivers/firmware/qcom/qcom_ice_tee.c       | 443 +++++++++++++++++++++++++++++
>  drivers/firmware/qcom/qcom_scm.c           |  90 +++---
>  drivers/soc/qcom/Kconfig                   |   1 +
>  drivers/soc/qcom/ice.c                     | 173 +++++++++--
>  drivers/tee/optee/ffa_abi.c                |   8 +
>  drivers/tee/optee/smc_abi.c                |   8 +
>  drivers/tee/tee_core.c                     |  21 ++
>  include/linux/firmware/qcom/qcom_ice_tee.h |  10 +
>  include/linux/firmware/qcom/qcom_scm.h     |  20 --
>  include/linux/tee_core.h                   |   9 +
>  include/soc/qcom/ice.h                     |  56 ++++
>  14 files changed, 784 insertions(+), 73 deletions(-)
> ---
> base-commit: 8f7df005a7564bd7ffa75da9c3085d5b996f1e16
> change-id: 20261021-ice-migrate-tee-svc-a5cd6959ae36
> 
> Best regards,

Re: [PATCH v2 1/6] soc: qcom: ice: Add TZ agnostic re-direction functions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] soc: qcom: ice: broken CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m guard
  in ice.h
- [High] soc: qcom: ice: broken TZ availability check in
  qcom_ice_tz_is_available()
- [Medium] soc: qcom: ice: qcom_ice_tz_has_wrapped_key_support() returns
  -ENODEV as bool
- [Low] soc: qcom: ice: non-atomic check-then-store in
  qcom_ice_tz_ops_register()
--

commit aaede1b942c524f288e35421c3d46bb0baa718ed
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

soc: qcom: ice: Add TZ agnostic re-direction functions

This commit adds TrustZone-agnostic redirection functions and a backend
operations registration interface so the Qualcomm ICE driver can route calls
to either QTEE SCM or an OP-TEE backend.

> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index e256595b9826f..452896b1858f1 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -118,6 +118,130 @@ struct qcom_ice {
>  static DEFINE_XARRAY(ice_handles);
>  static DEFINE_MUTEX(ice_mutex);
>  
> +static struct qcom_ice_tz_ops *ice_tz_ops;
> +
> +static bool qcom_ice_tz_ops_registered(void)
> +{
> +	/*
> +	 * The barrier is intended to synchronize the data stores for the
> +	 * ice_tz_ops data structure when client drivers (UFS/SDHC) are in
> +	 * parallel checking for ICE service availability.
> +	 */
> +	return !!smp_load_acquire(&ice_tz_ops);
> +}
> +
> +static bool qcom_ice_tz_is_available(void)
> +{
> +	return qcom_scm_is_available();
> +}

[Severity: High]
As reported by Eric Biggers on v1 [1], does qcom_ice_tz_is_available() cause
qcom_ice_create() to return -EPROBE_DEFER indefinitely on QTEE-only
platforms?

Later in this series, commit "tee: optee: Register TEE devices on the
platform bus" updates qcom_ice_tz_is_available() to return
(qcom_scm_is_available() & qcom_ice_tee_is_available()), and commit
"soc: qcom: ice: Migrate to the TZ agnostic ICE APIs" makes
qcom_ice_create() return -EPROBE_DEFER when qcom_ice_tz_is_available() is
false before checking qcom_ice_tz_ops_registered().  Because
qcom_ice_tee_is_available() only becomes true after an OP-TEE platform
device probes, it stays false on QTEE-only platforms (or when CONFIG_OPTEE
is disabled).  Since ufs_qcom_ice_init() and sdhci_msm_ice_init() only fall
back to running without inline encryption on -EOPNOTSUPP and propagate
-EPROBE_DEFER, UFS and eMMC/SDHC controllers never finish probing on those
platforms.

[ ... ]

> +static bool qcom_ice_tz_has_wrapped_key_support(void)
> +{
> +	struct qcom_ice_tz_ops *ops;
> +
> +	ops = ice_tz_ops;
> +	if (!ops)
> +		return -ENODEV;
> +
> +	return ops->ice_has_wrapped_key_support(ops->dev);
> +}

[Severity: Medium]
Does returning -ENODEV from a bool function evaluate to true when
ice_tz_ops is NULL?  Besides triggering a -Wint-in-bool-context compiler
warning, this reports that wrapped keys are supported when no TZ backend is
registered.

[ ... ]

> +int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops)
> +{
> +	if (!qcom_ice_tz_ops_registered())
> +		/* Paired with smp_load_acquire() in qcom_ice_tz_ops_registered() */
> +		smp_store_release(&ice_tz_ops, ops);
> +	else
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_tz_ops_register);
> +
> +void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops)
> +{
> +	if (READ_ONCE(ice_tz_ops) == ops)
> +		/* Paired with smp_load_acquire() in qcom_ice_tz_ops_registered() */
> +		smp_store_release(&ice_tz_ops, NULL);
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_tz_ops_unregister);

[Severity: Low]
Can two backends probing in parallel on the platform and TEE buses both pass
!qcom_ice_tz_ops_registered() and overwrite ice_tz_ops instead of returning
-EBUSY?  Using cmpxchg() or a mutex across register and unregister would make
the check and update atomic.

> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 4bee553f0a59d..89e62e151b85b 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h

[ ... ]

> @@ -31,4 +72,19 @@ int qcom_ice_import_key(struct qcom_ice *ice,
>  			u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
>  struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
>  
> +#ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
> +int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops);
> +void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops);
> +#else
> +static inline int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *__unused)
> +{
> +	return 0;
> +}
> +
> +static inline
> +void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *__unused)
> +{
> +}
> +#endif /* CONFIG_QCOM_INLINE_CRYPTO_ENGINE */
> +
>  #endif /* __QCOM_ICE_H__ */

[Severity: High]
Does #ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE break the build when
CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m?

QCOM_INLINE_CRYPTO_ENGINE is a tristate symbol (and defaults to =m in arm64
defconfig), so when built as a module only
CONFIG_QCOM_INLINE_CRYPTO_ENGINE_MODULE is defined.  The header then exposes
the static inline stubs, which conflict with the non-static definitions in
drivers/soc/qcom/ice.c and cause TZ backends to call no-op stubs.  Also, if
CONFIG_QCOM_SCM=y while CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m, built-in
qcom_scm cannot link against symbols exported by qcom_ice.ko.

[1] https://lore.kernel.org/r/20261024185602.GF2553@quark

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=1

Re: [PATCH v2 2/6] firmware: qcom_scm: Transition to ICE TZ agnostic operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] firmware: qcom_scm: broken qcom_ice_tz_ops_register() linkage
  when ICE=m
- [Medium] firmware: qcom_scm: missing static on
  __qcom_scm_ice_available()
- [Low] firmware: qcom_scm: kernel-doc mismatch on static
  __qcom_scm_ice_*()
--

commit 8abba45f5b733420aae4f360c4bcadfad9c878ba
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

firmware: qcom_scm: Transition to ICE TZ agnostic operations

This commit transitions the ICE-related SCM calls to static helpers
registered as a qcom_ice_tz_ops backend while keeping the existing
exported wrappers in place.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 43e250faea75a..6b66bcedc84fd 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
[ ... ]
> @@ -1397,6 +1399,14 @@ int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_ocmem_unlock);
>  
> +bool __qcom_scm_ice_available(struct device *dev)
> +{
> +	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> +					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
> +		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> +					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
> +}

[Severity: Medium]
Should __qcom_scm_ice_available() be marked static?

It is only used within drivers/firmware/qcom/qcom_scm.c and has no prior
prototype declaration, which triggers a -Wmissing-prototypes warning and
breaks CONFIG_WERROR=y builds at this commit until commit "firmware: qcom:
scm: Drop exported ICE wrapper functions" removes the helper later in the
series.

> @@ -1424,7 +1431,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
>   *
>   * Return: 0 on success; -errno on failure.
>   */
> -int qcom_scm_ice_invalidate_key(u32 index)
> +static int __qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
>  {
>  	struct qcom_scm_desc desc = {
>  		.svc = QCOM_SCM_SVC_ES,
> @@ -1434,7 +1441,12 @@ int qcom_scm_ice_invalidate_key(u32 index)
>  		.owner = ARM_SMCCC_OWNER_SIP,
>  	};
>  
> -	return qcom_scm_call(__scm->dev, &desc, NULL);
> +	return qcom_scm_call(dev, &desc, NULL);
> +}
> +
> +int qcom_scm_ice_invalidate_key(u32 index)
> +{
> +	return __qcom_scm_ice_invalidate_key(__scm->dev, index);
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_ice_invalidate_key);

[Severity: Low]
Does leaving the kernel-doc comments above __qcom_scm_ice_invalidate_key()
and the five other __qcom_scm_*() helpers cause scripts/kernel-doc warnings?

Each comment still documents the exported qcom_scm_*() symbol name and
omits the new @dev parameter on __qcom_scm_ice_invalidate_key(),
__qcom_scm_ice_set_key(), __qcom_scm_derive_sw_secret(),
__qcom_scm_generate_ice_key(), __qcom_scm_prepare_ice_key(), and
__qcom_scm_import_ice_key().  Later in the series, commit "firmware: qcom:
scm: Drop exported ICE wrapper functions" renames these static helpers
back to qcom_scm_*(), but @dev remains undocumented in all six kernel-doc
blocks.

[ ... ]

> @@ -2820,6 +2888,18 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  					     "Failed to request qcom-scm irq\n");
>  	}
>  
> +	/*
> +	 * If the ICE SCM interface is available, the operations MUST be
> +	 * registered before the SCM API is published below, otherwise
> +	 * ICE driver could attempt to call the ICE SCM pointers before
> +	 * they are registered and assume that they aren't available.
> +	 */
> +	if (__qcom_scm_ice_available(scm->dev)) {
> +		qcom_ice_scm_ops.dev = scm->dev;
> +		ret = qcom_ice_tz_ops_register(&qcom_ice_scm_ops);
> +		WARN(ret < 0, "failed to register ice SCM ops: %d\n", ret);
> +	}
> +
>  	/*
>  	 * Paired with smp_load_acquire() in qcom_scm_is_available().
>  	 *
> @@ -2876,6 +2956,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev)
>  	/* Clean shutdown, disable download mode to allow normal restart */
>  	qcom_scm_set_download_mode(__scm, QCOM_DLOAD_NODUMP);
>  	qcom_pas_ops_unregister();
> +	qcom_ice_tz_ops_unregister(&qcom_ice_scm_ops);
>  }

[Severity: High]
Does qcom_scm_probe() silently call the no-op stub for
qcom_ice_tz_ops_register() when CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m?

In include/soc/qcom/ice.h, the declarations are guarded with #ifdef rather
than IS_ENABLED():

include/soc/qcom/ice.h:
    #ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
    int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops);
    void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops);
    #else
    static inline int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *__unused)
    {
    	return 0;
    }

Because QCOM_INLINE_CRYPTO_ENGINE is a tristate symbol (and defaults to =m
in arm64 defconfig via CONFIG_SCSI_UFS_QCOM=m while CONFIG_QCOM_SCM=y),
qcom_scm_probe() and qcom_scm_shutdown() compile against the static inline
stubs, so qcom_ice_scm_ops is never registered and inline encryption fails
once the ICE driver checks qcom_ice_tz_ops_registered().

Even with IS_ENABLED(CONFIG_QCOM_INLINE_CRYPTO_ENGINE), CONFIG_QCOM_SCM=y
with CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m fails to link vmlinux against
qcom_ice_tz_ops_register() and qcom_ice_tz_ops_unregister() in qcom_ice.ko,
and building both as modules creates a circular dependency between
qcom-scm.ko and qcom_ice.ko while qcom_ice still calls qcom_scm_is_available().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=2

Re: [PATCH v2 4/6] tee: optee: Register TEE devices on the platform bus

[Harshal Dev]

+ op-tee@lists.trustedfirmware.org, Jens Wiklander <jens.wiklander@oss.qualcomm.com>

On 11-11-2026 07:47 pm, Harshal Dev wrote:
> The Qualcomm Inline Crypto Engine (ICE) driver is an independent platform
> driver which polls on availability of a TEE interface (SCM driver) before
> checking if the ICE secure service is supported by QTEE by querying the
> qcom_scm_ice_available() API.
> 
> The TEE subsystem currently does not provide any way to notify regarding
> the availability of a TEE interface, i.e. whether QCOMTEE or OPTEE have
> probed. And so when migrating to the TEE subsystem via an ICE TEE client
> driver, ICE needs to defer probe until the OPTEE ICE Pseudo TA becomes
> available on the TEE bus. This means that for OPTEE builds without the
> ICE PTA, the ICE driver would indefintely defer probe and the dependent
> UFS/SDHC storage driver will fail to come online.
> 
> Add support for optionally registering the back-end TEE devices which
> represent a particular TEE (say OPTEE) on the platform bus via the
> tee_platform_device_register() API. This allows drivers which poll on
> TEE secure interface availability to be notified even if the TEE service
> itself is not supported.
> 
> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
> ---
>  MAINTAINERS                                |  1 +
>  drivers/firmware/qcom/qcom_ice_tee.c       | 41 ++++++++++++++++++++++++++++++
>  drivers/soc/qcom/Kconfig                   |  1 +
>  drivers/soc/qcom/ice.c                     |  9 ++++++-
>  drivers/tee/optee/ffa_abi.c                |  8 ++++++
>  drivers/tee/optee/smc_abi.c                |  8 ++++++
>  drivers/tee/tee_core.c                     | 21 +++++++++++++++
>  include/linux/firmware/qcom/qcom_ice_tee.h | 10 ++++++++
>  include/linux/tee_core.h                   |  9 +++++++
>  9 files changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1132e456488b..06dba76996b8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23013,6 +23013,7 @@ L:	linux-arm-msm@vger.kernel.org
>  L:	linux-crypto@vger.kernel.org
>  S:	Maintained
>  F:	drivers/firmware/qcom/qcom_ice_tee.c
> +F:	include/linux/firmware/qcom/qcom_ice_tee.h
>  
>  QUALCOMM INTERCONNECT BWMON DRIVER
>  M:	Krzysztof Kozlowski <krzk@kernel.org>
> diff --git a/drivers/firmware/qcom/qcom_ice_tee.c b/drivers/firmware/qcom/qcom_ice_tee.c
> index 694aa05847f0..39f7094439d9 100644
> --- a/drivers/firmware/qcom/qcom_ice_tee.c
> +++ b/drivers/firmware/qcom/qcom_ice_tee.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/tee_drv.h>
>  #include <linux/uuid.h>
> @@ -22,6 +23,9 @@
>  
>  #define TEE_NUM_PARAMS				4
>  
> +/* This represents the back-end OPTEE device */
> +static struct device *__teedev;
> +
>  struct qcom_ice_tee_private {
>  	struct device *dev;
>  	struct tee_context *ctx;
> @@ -281,6 +285,18 @@ static int qcom_ice_tee_import_key(struct device *dev,
>  					     lt_key, lt_key_size);
>  }
>  
> +bool qcom_ice_tee_is_available(void)
> +{
> +	/*
> +	 * Paired with smp_load_acquire() in qcom_ice_plat_probe().
> +	 *
> +	 * This marks the availability of the OPTEE interface informing the
> +	 * polling ICE driver that the platform supports a TEE.
> +	 */
> +	return !!smp_load_acquire(&__teedev);
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_tee_is_available);
> +
>  static bool qcom_ice_tee_has_wrapped_key_support(struct device *dev)
>  {
>  	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
> @@ -398,5 +414,30 @@ static struct tee_client_driver qcom_ice_tee_driver = {
>  
>  module_tee_client_driver(qcom_ice_tee_driver);
>  
> +static int qcom_ice_plat_probe(struct platform_device *pdev)
> +{
> +	/* Paired with smp_load_acquire() in qcom_ice_tee_is_available() */
> +	smp_store_release(&__teedev, &pdev->dev);
> +	return 0;
> +}
> +
> +static const struct platform_device_id qcom_ice_plat_id_table[] = {
> +	{
> +		.name = "optee",
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, qcom_ice_plat_id_table);
> +
> +static struct platform_driver qcom_ice_plat_driver = {
> +	.probe = qcom_ice_plat_probe,
> +	.id_table = qcom_ice_plat_id_table,
> +	.driver = {
> +		.name = "qcom-ice-plat",
> +	},
> +};
> +
> +module_platform_driver(qcom_ice_plat_driver);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Qualcomm ICE TEE driver");
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 535c8619197b..8dc75c252f52 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -296,6 +296,7 @@ endif
>  config QCOM_INLINE_CRYPTO_ENGINE
>  	tristate
>  	select QCOM_SCM
> +	select QCOM_ICE_TEE
>  
>  config QCOM_KRYO_L2_ACCESSORS
>  	bool
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 452896b1858f..4844621b7497 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/xarray.h>
>  
> +#include <linux/firmware/qcom/qcom_ice_tee.h>
>  #include <linux/firmware/qcom/qcom_scm.h>
>  
>  #include <soc/qcom/ice.h>
> @@ -132,7 +133,13 @@ static bool qcom_ice_tz_ops_registered(void)
>  
>  static bool qcom_ice_tz_is_available(void)
>  {
> -	return qcom_scm_is_available();
> +	/*
> +	 * Both back-ends should be available before we check for
> +	 * qcom_ice_tz_ops_registered(), otherwise we could pre-maturely
> +	 * assume that TZ does not provide any ICE secure interface.
> +	 */
> +	return (qcom_scm_is_available() & qcom_ice_tee_is_available());
> +
>  }
>  
>  static int qcom_ice_tz_invalidate_key(u32 index)
> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index 702da7d98fbc..2c0065f3d9e4 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -1177,6 +1177,14 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  	if (rc)
>  		goto err_unregister_devices;
>  
> +	/*
> +	 * Device should only be visible on the platform bus once the
> +	 * child devices have already been enumerated on the TEE bus.
> +	 */
> +	rc = tee_platform_device_register(optee->teedev, "optee");
> +	if (rc)
> +		goto err_unregister_devices;
> +
>  	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
>  	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
>  	blocking_notifier_chain_register(&optee_rpmb_intf_added,
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index 51624443359b..85ee76173982 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -1920,6 +1920,14 @@ static int optee_probe(struct platform_device *pdev)
>  	if (rc)
>  		goto err_disable_shm_cache;
>  
> +	/*
> +	 * Device should only be visible on the platform bus once the
> +	 * child devices have already been enumerated on the TEE bus.
> +	 */
> +	rc = tee_platform_device_register(optee->teedev, "optee");
> +	if (rc)
> +		goto err_disable_shm_cache;
> +
>  	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
>  	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
>  	blocking_notifier_chain_register(&optee_rpmb_intf_added,
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 1aac50c7c1de..0d3c266b29c7 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -10,6 +10,7 @@
>  #include <linux/fs.h>
>  #include <linux/idr.h>
>  #include <linux/module.h>
> +#include <linux/platform_device.h>
>  #include <linux/overflow.h>
>  #include <linux/slab.h>
>  #include <linux/tee_core.h>
> @@ -1225,6 +1226,26 @@ int tee_device_register(struct tee_device *teedev)
>  }
>  EXPORT_SYMBOL_GPL(tee_device_register);
>  
> +static void plat_tee_free(void *data)
> +{
> +	struct platform_device *plat_dev = data;
> +
> +	platform_device_unregister(plat_dev);
> +}
> +
> +int tee_platform_device_register(struct tee_device *teedev, const char *teename)
> +{
> +	struct platform_device *plat_dev;
> +
> +	plat_dev = platform_device_register_data(&teedev->dev, teename,
> +						 PLATFORM_DEVID_NONE, NULL, 0);
> +	if (IS_ERR(plat_dev))
> +		return PTR_ERR(plat_dev);
> +
> +	devm_add_action_or_reset(&teedev->dev, plat_tee_free, plat_dev);
> +	return 0;
> +}
> +
>  void tee_device_put(struct tee_device *teedev)
>  {
>  	mutex_lock(&teedev->mutex);
> diff --git a/include/linux/firmware/qcom/qcom_ice_tee.h b/include/linux/firmware/qcom/qcom_ice_tee.h
> new file mode 100644
> index 000000000000..a5906c5d4e39
> --- /dev/null
> +++ b/include/linux/firmware/qcom/qcom_ice_tee.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#ifndef __QCOM_ICE_TEE
> +#define __QCOM_ICE_TEE
> +
> +bool qcom_ice_tee_is_available(void);
> +
> +#endif /* __QCOM_ICE_TEE */
> diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h
> index f993d5118edd..d5310b404c1e 100644
> --- a/include/linux/tee_core.h
> +++ b/include/linux/tee_core.h
> @@ -205,6 +205,15 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
>   */
>  int tee_device_register(struct tee_device *teedev);
>  
> +/**
> + * tee_platform_device_register() - Registers a TEE platform device
> + * @teedev:	Device to register
> + * @teename:	Name of the TEE the device represents
> + *
> + * @returns: < 0 on failure
> + */
> +int tee_platform_device_register(struct tee_device *teedev, const char *teename);
> +
>  /**
>   * tee_device_unregister() - Removes a TEE device
>   * @teedev:	Device to unregister
> 

Re: [PATCH v2 3/6] firmware: qcom: Add a TEE bus based ICE back-end

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] firmware: qcom: missing qcom_ice_tee.h breaks build in
  qcom_ice_tee.c
- [High] firmware: qcom: broken stub guard and Kconfig deps for
  qcom_ice_tee_probe()
--

commit dca5daf641df3494a0c5504854d7604efeca0f27
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

firmware: qcom: Add a TEE bus based ICE back-end

This commit adds an OP-TEE client driver that registers TrustZone operations
for the Qualcomm Inline Crypto Engine (ICE) driver.

> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> index 3ad22f8fc3e5f..189ada7fdcd67 100644
> --- a/drivers/firmware/qcom/Kconfig
> +++ b/drivers/firmware/qcom/Kconfig
> @@ -25,6 +25,15 @@ config QCOM_PAS_TEE
>  	  Enable the generic Peripheral Authentication Service (PAS) provided
>  	  by the firmware TEE implementation as the backend.
>  
> +config QCOM_ICE_TEE
> +	tristate
> +	depends on TEE
> +	depends on !CPU_BIG_ENDIAN
> +	default m if ARCH_QCOM
> +	help
> +	  Enable the generic Inline Crypto Engine (ICE) firmware service TEE
> +	  implementation as a backend.
> +
>  config QCOM_SCM
>  	tristate "Qualcomm PAS SCM interface driver" if ARCH_QCOM || COMPILE_TEST
>  	select QCOM_PAS
> diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
> index 88ce74d74c3e2..5ef64f17e6e59 100644
> --- a/drivers/firmware/qcom/Makefile
> +++ b/drivers/firmware/qcom/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_QCOM_QSEECOM)	+= qcom_qseecom.o
>  obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
>  obj-$(CONFIG_QCOM_PAS)		+= qcom_pas.o
>  obj-$(CONFIG_QCOM_PAS_TEE)	+= qcom_pas_tee.o
> +obj-$(CONFIG_QCOM_ICE_TEE)     += qcom_ice_tee.o
> diff --git a/drivers/firmware/qcom/qcom_ice_tee.c b/drivers/firmware/qcom/qcom_ice_tee.c
> new file mode 100644
> index 0000000000000..694aa05847f0e
> --- /dev/null
> +++ b/drivers/firmware/qcom/qcom_ice_tee.c
> @@ -0,0 +1,402 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/uuid.h>
> +#include <linux/firmware/qcom/qcom_ice_tee.h>

[Severity: High]
Does including <linux/firmware/qcom/qcom_ice_tee.h> here break compilation at
this commit?

include/linux/firmware/qcom/qcom_ice_tee.h is not created until a later patch
in the series, commit "tee: optee: Register TEE devices on the platform bus",
and nothing in qcom_ice_tee.c uses it.  Moving this #include to that commit
would keep this patch compilable and bisectable.

[ ... ]

> +	qcom_ice_tee_ops.dev = dev;
> +	ret = qcom_ice_tz_ops_register(&qcom_ice_tee_ops);
> +	if (ret)
> +		goto err_ops;
> +
> +	return 0;
> +err_ops:
> +	tee_client_close_session(data->ctx, data->session_id);
> +err_sess:
> +	tee_client_close_context(data->ctx);
> +	return ret;
> +}
> +
> +static void qcom_ice_tee_remove(struct tee_client_device *ice_dev)
> +{
> +	struct device *dev = &ice_dev->dev;
> +	struct qcom_ice_tee_private *data = dev_get_drvdata(dev);
> +
> +	qcom_ice_tz_ops_unregister(&qcom_ice_tee_ops);

[Severity: High]
When the ICE driver is built as a module, do qcom_ice_tz_ops_register() and
qcom_ice_tz_ops_unregister() here resolve to the no-op stubs in
include/soc/qcom/ice.h?

In commit "soc: qcom: ice: Add TZ agnostic re-direction functions", the
declarations in include/soc/qcom/ice.h are guarded with #ifdef rather than
IS_ENABLED():

include/soc/qcom/ice.h:
    #ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
    int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops);
    void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops);
    #else
    static inline int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *__unused)
    {
    	return 0;
    }

Because qcom_inline_crypto_engine is a tristate symbol (defaulting to m in
arm64 defconfig), modular builds define CONFIG_QCOM_INLINE_CRYPTO_ENGINE_MODULE
instead of CONFIG_QCOM_INLINE_CRYPTO_ENGINE, so qcom_ice_tee_probe() silently
calls the stub and never registers qcom_ice_tee_ops with the ICE driver.

Also, in drivers/firmware/qcom/Kconfig, the new qcom_ice_tee entry has no
dependency on qcom_inline_crypto_engine (allowing qcom_ice_tee to be built-in
while qcom_inline_crypto_engine is a module, or built when it is disabled) and
defines help text on a promptless tristate symbol.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=3

Re: [PATCH v2 4/6] tee: optee: Register TEE devices on the platform bus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential
issues to consider:
- [High] firmware: qcom: duplicate module_init() and module_exit() in
  qcom_ice_tee.c
- [High] tee: missing EXPORT_SYMBOL_GPL() for
  tee_platform_device_register()
- [High] soc: qcom: Kconfig dependency and module cycle selecting
  QCOM_ICE_TEE
- [High] tee: optee: platform device name collision with optee_driver
- [High] soc: qcom: ice: qcom_ice_tz_is_available() requires both SCM
  and OP-TEE
- [Medium] tee: unchecked devm_add_action_or_reset() in
  tee_platform_device_register()
- [Medium] firmware: qcom: missing remove callback in
  qcom_ice_plat_driver
--

commit c9fd96220ba8e5c6eeef8806d3112f1e39c17581
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

tee: optee: Register TEE devices on the platform bus

This commit adds tee_platform_device_register() to register OP-TEE devices on
the platform bus and updates the Qualcomm ICE driver and its TEE backend to
check whether OP-TEE has probed.

> diff --git a/drivers/firmware/qcom/qcom_ice_tee.c b/drivers/firmware/qcom/qcom_ice_tee.c
> index 694aa05847f0e..39f7094439d99 100644
> --- a/drivers/firmware/qcom/qcom_ice_tee.c
> +++ b/drivers/firmware/qcom/qcom_ice_tee.c

[ ... ]

> @@ -398,5 +414,30 @@ static struct tee_client_driver qcom_ice_tee_driver = {
>  
>  module_tee_client_driver(qcom_ice_tee_driver);
>  
> +static int qcom_ice_plat_probe(struct platform_device *pdev)
> +{
> +	/* Paired with smp_load_acquire() in qcom_ice_tee_is_available() */
> +	smp_store_release(&__teedev, &pdev->dev);
> +	return 0;
> +}
> +
> +static const struct platform_device_id qcom_ice_plat_id_table[] = {
> +	{
> +		.name = "optee",
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, qcom_ice_plat_id_table);
> +
> +static struct platform_driver qcom_ice_plat_driver = {
> +	.probe = qcom_ice_plat_probe,
> +	.id_table = qcom_ice_plat_id_table,
> +	.driver = {
> +		.name = "qcom-ice-plat",
> +	},
> +};

[Severity: Medium]
Should qcom_ice_plat_driver also provide a .remove callback to clear
__teedev?  When the parent tee_device is unregistered or qcom-ice-plat is
unbound, plat_tee_free() unregisters and frees pdev, leaving __teedev as a
dangling pointer and causing qcom_ice_tee_is_available() to keep returning
true.

> +
> +module_platform_driver(qcom_ice_plat_driver);

[Severity: High]
Does adding module_platform_driver(qcom_ice_plat_driver) in the same file as
module_tee_client_driver(qcom_ice_tee_driver) break modular builds?  Both
macros expand to module_driver(), which defines __inittest(), init_module(),
__exittest(), and cleanup_module(), causing duplicate symbol definitions when
built as a module.

A single module_init() and module_exit() pair is needed to register
qcom_ice_tee_driver before qcom_ice_plat_driver, unwind on registration
failure, and unregister both in reverse order.

> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 535c8619197b3..8dc75c252f52b 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -296,6 +296,7 @@ endif
>  config QCOM_INLINE_CRYPTO_ENGINE
>  	tristate
>  	select QCOM_SCM
> +	select QCOM_ICE_TEE

[Severity: High]
Can QCOM_INLINE_CRYPTO_ENGINE unconditionally select QCOM_ICE_TEE here?

QCOM_ICE_TEE depends on TEE and !CPU_BIG_ENDIAN in
drivers/firmware/qcom/Kconfig, whereas QCOM_INLINE_CRYPTO_ENGINE is selected
by MMC_SDHCI_MSM and SCSI_UFS_QCOM without those dependencies.  Selecting
QCOM_ICE_TEE when TEE is disabled (or when QCOM_INLINE_CRYPTO_ENGINE is
built-in and TEE is modular) causes unmet direct dependency warnings and link
failures for tee_client_* symbols, and include/linux/firmware/qcom/qcom_ice_tee.h
has no fallback stub for qcom_ice_tee_is_available() (nor an include of
linux/types.h for bool).

In addition, qcom_ice.c now calls qcom_ice_tee_is_available() from
qcom_ice_tee.c while qcom_ice_tee.c calls qcom_ice_tz_ops_register() and
qcom_ice_tz_ops_unregister() from qcom_ice.c, creating a circular dependency
between the two modules.

> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 452896b1858f1..4844621b74970 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/xarray.h>
>  
> +#include <linux/firmware/qcom/qcom_ice_tee.h>
>  #include <linux/firmware/qcom/qcom_scm.h>
>  
>  #include <soc/qcom/ice.h>
> @@ -132,7 +133,13 @@ static bool qcom_ice_tz_ops_registered(void)
>  
>  static bool qcom_ice_tz_is_available(void)
>  {
> -	return qcom_scm_is_available();
> +	/*
> +	 * Both back-ends should be available before we check for
> +	 * qcom_ice_tz_ops_registered(), otherwise we could pre-maturely
> +	 * assume that TZ does not provide any ICE secure interface.
> +	 */
> +	return (qcom_scm_is_available() & qcom_ice_tee_is_available());
> +
>  }

[Severity: High]
Does requiring both qcom_scm_is_available() and qcom_ice_tee_is_available()
break all QTEE-based platforms where OP-TEE is not present?

__teedev is only set when OP-TEE probes and registers its platform device.
On platforms using QTEE via SCM (or when OP-TEE is not enabled),
qcom_ice_tee_is_available() remains false forever, so once qcom_ice_create()
checks qcom_ice_tz_is_available() in the next patch it will defer probe
indefinitely and prevent UFS/SDHC storage from coming online.

Also, this uses bitwise & on booleans instead of &&, and at this commit
qcom_ice_tz_is_available() is an unused static function, triggering a
-Wunused-function warning during bisection.

> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index 51624443359bf..85ee761739826 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -1920,6 +1920,14 @@ static int optee_probe(struct platform_device *pdev)
>  	if (rc)
>  		goto err_disable_shm_cache;
>  
> +	/*
> +	 * Device should only be visible on the platform bus once the
> +	 * child devices have already been enumerated on the TEE bus.
> +	 */
> +	rc = tee_platform_device_register(optee->teedev, "optee");
> +	if (rc)
> +		goto err_disable_shm_cache;

[Severity: High]
Does registering a platform device named "optee" here (and in
optee_ffa_probe()) collide with optee_driver on the platform bus?

drivers/tee/optee/smc_abi.c defines optee_driver with:

drivers/tee/optee/smc_abi.c:
	static struct platform_driver optee_driver = {
		.probe  = optee_probe,
		.remove = optee_smc_remove,
		.shutdown = optee_shutdown,
		.driver = {
			.name = "optee",
			.of_match_table = optee_dt_match,
		},
	};

Because optee_driver has .driver.name = "optee" and no .id_table,
platform_match() falls back to strcmp(pdev->name, drv->name) == 0 and matches
this new child platform device against optee_driver, recursively calling
optee_probe() (which logs errors and fails with -ENXIO).  On non-DT systems
that already have a platform device named "optee" with PLATFORM_DEVID_NONE,
registering a second one also fails with -EEXIST, and only a single platform
driver can bind to the device at a time.

> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 1aac50c7c1de7..0d3c266b29c7b 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c

[ ... ]

> @@ -1225,6 +1226,26 @@ int tee_device_register(struct tee_device *teedev)
>  }
>  EXPORT_SYMBOL_GPL(tee_device_register);
>  
> +static void plat_tee_free(void *data)
> +{
> +	struct platform_device *plat_dev = data;
> +
> +	platform_device_unregister(plat_dev);
> +}
> +
> +int tee_platform_device_register(struct tee_device *teedev, const char *teename)
> +{
> +	struct platform_device *plat_dev;
> +
> +	plat_dev = platform_device_register_data(&teedev->dev, teename,
> +						 PLATFORM_DEVID_NONE, NULL, 0);
> +	if (IS_ERR(plat_dev))
> +		return PTR_ERR(plat_dev);
> +
> +	devm_add_action_or_reset(&teedev->dev, plat_tee_free, plat_dev);
> +	return 0;

[Severity: Medium]
Should tee_platform_device_register() return the result of
devm_add_action_or_reset()?  __devm_add_action_or_reset() is marked
__must_check and on allocation failure it runs plat_tee_free() to unregister
plat_dev, while this function still returns 0 to the caller.

> +}

[Severity: High]
Does tee_platform_device_register() need EXPORT_SYMBOL_GPL()?  It is defined
in tee.ko and called from optee_probe() and optee_ffa_probe() in optee.ko, so
building OP-TEE as a module fails at modpost with an undefined symbol.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=4

Re: [PATCH v2 5/6] soc: qcom: ice: Migrate to the TZ agnostic ICE APIs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] soc: qcom: ice: infinite probe deferral in qcom_ice_create()
  without OP-TEE
- [High] soc: qcom: ice: circular module dependency and build failure in
  qcom_ice_create()
- [Medium] soc: qcom: ice: -ENODEV converts to true in
  qcom_ice_tz_has_wrapped_key_support()
- [Medium] soc: qcom: ice: unsynchronized ice_tz_ops access races with
  backend teardown
--

commit 6c35fbb5f17a57538422d90392b9a647da09687a
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

soc: qcom: ice: Migrate to the TZ agnostic ICE APIs

This commit switches the Qualcomm Inline Crypto Engine driver from calling
qcom_scm_*() directly to using the qcom_ice_tz_*() wrapper functions.

> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 4844621b74970..9bab177f7ce55 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -307,7 +307,7 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
>  	 * advertise support for raw keys or wrapped keys, HWKM cannot be used
>  	 * unconditionally.  A module parameter is used to opt into using it.
>  	 */
> -	if (ice->hwkm_version && qcom_scm_has_wrapped_key_support()) {
> +	if (ice->hwkm_version && qcom_ice_tz_has_wrapped_key_support()) {

[Severity: Medium]
Does qcom_ice_tz_has_wrapped_key_support() evaluate to true when ice_tz_ops
is NULL?

drivers/soc/qcom/ice.c:qcom_ice_tz_has_wrapped_key_support() {
	struct qcom_ice_tz_ops *ops;

	ops = ice_tz_ops;
	if (!ops)
		return -ENODEV;

	return ops->ice_has_wrapped_key_support(ops->dev);
}

Because the return type is bool, returning -ENODEV converts to true,
causing qcom_ice_check_supported() to falsely enable ice->use_hwkm when
ice_tz_ops is NULL.

[ ... ]

> @@ -553,9 +553,9 @@ int qcom_ice_program_key(struct qcom_ice *ice, unsigned int slot,
>  	for (i = 0; i < ARRAY_SIZE(key.words); i++)
>  		__cpu_to_be32s(&key.words[i]);
>  
> -	err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> -				   QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> -				   blk_key->crypto_cfg.data_unit_size / 512);
> +	err = qcom_ice_tz_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> +				  QCOM_ICE_CIPHER_AES_256_XTS,
> +				  blk_key->crypto_cfg.data_unit_size / 512);

[Severity: Medium]
Can qcom_ice_tz_set_key() and the other qcom_ice_tz_*() wrappers race with
qcom_ice_tz_ops_unregister() during backend teardown?

CPU0                              CPU1
qcom_ice_program_key()
  qcom_ice_tz_set_key()
    ops = ice_tz_ops;
    if (!ops)
                                  qcom_ice_tee_remove()
                                    qcom_ice_tz_ops_unregister(...)
                                      smp_store_release(&ice_tz_ops, NULL)
                                    tee_client_close_session(...)
                                    tee_client_close_context(...)
                                    /* devm frees qcom_ice_tee_private */
    ops->ice_set_key(ops->dev, ...)
      ...                         <-- use-after-free on ops->dev data

Each qcom_ice_tz_*() wrapper reads ice_tz_ops without holding a lock or
reference, and qcom_ice_tz_ops_unregister() clears ice_tz_ops without
waiting for in-flight calls to complete before qcom_ice_tee_remove() tears
down the TEE session and frees its devm-managed driver data.

[ ... ]

> @@ -696,11 +696,11 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  {
>  	struct qcom_ice *engine;
>  
> -	if (!qcom_scm_is_available())
> +	if (!qcom_ice_tz_is_available())
>  		return ERR_PTR(-EPROBE_DEFER);

[Severity: High]
Does checking qcom_ice_tz_is_available() here cause qcom_ice_create() to
return -EPROBE_DEFER forever on platforms that do not have both QTEE SCM
and OP-TEE available at the same time?

drivers/soc/qcom/ice.c:qcom_ice_tz_is_available() {
	...
	return (qcom_scm_is_available() & qcom_ice_tee_is_available());
}

qcom_ice_tee_is_available() only becomes true when an OP-TEE platform
device binds to qcom_ice_plat_probe().  On any QTEE/SCM platform without
OP-TEE (or on an OP-TEE platform without qcom_scm),
qcom_ice_tz_is_available() never becomes true even after ice_tz_ops is
registered, so UFS and SDHCI controllers calling devm_of_qcom_ice_get()
defer probe indefinitely and cannot mount the root filesystem.

Conversely, on OP-TEE platforms, qcom_ice_plat_probe() and
qcom_ice_tee_probe() bind on different buses; if qcom_ice_plat_probe() sets
__teedev before qcom_ice_tee_probe() registers qcom_ice_tee_ops,
qcom_ice_tz_is_available() returns true while qcom_ice_tz_ops_registered()
is still false, causing qcom_ice_create() to fail permanently with
-EOPNOTSUPP instead of deferring probe.

>  
> -	if (!qcom_scm_ice_available()) {
> -		dev_warn(dev, "ICE SCM interface not found\n");
> +	if (!qcom_ice_tz_ops_registered()) {
> +		dev_warn(dev, "ICE firmware service interface not found\n");
>  		return ERR_PTR(-EOPNOTSUPP);
>  	}

[Severity: High]
Does calling qcom_ice_tz_is_available() and qcom_ice_tz_ops_registered() in
qcom_ice_create() introduce a circular module dependency and break modular
builds?

Making qcom_ice_tz_is_available() live causes qcom_ice to reference
qcom_ice_tee_is_available() from qcom_ice_tee and qcom_scm_is_available()
from qcom_scm, while both of those modules call qcom_ice_tz_ops_register()
and qcom_ice_tz_ops_unregister() exported by qcom_ice.  This also fails to
link when CONFIG_QCOM_INLINE_CRYPTO_ENGINE=y and CONFIG_QCOM_ICE_TEE is
modular or disabled.

In addition, include/soc/qcom/ice.h guards qcom_ice_tz_ops_register() and
qcom_ice_tz_ops_unregister() with #ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
instead of IS_ENABLED(CONFIG_QCOM_INLINE_CRYPTO_ENGINE):

include/soc/qcom/ice.h:
#ifdef CONFIG_QCOM_INLINE_CRYPTO_ENGINE
int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *ops);
void qcom_ice_tz_ops_unregister(struct qcom_ice_tz_ops *ops);
#else
static inline int qcom_ice_tz_ops_register(struct qcom_ice_tz_ops *__unused)
{
	return 0;
}

When CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m (such as in arm64 defconfig), the
static inline stubs in include/soc/qcom/ice.h conflict with the definitions
in drivers/soc/qcom/ice.c and turn registration in the backends into a
no-op, leaving qcom_ice_tz_ops_registered() permanently false.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=5

Re: [PATCH v2 6/6] firmware: qcom: scm: Drop exported ICE wrapper functions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] firmware: qcom: scm: missing @dev in ICE helper kernel-doc
  comments
--

commit 7bfa90e5e53a6765c06df911105e2ee2369b3560
Author: Harshal Dev <harshal.dev@oss.qualcomm.com>

firmware: qcom: scm: Drop exported ICE wrapper functions

This commit removes the exported ICE wrapper functions from the SCM driver
and renames the internal static helpers now that the ICE driver uses the TZ
agnostic ICE APIs.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 6b66bcedc84fd..277334f71178d 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1399,25 +1399,20 @@ int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_ocmem_unlock);
>  
> -bool __qcom_scm_ice_available(struct device *dev)
> -{
> -	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> -					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
> -		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> -					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
> -}
> -
>  /**
>   * qcom_scm_ice_available() - Is the ICE key programming interface available?
> + * @dev: The SCM device
>   *
>   * Return: true iff the SCM calls wrapped by qcom_scm_ice_invalidate_key() and
>   *	   qcom_scm_ice_set_key() are available.
>   */
> -bool qcom_scm_ice_available(void)
> +static bool qcom_scm_ice_available(struct device *dev)
>  {
> -	return __qcom_scm_ice_available(__scm->dev);
> +	return __qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> +					    QCOM_SCM_ES_INVALIDATE_ICE_KEY) &&
> +		__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_ES,
> +					     QCOM_SCM_ES_CONFIG_SET_ICE_KEY);
>  }
> -EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
>  
>  /**
>   * qcom_scm_ice_invalidate_key() - Invalidate an inline encryption key
> @@ -1431,7 +1426,7 @@ EXPORT_SYMBOL_GPL(qcom_scm_ice_available);
>   *
>   * Return: 0 on success; -errno on failure.
>   */
> -static int __qcom_scm_ice_invalidate_key(struct device *dev, u32 index)
> +static int qcom_scm_ice_invalidate_key(struct device *dev, u32 index)

[Severity: Low]
Should the kernel-doc comments for qcom_scm_ice_invalidate_key(),
qcom_scm_ice_set_key(), qcom_scm_derive_sw_secret(),
qcom_scm_generate_ice_key(), qcom_scm_prepare_ice_key(), and
qcom_scm_import_ice_key() also document @dev like qcom_scm_ice_available()
above?  Renaming these static helpers to match the preceding kernel-doc
blocks without documenting @dev causes W=1 kernel-doc warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ice-migrate-tee-svc-v2-0-29d19f6c92ab@oss.qualcomm.com?part=6