[PATCH RFC v2 0/8] tee: optee: add RPMI backend support on RISC-V

[PATCH RFC v2 0/8] tee: optee: add RPMI backend support on RISC-V

[Amirreza Zarrabi]

This series adds an RPMI backend to the OP-TEE driver for RISC-V.
It builds on the separately posted RPMI TEE transport series [1],
which provides service discovery, synchronous calls, memory parcels
and asynchronous signals.

This revision is effectively a full rewrite of the original RFC.
The generic RPMI transport has been separated from the OP-TEE backend,
and the backend has been reworked around the service bus and a new
OP-TEE control ABI.

Unlike v1, the OP-TEE backend no longer manages SBI MPXY mailbox
channels or implements RPMI TEE service-group operations directly.
It binds to the OP-TEE service UUID on the RPMI TEE bus and uses the
transport's public operations. No additional OP-TEE device-tree node
is required.

The backend reuses the common OP-TEE session, shared-memory, argument
cache, call queue and RPC infrastructure. Shared memory is represented
by RPMI parcel IDs and nonces. Yielding calls pass command and RPC
buffer ranges within a parcel and resume suspended execution using
an opaque token returned by OP-TEE.

The OP-TEE control ABI uses fixed-width little-endian messages carried
in TEE_CALL payloads rather than reproducing FF-A's register-based
message layout. Asynchronous notifications use an allocated TEE-to-REE
signal as a bottom-half doorbell, while ordinary logical notifications
continue through the common RPC path.

Matching OP-TEE OS support for this ABI has not yet been implemented.
This remains an RFC for review of the backend integration and the
Linux-to-OP-TEE protocol.

[1] RPMI TEE transport dependency:
https://lore.kernel.org/op-tee/20260928-riscv-rpmi-tee-abi-v1-0-04908b81d885@oss.qualcomm.com/

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
Changes in v2:
- Effectively rewrite the backend around the separately posted RPMI
  TEE transport and a new OP-TEE control ABI.
- Remove backend-owned per-hart mailbox channels and the OP-TEE-specific
  device-tree binding.
- Replace the register-like control payload with fixed-width messages,
  explicit command/RPC buffer ranges and opaque resume tokens.
- Add a parcel-reference parameter layout carrying the parcel ID,
  nonce, and 64-bit offset and size without changing parameter size.
- Make argument-offset support part of the baseline ABI and retain
  the common shared argument cache.
- Use a transport-allocated signal for asynchronous bottom-half
  notifications.
- Link to v1: https://lore.kernel.org/r/20260912-rpmi-tee-service-grp-dev-v1-0-1d1d35c2a859@oss.qualcomm.com

---
Amirreza Zarrabi (8):
      tee: optee: allow RPMI transport builds on RISC-V
      tee: optee: define the RPMI control and parcel-reference ABI
      tee: optee: add RPMI shared-memory and parameter support
      tee: optee: add RPMI dynamic shared-memory pool
      tee: optee: add RPMI RPC handling
      tee: optee: execute yielding RPMI calls
      tee: optee: bind RPMI services and negotiate backend capabilities
      tee: optee: support RPMI asynchronous notification doorbells

 drivers/tee/Kconfig               |    3 +-
 drivers/tee/optee/Kconfig         |    3 +-
 drivers/tee/optee/Makefile        |    1 +
 drivers/tee/optee/call.c          |    2 +
 drivers/tee/optee/core.c          |   10 +-
 drivers/tee/optee/optee_msg.h     |   38 +-
 drivers/tee/optee/optee_private.h |   54 +-
 drivers/tee/optee/optee_rpmi.h    |  234 ++++++++
 drivers/tee/optee/rpmi_abi.c      | 1059 +++++++++++++++++++++++++++++++++++++
 9 files changed, 1389 insertions(+), 15 deletions(-)
---
base-commit: bf1ee2bd5c2da8992f33c8950ef194aaff74ed6c
change-id: 20260912-rpmi-tee-service-grp-dev-b2ce2f63e0df

Best regards,
-- 
Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

[PATCH RFC v2 1/8] tee: optee: allow RPMI transport builds on RISC-V

[Amirreza Zarrabi]

Allow the TEE subsystem and OP-TEE driver to be selected on RISC-V
systems with the RPMI TEE transport enabled. Prevent OP-TEE from being
built in when the transport is modular.

Common OP-TEE code checks the memory type when registering shared
memory. Add the RISC-V implementation so that code builds without
reaching the unsupported-architecture error. Reject mappings with
explicit non-cacheable or I/O memory types.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/Kconfig       | 3 ++-
 drivers/tee/optee/Kconfig | 3 ++-
 drivers/tee/optee/call.c  | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index 98c3ad083940..ad754b647a3c 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -2,7 +2,8 @@
 # Generic Trusted Execution Environment Configuration
 menuconfig TEE
 	tristate "Trusted Execution Environment support"
-	depends on HAVE_ARM_SMCCC || COMPILE_TEST || CPU_SUP_AMD
+	depends on HAVE_ARM_SMCCC || COMPILE_TEST || CPU_SUP_AMD || \
+		   RISCV_RPMI_TEE_TRANSPORT
 	select CRYPTO_LIB_SHA1
 	select DMA_SHARED_BUFFER
 	select GENERIC_ALLOCATOR
diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
index 891dac63cab8..cc057c76de62 100644
--- a/drivers/tee/optee/Kconfig
+++ b/drivers/tee/optee/Kconfig
@@ -2,10 +2,11 @@
 # OP-TEE Trusted Execution Environment Configuration
 config OPTEE
 	tristate "OP-TEE"
-	depends on HAVE_ARM_SMCCC
+	depends on HAVE_ARM_SMCCC || RISCV_RPMI_TEE_TRANSPORT
 	depends on MMU
 	depends on RPMB || !RPMB
 	depends on ARM_FFA_TRANSPORT || !ARM_FFA_TRANSPORT
+	depends on RISCV_RPMI_TEE_TRANSPORT || !RISCV_RPMI_TEE_TRANSPORT
 	help
 	  This implements the OP-TEE Trusted Execution Environment (TEE)
 	  driver.
diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index e046aff61828..267513cf4ec0 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -604,6 +604,8 @@ static bool is_normal_memory(pgprot_t p)
 #elif defined(CONFIG_ARM64)
 	return ((pgprot_val(p) & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL)) ||
 	       ((pgprot_val(p) & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL_TAGGED));
+#elif defined(CONFIG_RISCV)
+	return !(pgprot_val(p) & _PAGE_MTMASK);
 #else
 #error "Unsupported architecture"
 #endif

-- 
2.34.1

Re: [PATCH RFC v2 1/8] tee: optee: allow RPMI transport builds on RISC-V

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kconfig: referencing undefined symbol RISCV_RPMI_TEE_TRANSPORT prevents RISC-V builds
--

commit f52e668fa2e0c3cbca87392857f87a81c088348f
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

tee: optee: allow RPMI transport builds on RISC-V

This commit updates the Kconfig files to allow the TEE subsystem and OP-TEE
driver to be selected on RISC-V systems using the RPMI transport. It also
implements RISC-V specific memory type checking when registering shared
memory.

> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> index 98c3ad0839409..ad754b647a3c5 100644
> --- a/drivers/tee/Kconfig
> +++ b/drivers/tee/Kconfig
> @@ -2,7 +2,8 @@
>  # Generic Trusted Execution Environment Configuration
>  menuconfig TEE
>  	tristate "Trusted Execution Environment support"
> -	depends on HAVE_ARM_SMCCC || COMPILE_TEST || CPU_SUP_AMD
> +	depends on HAVE_ARM_SMCCC || COMPILE_TEST || CPU_SUP_AMD || \
> +		   RISCV_RPMI_TEE_TRANSPORT

[Severity: Medium]
Does the RISCV_RPMI_TEE_TRANSPORT config symbol exist? It appears this adds
a dependency on an undefined symbol, which evaluates to false and makes it
impossible to satisfy this dependency for the core TEE subsystem on RISC-V
platforms without SMCCC.

>  	select CRYPTO_LIB_SHA1
>  	select DMA_SHARED_BUFFER

[ ... ]

> diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
> index 891dac63cab85..cc057c76de622 100644
> --- a/drivers/tee/optee/Kconfig
> +++ b/drivers/tee/optee/Kconfig
> @@ -2,10 +2,11 @@
>  # OP-TEE Trusted Execution Environment Configuration
>  config OPTEE
>  	tristate "OP-TEE"
> -	depends on HAVE_ARM_SMCCC
> +	depends on HAVE_ARM_SMCCC || RISCV_RPMI_TEE_TRANSPORT

[Severity: Medium]
Since RISCV_RPMI_TEE_TRANSPORT does not appear to be defined anywhere in the
kernel tree, won't this silently disable the OP-TEE driver on RISC-V because
the unfulfilled dependency always evaluates to false?

>  	depends on MMU
>  	depends on RPMB || !RPMB
>  	depends on ARM_FFA_TRANSPORT || !ARM_FFA_TRANSPORT
> +	depends on RISCV_RPMI_TEE_TRANSPORT || !RISCV_RPMI_TEE_TRANSPORT

[Severity: Medium]
Similar to the above, this relies on the undefined RISCV_RPMI_TEE_TRANSPORT
symbol to enforce module compilation. Could this be a typo, or is there a
missing patch in the series that introduces this transport config?

>  	help
>  	  This implements the OP-TEE Trusted Execution Environment (TEE)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=1

[PATCH RFC v2 2/8] tee: optee: define the RPMI control and parcel-reference ABI

[Amirreza Zarrabi]

Define the OP-TEE service protocol carried by RPMI TEE_CALL payloads.
Add operations for version and capability queries, shared-memory
unregistration, asynchronous notification enablement, and yielding-call
start and resume. Use fixed-width little-endian control fields and RPMI
error codes for control responses.

Add a parcel memory-reference layout to the common message parameter
union. Identify shared memory by its parcel ID and nonce, with a 64-bit
byte offset and size, without changing the message parameter size.
Encode NULL references with a zero parcel ID and nonce, leaving parcel
ID zero usable with a nonzero nonce.

Document the wire layouts and ownership rules for matching Linux and
OP-TEE implementations.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/optee_msg.h  |  38 +++++--
 drivers/tee/optee/optee_rpmi.h | 234 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 264 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
index 6c3043f8da33..028f8cd95fdd 100644
--- a/drivers/tee/optee/optee_msg.h
+++ b/drivers/tee/optee/optee_msg.h
@@ -31,6 +31,9 @@
 #define OPTEE_MSG_ATTR_TYPE_FMEM_INPUT		OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
 #define OPTEE_MSG_ATTR_TYPE_FMEM_OUTPUT		OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
 #define OPTEE_MSG_ATTR_TYPE_FMEM_INOUT		OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
+#define OPTEE_MSG_ATTR_TYPE_PMEM_INPUT		OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
+#define OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT		OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
+#define OPTEE_MSG_ATTR_TYPE_PMEM_INOUT		OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
 #define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT		0x9
 #define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT		0xa
 #define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT		0xb
@@ -149,6 +152,23 @@ struct optee_msg_param_fmem {
 	u64 global_id;
 };
 
+/**
+ * struct optee_msg_param_pmem - RPMI parcel memory reference
+ * @offs: full-width byte offset from the parcel's first byte
+ * @size: logical reference size, or required size for a short-buffer response
+ * @parcel_id: firmware-assigned parcel identifier
+ * @nonce: nonzero REE nonce supplied when sharing the parcel
+ *
+ * A zero parcel ID and nonce encode a NULL reference. Its offset must be zero;
+ * its size is preserved. Parcel ID zero remains valid with a nonzero nonce.
+ */
+struct optee_msg_param_pmem {
+	u64 offs;
+	u64 size;
+	u32 parcel_id;
+	u32 nonce;
+};
+
 /**
  * struct optee_msg_param_value - opaque value parameter
  * @a: first opaque value
@@ -166,18 +186,19 @@ struct optee_msg_param_value {
 /**
  * struct optee_msg_param - parameter used together with struct optee_msg_arg
  * @attr:	attributes
- * @tmem:	parameter by temporary memory reference
- * @rmem:	parameter by registered memory reference
- * @fmem:	parameter by FF-A registered memory reference
- * @value:	parameter by opaque value
- * @octets:	parameter by octet string
+ * @u.tmem:	parameter by temporary memory reference
+ * @u.rmem:	parameter by registered memory reference
+ * @u.fmem:	parameter by FF-A registered memory reference
+ * @u.pmem:	parameter by RPMI parcel memory reference
+ * @u.value:	parameter by opaque value
+ * @u.octets:	parameter by octet string
  * @u:		union holding OP-TEE msg parameter
  *
  * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
  * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
- * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
- * OPTEE_MSG_ATTR_TYPE_RMEM_* or the alias PTEE_MSG_ATTR_TYPE_FMEM_* indicates
- * @rmem or @fmem depending on the conduit.
+ * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @u.tmem. OPTEE_MSG_ATTR_TYPE_RMEM_*
+ * and its FMEM/PMEM aliases indicate @u.rmem, @u.fmem or @u.pmem depending
+ * on the conduit.
  * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
  */
 struct optee_msg_param {
@@ -186,6 +207,7 @@ struct optee_msg_param {
 		struct optee_msg_param_tmem tmem;
 		struct optee_msg_param_rmem rmem;
 		struct optee_msg_param_fmem fmem;
+		struct optee_msg_param_pmem pmem;
 		struct optee_msg_param_value value;
 		u8 octets[24];
 	} u;
diff --git a/drivers/tee/optee/optee_rpmi.h b/drivers/tee/optee/optee_rpmi.h
new file mode 100644
index 000000000000..252216aad09b
--- /dev/null
+++ b/drivers/tee/optee/optee_rpmi.h
@@ -0,0 +1,234 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#ifndef OPTEE_RPMI_H
+#define OPTEE_RPMI_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+/*
+ * OP-TEE service ABI over RPMI TEE_CALL.
+ *
+ * Requests and responses are carried in the TEE_CALL service payload.
+ * Control fields are little-endian. Response status fields contain signed
+ * RPMI error codes, distinct from the outer TEE_CALL status and the GP
+ * command result in optee_msg_arg.ret.
+ */
+#define OPTEE_RPMI_SERVICE_UUID \
+	UUID_INIT(0x486178e0, 0xe7f8, 0x11e3, \
+		  0xbc, 0x5e, 0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b)
+
+#define OPTEE_RPMI_VERSION_MAJOR		1
+#define OPTEE_RPMI_VERSION_MINOR		0
+
+/**
+ * struct optee_rpmi_probe_req - request without operation-specific arguments
+ * @op: GET_API_VERSION, GET_OS_VERSION or EXCHANGE_CAPABILITIES
+ */
+struct optee_rpmi_probe_req {
+	__le32 op;
+} __packed;
+
+/**
+ * struct optee_rpmi_status_resp - response carrying only an RPMI status
+ * @status: signed RPMI error code
+ */
+struct optee_rpmi_status_resp {
+	__le32 status;
+} __packed;
+
+/*
+ * Return the service API version.
+ *
+ * Request:  struct optee_rpmi_probe_req
+ * Response: struct optee_rpmi_api_resp
+ */
+#define OPTEE_RPMI_GET_API_VERSION	0
+
+/**
+ * struct optee_rpmi_api_resp - GET_API_VERSION response
+ * @status: signed RPMI error code
+ * @major: incompatible protocol revision
+ * @minor: compatible protocol revision
+ */
+struct optee_rpmi_api_resp {
+	__le32 status;
+	__le32 major;
+	__le32 minor;
+} __packed;
+
+/*
+ * Return the trusted OS revision, not the service API revision.
+ *
+ * Request:  struct optee_rpmi_probe_req
+ * Response: struct optee_rpmi_os_resp
+ */
+#define OPTEE_RPMI_GET_OS_VERSION	1
+
+/**
+ * struct optee_rpmi_os_resp - GET_OS_VERSION response
+ * @status: signed RPMI error code
+ * @major: trusted OS major revision
+ * @minor: trusted OS minor revision
+ * @reserved: must be zero
+ * @build_id: trusted OS build identifier, zero if unspecified
+ */
+struct optee_rpmi_os_resp {
+	__le32 status;
+	__le32 major;
+	__le32 minor;
+	__le32 reserved;
+	__le64 build_id;
+} __packed;
+
+/*
+ * Query secure-world capabilities and limits.
+ *
+ * Request:  struct optee_rpmi_probe_req
+ * Response: struct optee_rpmi_caps_resp
+ */
+#define OPTEE_RPMI_EXCHANGE_CAPABILITIES	2
+
+/**
+ * struct optee_rpmi_caps_resp - EXCHANGE_CAPABILITIES response
+ * @status: signed RPMI error code
+ * @secure_caps: reserved for future optional features; zero in version 1
+ * @rpc_param_count: nonzero parameter capacity of each RPC argument buffer
+ * @notification_count: nonzero logical key count, including synchronous keys
+ *			Keys range from zero through notification_count - 1.
+ *
+ * Version 1 defines no capability bits. Unknown bits are ignored by Linux
+ * for compatibility with future extensions.
+ */
+struct optee_rpmi_caps_resp {
+	__le32 status;
+	__le32 secure_caps;
+	__le32 rpc_param_count;
+	__le32 notification_count;
+} __packed;
+
+/*
+ * Unregister a shared parcel from OP-TEE.
+ *
+ * Request:  struct optee_rpmi_unregister_req
+ * Response: struct optee_rpmi_status_resp
+ */
+#define OPTEE_RPMI_UNREGISTER_SHM	3
+
+/**
+ * struct optee_rpmi_unregister_req - retire a shared parcel in OP-TEE
+ * @op: OPTEE_RPMI_UNREGISTER_SHM
+ * @parcel_id: parcel to retire; zero is valid with a nonzero nonce
+ * @nonce: nonzero nonce associated with the parcel
+ *
+ * Success means OP-TEE stopped using the mapping and released its receiver
+ * interest.
+ */
+struct optee_rpmi_unregister_req {
+	__le32 op;
+	__le32 parcel_id;
+	__le32 nonce;
+} __packed;
+
+/*
+ * Enable asynchronous notification delivery.
+ *
+ * Request:  struct optee_rpmi_enable_notif_req
+ * Response: struct optee_rpmi_status_resp
+ */
+#define OPTEE_RPMI_ENABLE_ASYNC_NOTIF	4
+
+/**
+ * struct optee_rpmi_enable_notif_req - bind an incoming RPMI doorbell
+ * @op: OPTEE_RPMI_ENABLE_ASYNC_NOTIF
+ * @signal_id: allocated TEE-to-REE signal, not a logical notification key
+ *
+ * Success activates delivery and raises the doorbell for pending work.
+ * Raising the signal requests OPTEE_MSG_CMD_DO_BOTTOM_HALF.
+ * OPTEE_MSG_CMD_STOP_ASYNC_NOTIF stops future doorbell generation, but does
+ * not drain already-raised signals or release the signal ID.
+ */
+struct optee_rpmi_enable_notif_req {
+	__le32 op;
+	__le32 signal_id;
+} __packed;
+
+/*
+ * Start a yielding command using shared command and RPC arguments.
+ *
+ * Request:  struct optee_rpmi_call_req
+ * Response: struct optee_rpmi_call_resp
+ */
+#define OPTEE_RPMI_YIELDING_CALL_WITH_ARG		5
+
+#define OPTEE_RPMI_YIELDING_CALL_RETURN_DONE		0
+#define OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD		1
+#define OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT	2
+
+/**
+ * struct optee_rpmi_call_req - start a yielding command
+ * @op: OPTEE_RPMI_YIELDING_CALL_WITH_ARG
+ * @parcel_id: argument parcel identity
+ * @nonce: nonce associated with the parcel
+ * @flags: zero in version 1
+ * @arg_offset: command argument byte offset from the parcel's first byte
+ * @rpc_offset: RPC argument byte offset from the parcel's first byte
+ * @arg_size: command argument extent in bytes
+ * @rpc_size: RPC argument capacity in bytes
+ *
+ * Firmware validates ownership, RW access, alignment and disjoint ranges.
+ * RPMI_ERR_BUSY rejects an initial request without acquiring call ownership.
+ */
+struct optee_rpmi_call_req {
+	__le32 op;
+	__le32 parcel_id;
+	__le32 nonce;
+	__le32 flags;
+	__le64 arg_offset;
+	__le64 rpc_offset;
+	__le32 arg_size;
+	__le32 rpc_size;
+} __packed;
+
+/**
+ * struct optee_rpmi_call_resp - yielding command response
+ * @status: signed RPMI error code, distinct from the command's GP result
+ * @result: OPTEE_RPMI_YIELDING_CALL_RETURN_* value when status is success
+ * @resume_token: zero for DONE, nonzero opaque token for a suspended command
+ *
+ * DONE releases all access to the call's argument and RPC ranges. RPC_CMD
+ * requests RPC command handling; INTERRUPT requests resumption without an
+ * RPC command. Tokens belong to one accepted call, service and caller.
+ */
+struct optee_rpmi_call_resp {
+	__le32 status;
+	__le32 result;
+	__le64 resume_token;
+} __packed;
+
+/*
+ * Resume a suspended yielding command.
+ *
+ * Request:  struct optee_rpmi_resume_req
+ * Response: struct optee_rpmi_call_resp
+ */
+#define OPTEE_RPMI_YIELDING_CALL_RESUME	6
+
+/**
+ * struct optee_rpmi_resume_req - resume a suspended command
+ * @op: OPTEE_RPMI_YIELDING_CALL_RESUME
+ * @reserved: must be zero
+ * @resume_token: token from the preceding response for this call
+ *
+ * Resume must not return RPMI_ERR_BUSY.
+ */
+struct optee_rpmi_resume_req {
+	__le32 op;
+	__le32 reserved;
+	__le64 resume_token;
+} __packed;
+
+#endif /* OPTEE_RPMI_H */

-- 
2.34.1

Re: [PATCH RFC v2 2/8] tee: optee: define the RPMI control and parcel-reference ABI

[Jens Wiklander]

Hi Amir,

On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> Define the OP-TEE service protocol carried by RPMI TEE_CALL payloads.
> Add operations for version and capability queries, shared-memory
> unregistration, asynchronous notification enablement, and yielding-call
> start and resume. Use fixed-width little-endian control fields and RPMI
> error codes for control responses.
>
> Add a parcel memory-reference layout to the common message parameter
> union. Identify shared memory by its parcel ID and nonce, with a 64-bit
> byte offset and size, without changing the message parameter size.
> Encode NULL references with a zero parcel ID and nonce, leaving parcel
> ID zero usable with a nonzero nonce.
>
> Document the wire layouts and ownership rules for matching Linux and
> OP-TEE implementations.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
>  drivers/tee/optee/optee_msg.h  |  38 +++++--
>  drivers/tee/optee/optee_rpmi.h | 234 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 264 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> index 6c3043f8da33..028f8cd95fdd 100644
> --- a/drivers/tee/optee/optee_msg.h
> +++ b/drivers/tee/optee/optee_msg.h
> @@ -31,6 +31,9 @@
>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
>  #define OPTEE_MSG_ATTR_TYPE_FMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
> +#define OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT         0x9
>  #define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT                0xa
>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT         0xb
> @@ -149,6 +152,23 @@ struct optee_msg_param_fmem {
>         u64 global_id;
>  };
>
> +/**
> + * struct optee_msg_param_pmem - RPMI parcel memory reference
> + * @offs: full-width byte offset from the parcel's first byte
> + * @size: logical reference size, or required size for a short-buffer response
> + * @parcel_id: firmware-assigned parcel identifier
> + * @nonce: nonzero REE nonce supplied when sharing the parcel
> + *
> + * A zero parcel ID and nonce encode a NULL reference. Its offset must be zero;
> + * its size is preserved. Parcel ID zero remains valid with a nonzero nonce.
> + */
> +struct optee_msg_param_pmem {

Without an internal offset like in struct optee_msg_param_fmem, an
explicit register SHM call into OP-TEE is needed before it can be
used. You'd still need the TEE_MEMORY_PARCEL_CREATE, but we can save
one round-trip into the secure world.

> +       u64 offs;
> +       u64 size;
> +       u32 parcel_id;
> +       u32 nonce;

I wonder if parcel_id and nonce wouldn't be better combined into a
single field. They need to be separate when preparing arguments for
the TEE_MEMORY_* calls. Everywhere else, it's only an opaque memory
handle, and one handle is easier to keep track of than two.

> +};
> +
>  /**
>   * struct optee_msg_param_value - opaque value parameter
>   * @a: first opaque value
> @@ -166,18 +186,19 @@ struct optee_msg_param_value {
>  /**
>   * struct optee_msg_param - parameter used together with struct optee_msg_arg
>   * @attr:      attributes
> - * @tmem:      parameter by temporary memory reference
> - * @rmem:      parameter by registered memory reference
> - * @fmem:      parameter by FF-A registered memory reference
> - * @value:     parameter by opaque value
> - * @octets:    parameter by octet string
> + * @u.tmem:    parameter by temporary memory reference
> + * @u.rmem:    parameter by registered memory reference
> + * @u.fmem:    parameter by FF-A registered memory reference
> + * @u.pmem:    parameter by RPMI parcel memory reference
> + * @u.value:   parameter by opaque value
> + * @u.octets:  parameter by octet string
>   * @u:         union holding OP-TEE msg parameter
>   *
>   * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
>   * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
> - * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
> - * OPTEE_MSG_ATTR_TYPE_RMEM_* or the alias PTEE_MSG_ATTR_TYPE_FMEM_* indicates
> - * @rmem or @fmem depending on the conduit.
> + * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @u.tmem. OPTEE_MSG_ATTR_TYPE_RMEM_*
> + * and its FMEM/PMEM aliases indicate @u.rmem, @u.fmem or @u.pmem depending
> + * on the conduit.
>   * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
>   */
>  struct optee_msg_param {
> @@ -186,6 +207,7 @@ struct optee_msg_param {
>                 struct optee_msg_param_tmem tmem;
>                 struct optee_msg_param_rmem rmem;
>                 struct optee_msg_param_fmem fmem;
> +               struct optee_msg_param_pmem pmem;
>                 struct optee_msg_param_value value;
>                 u8 octets[24];
>         } u;
> diff --git a/drivers/tee/optee/optee_rpmi.h b/drivers/tee/optee/optee_rpmi.h
> new file mode 100644
> index 000000000000..252216aad09b
> --- /dev/null
> +++ b/drivers/tee/optee/optee_rpmi.h
> @@ -0,0 +1,234 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#ifndef OPTEE_RPMI_H
> +#define OPTEE_RPMI_H
> +
> +#include <linux/bitops.h>
> +#include <linux/types.h>
> +#include <linux/uuid.h>
> +
> +/*
> + * OP-TEE service ABI over RPMI TEE_CALL.
> + *
> + * Requests and responses are carried in the TEE_CALL service payload.
> + * Control fields are little-endian. Response status fields contain signed
> + * RPMI error codes, distinct from the outer TEE_CALL status and the GP
> + * command result in optee_msg_arg.ret.
> + */
> +#define OPTEE_RPMI_SERVICE_UUID \
> +       UUID_INIT(0x486178e0, 0xe7f8, 0x11e3, \
> +                 0xbc, 0x5e, 0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b)
> +
> +#define OPTEE_RPMI_VERSION_MAJOR               1
> +#define OPTEE_RPMI_VERSION_MINOR               0
> +
> +/**
> + * struct optee_rpmi_probe_req - request without operation-specific arguments
> + * @op: GET_API_VERSION, GET_OS_VERSION or EXCHANGE_CAPABILITIES
> + */
> +struct optee_rpmi_probe_req {
> +       __le32 op;
> +} __packed;
> +
> +/**
> + * struct optee_rpmi_status_resp - response carrying only an RPMI status
> + * @status: signed RPMI error code
> + */
> +struct optee_rpmi_status_resp {
> +       __le32 status;
> +} __packed;
> +
> +/*
> + * Return the service API version.
> + *
> + * Request:  struct optee_rpmi_probe_req
> + * Response: struct optee_rpmi_api_resp
> + */
> +#define OPTEE_RPMI_GET_API_VERSION     0
> +
> +/**
> + * struct optee_rpmi_api_resp - GET_API_VERSION response
> + * @status: signed RPMI error code
> + * @major: incompatible protocol revision
> + * @minor: compatible protocol revision
> + */
> +struct optee_rpmi_api_resp {
> +       __le32 status;
> +       __le32 major;
> +       __le32 minor;
> +} __packed;

Why do these communication structs have to be packed? With careful
design of the layout, padding, alignment, etc, it shouldn't be an
issue.

> +
> +/*
> + * Return the trusted OS revision, not the service API revision.
> + *
> + * Request:  struct optee_rpmi_probe_req
> + * Response: struct optee_rpmi_os_resp
> + */
> +#define OPTEE_RPMI_GET_OS_VERSION      1
> +
> +/**
> + * struct optee_rpmi_os_resp - GET_OS_VERSION response
> + * @status: signed RPMI error code
> + * @major: trusted OS major revision
> + * @minor: trusted OS minor revision
> + * @reserved: must be zero
> + * @build_id: trusted OS build identifier, zero if unspecified
> + */
> +struct optee_rpmi_os_resp {
> +       __le32 status;
> +       __le32 major;
> +       __le32 minor;
> +       __le32 reserved;
> +       __le64 build_id;
> +} __packed;
> +
> +/*
> + * Query secure-world capabilities and limits.
> + *
> + * Request:  struct optee_rpmi_probe_req
> + * Response: struct optee_rpmi_caps_resp
> + */
> +#define OPTEE_RPMI_EXCHANGE_CAPABILITIES       2
> +
> +/**
> + * struct optee_rpmi_caps_resp - EXCHANGE_CAPABILITIES response
> + * @status: signed RPMI error code
> + * @secure_caps: reserved for future optional features; zero in version 1
> + * @rpc_param_count: nonzero parameter capacity of each RPC argument buffer
> + * @notification_count: nonzero logical key count, including synchronous keys
> + *                     Keys range from zero through notification_count - 1.
> + *
> + * Version 1 defines no capability bits. Unknown bits are ignored by Linux
> + * for compatibility with future extensions.

Note that we expect the ABI version to stay at 1.0 for a foreseeable
future. Extensions to the ABI are primarily negotiated using
capabilities.

Cheers,
Jens

> + */
> +struct optee_rpmi_caps_resp {
> +       __le32 status;
> +       __le32 secure_caps;
> +       __le32 rpc_param_count;
> +       __le32 notification_count;
> +} __packed;
> +
> +/*
> + * Unregister a shared parcel from OP-TEE.
> + *
> + * Request:  struct optee_rpmi_unregister_req
> + * Response: struct optee_rpmi_status_resp
> + */
> +#define OPTEE_RPMI_UNREGISTER_SHM      3
> +
> +/**
> + * struct optee_rpmi_unregister_req - retire a shared parcel in OP-TEE
> + * @op: OPTEE_RPMI_UNREGISTER_SHM
> + * @parcel_id: parcel to retire; zero is valid with a nonzero nonce
> + * @nonce: nonzero nonce associated with the parcel
> + *
> + * Success means OP-TEE stopped using the mapping and released its receiver
> + * interest.
> + */
> +struct optee_rpmi_unregister_req {
> +       __le32 op;
> +       __le32 parcel_id;
> +       __le32 nonce;
> +} __packed;
> +
> +/*
> + * Enable asynchronous notification delivery.
> + *
> + * Request:  struct optee_rpmi_enable_notif_req
> + * Response: struct optee_rpmi_status_resp
> + */
> +#define OPTEE_RPMI_ENABLE_ASYNC_NOTIF  4
> +
> +/**
> + * struct optee_rpmi_enable_notif_req - bind an incoming RPMI doorbell
> + * @op: OPTEE_RPMI_ENABLE_ASYNC_NOTIF
> + * @signal_id: allocated TEE-to-REE signal, not a logical notification key
> + *
> + * Success activates delivery and raises the doorbell for pending work.
> + * Raising the signal requests OPTEE_MSG_CMD_DO_BOTTOM_HALF.
> + * OPTEE_MSG_CMD_STOP_ASYNC_NOTIF stops future doorbell generation, but does
> + * not drain already-raised signals or release the signal ID.
> + */
> +struct optee_rpmi_enable_notif_req {
> +       __le32 op;
> +       __le32 signal_id;
> +} __packed;
> +
> +/*
> + * Start a yielding command using shared command and RPC arguments.
> + *
> + * Request:  struct optee_rpmi_call_req
> + * Response: struct optee_rpmi_call_resp
> + */
> +#define OPTEE_RPMI_YIELDING_CALL_WITH_ARG              5
> +
> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_DONE           0
> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD                1
> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT      2
> +
> +/**
> + * struct optee_rpmi_call_req - start a yielding command
> + * @op: OPTEE_RPMI_YIELDING_CALL_WITH_ARG
> + * @parcel_id: argument parcel identity
> + * @nonce: nonce associated with the parcel
> + * @flags: zero in version 1
> + * @arg_offset: command argument byte offset from the parcel's first byte
> + * @rpc_offset: RPC argument byte offset from the parcel's first byte
> + * @arg_size: command argument extent in bytes
> + * @rpc_size: RPC argument capacity in bytes
> + *
> + * Firmware validates ownership, RW access, alignment and disjoint ranges.
> + * RPMI_ERR_BUSY rejects an initial request without acquiring call ownership.
> + */
> +struct optee_rpmi_call_req {
> +       __le32 op;
> +       __le32 parcel_id;
> +       __le32 nonce;
> +       __le32 flags;
> +       __le64 arg_offset;
> +       __le64 rpc_offset;
> +       __le32 arg_size;
> +       __le32 rpc_size;
> +} __packed;
> +
> +/**
> + * struct optee_rpmi_call_resp - yielding command response
> + * @status: signed RPMI error code, distinct from the command's GP result
> + * @result: OPTEE_RPMI_YIELDING_CALL_RETURN_* value when status is success
> + * @resume_token: zero for DONE, nonzero opaque token for a suspended command
> + *
> + * DONE releases all access to the call's argument and RPC ranges. RPC_CMD
> + * requests RPC command handling; INTERRUPT requests resumption without an
> + * RPC command. Tokens belong to one accepted call, service and caller.
> + */
> +struct optee_rpmi_call_resp {
> +       __le32 status;
> +       __le32 result;
> +       __le64 resume_token;
> +} __packed;
> +
> +/*
> + * Resume a suspended yielding command.
> + *
> + * Request:  struct optee_rpmi_resume_req
> + * Response: struct optee_rpmi_call_resp
> + */
> +#define OPTEE_RPMI_YIELDING_CALL_RESUME        6
> +
> +/**
> + * struct optee_rpmi_resume_req - resume a suspended command
> + * @op: OPTEE_RPMI_YIELDING_CALL_RESUME
> + * @reserved: must be zero
> + * @resume_token: token from the preceding response for this call
> + *
> + * Resume must not return RPMI_ERR_BUSY.
> + */
> +struct optee_rpmi_resume_req {
> +       __le32 op;
> +       __le32 reserved;
> +       __le64 resume_token;
> +} __packed;
> +
> +#endif /* OPTEE_RPMI_H */
>
> --
> 2.34.1
>

Re: [PATCH RFC v2 2/8] tee: optee: define the RPMI control and parcel-reference ABI

[Amirreza Zarrabi]

Hi Jens,

On 10/8/2026 5:51 PM, Jens Wiklander wrote:
> Hi Amir,
> 
> On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> Define the OP-TEE service protocol carried by RPMI TEE_CALL payloads.
>> Add operations for version and capability queries, shared-memory
>> unregistration, asynchronous notification enablement, and yielding-call
>> start and resume. Use fixed-width little-endian control fields and RPMI
>> error codes for control responses.
>>
>> Add a parcel memory-reference layout to the common message parameter
>> union. Identify shared memory by its parcel ID and nonce, with a 64-bit
>> byte offset and size, without changing the message parameter size.
>> Encode NULL references with a zero parcel ID and nonce, leaving parcel
>> ID zero usable with a nonzero nonce.
>>
>> Document the wire layouts and ownership rules for matching Linux and
>> OP-TEE implementations.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>>  drivers/tee/optee/optee_msg.h  |  38 +++++--
>>  drivers/tee/optee/optee_rpmi.h | 234 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 264 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
>> index 6c3043f8da33..028f8cd95fdd 100644
>> --- a/drivers/tee/optee/optee_msg.h
>> +++ b/drivers/tee/optee/optee_msg.h
>> @@ -31,6 +31,9 @@
>>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
>>  #define OPTEE_MSG_ATTR_TYPE_FMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
>>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
>> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
>> +#define OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
>> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
>>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT         0x9
>>  #define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT                0xa
>>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT         0xb
>> @@ -149,6 +152,23 @@ struct optee_msg_param_fmem {
>>         u64 global_id;
>>  };
>>
>> +/**
>> + * struct optee_msg_param_pmem - RPMI parcel memory reference
>> + * @offs: full-width byte offset from the parcel's first byte
>> + * @size: logical reference size, or required size for a short-buffer response
>> + * @parcel_id: firmware-assigned parcel identifier
>> + * @nonce: nonzero REE nonce supplied when sharing the parcel
>> + *
>> + * A zero parcel ID and nonce encode a NULL reference. Its offset must be zero;
>> + * its size is preserved. Parcel ID zero remains valid with a nonzero nonce.
>> + */
>> +struct optee_msg_param_pmem {
> 
> Without an internal offset like in struct optee_msg_param_fmem, an
> explicit register SHM call into OP-TEE is needed before it can be
> used. You'd still need the TEE_MEMORY_PARCEL_CREATE, but we can save
> one round-trip into the secure world.
> 

The original intenation was to use parcel-relative offsets, with the
secure-side memory object covers the entire parcel. OP-TEE can retrieve it
lazily and apply the supplied offset directly, so this design does not require
an explicit SHM registration call or an additional round trip.

Since we are implementing the RPMI parcel memory-object cache independently of
FF-A's cache in OP-TEE, this seems simpler: it avoids a separate
initial-offset property, preserves a full-width offset reference, while
keeping OP-TEE ignorant from the SHM concept which is a Linux side concept.

That said, I can separate the offsets to matching FF-A's memory-object
model. But I am not sure what we achive?

>> +       u64 offs;
>> +       u64 size;
>> +       u32 parcel_id;
>> +       u32 nonce;
> 
> I wonder if parcel_id and nonce wouldn't be better combined into a
> single field. They need to be separate when preparing arguments for
> the TEE_MEMORY_* calls. Everywhere else, it's only an opaque memory
> handle, and one handle is easier to keep track of than two.

I thought about that before. This would push the conversion to the
firmware boundary. I was not sure if it is acceptable. I'll do that :).

> 
>> +};
>> +
>>  /**
>>   * struct optee_msg_param_value - opaque value parameter
>>   * @a: first opaque value
>> @@ -166,18 +186,19 @@ struct optee_msg_param_value {
>>  /**
>>   * struct optee_msg_param - parameter used together with struct optee_msg_arg
>>   * @attr:      attributes
>> - * @tmem:      parameter by temporary memory reference
>> - * @rmem:      parameter by registered memory reference
>> - * @fmem:      parameter by FF-A registered memory reference
>> - * @value:     parameter by opaque value
>> - * @octets:    parameter by octet string
>> + * @u.tmem:    parameter by temporary memory reference
>> + * @u.rmem:    parameter by registered memory reference
>> + * @u.fmem:    parameter by FF-A registered memory reference
>> + * @u.pmem:    parameter by RPMI parcel memory reference
>> + * @u.value:   parameter by opaque value
>> + * @u.octets:  parameter by octet string
>>   * @u:         union holding OP-TEE msg parameter
>>   *
>>   * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
>>   * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
>> - * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
>> - * OPTEE_MSG_ATTR_TYPE_RMEM_* or the alias PTEE_MSG_ATTR_TYPE_FMEM_* indicates
>> - * @rmem or @fmem depending on the conduit.
>> + * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @u.tmem. OPTEE_MSG_ATTR_TYPE_RMEM_*
>> + * and its FMEM/PMEM aliases indicate @u.rmem, @u.fmem or @u.pmem depending
>> + * on the conduit.
>>   * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
>>   */
>>  struct optee_msg_param {
>> @@ -186,6 +207,7 @@ struct optee_msg_param {
>>                 struct optee_msg_param_tmem tmem;
>>                 struct optee_msg_param_rmem rmem;
>>                 struct optee_msg_param_fmem fmem;
>> +               struct optee_msg_param_pmem pmem;
>>                 struct optee_msg_param_value value;
>>                 u8 octets[24];
>>         } u;
>> diff --git a/drivers/tee/optee/optee_rpmi.h b/drivers/tee/optee/optee_rpmi.h
>> new file mode 100644
>> index 000000000000..252216aad09b
>> --- /dev/null
>> +++ b/drivers/tee/optee/optee_rpmi.h
>> @@ -0,0 +1,234 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +#ifndef OPTEE_RPMI_H
>> +#define OPTEE_RPMI_H
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/types.h>
>> +#include <linux/uuid.h>
>> +
>> +/*
>> + * OP-TEE service ABI over RPMI TEE_CALL.
>> + *
>> + * Requests and responses are carried in the TEE_CALL service payload.
>> + * Control fields are little-endian. Response status fields contain signed
>> + * RPMI error codes, distinct from the outer TEE_CALL status and the GP
>> + * command result in optee_msg_arg.ret.
>> + */
>> +#define OPTEE_RPMI_SERVICE_UUID \
>> +       UUID_INIT(0x486178e0, 0xe7f8, 0x11e3, \
>> +                 0xbc, 0x5e, 0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b)
>> +
>> +#define OPTEE_RPMI_VERSION_MAJOR               1
>> +#define OPTEE_RPMI_VERSION_MINOR               0
>> +
>> +/**
>> + * struct optee_rpmi_probe_req - request without operation-specific arguments
>> + * @op: GET_API_VERSION, GET_OS_VERSION or EXCHANGE_CAPABILITIES
>> + */
>> +struct optee_rpmi_probe_req {
>> +       __le32 op;
>> +} __packed;
>> +
>> +/**
>> + * struct optee_rpmi_status_resp - response carrying only an RPMI status
>> + * @status: signed RPMI error code
>> + */
>> +struct optee_rpmi_status_resp {
>> +       __le32 status;
>> +} __packed;
>> +
>> +/*
>> + * Return the service API version.
>> + *
>> + * Request:  struct optee_rpmi_probe_req
>> + * Response: struct optee_rpmi_api_resp
>> + */
>> +#define OPTEE_RPMI_GET_API_VERSION     0
>> +
>> +/**
>> + * struct optee_rpmi_api_resp - GET_API_VERSION response
>> + * @status: signed RPMI error code
>> + * @major: incompatible protocol revision
>> + * @minor: compatible protocol revision
>> + */
>> +struct optee_rpmi_api_resp {
>> +       __le32 status;
>> +       __le32 major;
>> +       __le32 minor;
>> +} __packed;
> 
> Why do these communication structs have to be packed? With careful
> design of the layout, padding, alignment, etc, it shouldn't be an
> issue.

Agreed. The structures already have naturally aligned fields and
explicit reserved fields where needed, so `__packed` is unnecessary for
their current layouts. I'll remove it and keep the wire layout explicitly
defined, retaining unaligned access helpers where transport-buffer alignment
is not guaranteed.

> 
>> +
>> +/*
>> + * Return the trusted OS revision, not the service API revision.
>> + *
>> + * Request:  struct optee_rpmi_probe_req
>> + * Response: struct optee_rpmi_os_resp
>> + */
>> +#define OPTEE_RPMI_GET_OS_VERSION      1
>> +
>> +/**
>> + * struct optee_rpmi_os_resp - GET_OS_VERSION response
>> + * @status: signed RPMI error code
>> + * @major: trusted OS major revision
>> + * @minor: trusted OS minor revision
>> + * @reserved: must be zero
>> + * @build_id: trusted OS build identifier, zero if unspecified
>> + */
>> +struct optee_rpmi_os_resp {
>> +       __le32 status;
>> +       __le32 major;
>> +       __le32 minor;
>> +       __le32 reserved;
>> +       __le64 build_id;
>> +} __packed;
>> +
>> +/*
>> + * Query secure-world capabilities and limits.
>> + *
>> + * Request:  struct optee_rpmi_probe_req
>> + * Response: struct optee_rpmi_caps_resp
>> + */
>> +#define OPTEE_RPMI_EXCHANGE_CAPABILITIES       2
>> +
>> +/**
>> + * struct optee_rpmi_caps_resp - EXCHANGE_CAPABILITIES response
>> + * @status: signed RPMI error code
>> + * @secure_caps: reserved for future optional features; zero in version 1
>> + * @rpc_param_count: nonzero parameter capacity of each RPC argument buffer
>> + * @notification_count: nonzero logical key count, including synchronous keys
>> + *                     Keys range from zero through notification_count - 1.
>> + *
>> + * Version 1 defines no capability bits. Unknown bits are ignored by Linux
>> + * for compatibility with future extensions.
> 
> Note that we expect the ABI version to stay at 1.0 for a foreseeable
> future. Extensions to the ABI are primarily negotiated using
> capabilities.
> 

Ack.

Thanks Jens,

Best regards,
Amir

> Cheers,
> Jens
> 
>> + */
>> +struct optee_rpmi_caps_resp {
>> +       __le32 status;
>> +       __le32 secure_caps;
>> +       __le32 rpc_param_count;
>> +       __le32 notification_count;
>> +} __packed;
>> +
>> +/*
>> + * Unregister a shared parcel from OP-TEE.
>> + *
>> + * Request:  struct optee_rpmi_unregister_req
>> + * Response: struct optee_rpmi_status_resp
>> + */
>> +#define OPTEE_RPMI_UNREGISTER_SHM      3
>> +
>> +/**
>> + * struct optee_rpmi_unregister_req - retire a shared parcel in OP-TEE
>> + * @op: OPTEE_RPMI_UNREGISTER_SHM
>> + * @parcel_id: parcel to retire; zero is valid with a nonzero nonce
>> + * @nonce: nonzero nonce associated with the parcel
>> + *
>> + * Success means OP-TEE stopped using the mapping and released its receiver
>> + * interest.
>> + */
>> +struct optee_rpmi_unregister_req {
>> +       __le32 op;
>> +       __le32 parcel_id;
>> +       __le32 nonce;
>> +} __packed;
>> +
>> +/*
>> + * Enable asynchronous notification delivery.
>> + *
>> + * Request:  struct optee_rpmi_enable_notif_req
>> + * Response: struct optee_rpmi_status_resp
>> + */
>> +#define OPTEE_RPMI_ENABLE_ASYNC_NOTIF  4
>> +
>> +/**
>> + * struct optee_rpmi_enable_notif_req - bind an incoming RPMI doorbell
>> + * @op: OPTEE_RPMI_ENABLE_ASYNC_NOTIF
>> + * @signal_id: allocated TEE-to-REE signal, not a logical notification key
>> + *
>> + * Success activates delivery and raises the doorbell for pending work.
>> + * Raising the signal requests OPTEE_MSG_CMD_DO_BOTTOM_HALF.
>> + * OPTEE_MSG_CMD_STOP_ASYNC_NOTIF stops future doorbell generation, but does
>> + * not drain already-raised signals or release the signal ID.
>> + */
>> +struct optee_rpmi_enable_notif_req {
>> +       __le32 op;
>> +       __le32 signal_id;
>> +} __packed;
>> +
>> +/*
>> + * Start a yielding command using shared command and RPC arguments.
>> + *
>> + * Request:  struct optee_rpmi_call_req
>> + * Response: struct optee_rpmi_call_resp
>> + */
>> +#define OPTEE_RPMI_YIELDING_CALL_WITH_ARG              5
>> +
>> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_DONE           0
>> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD                1
>> +#define OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT      2
>> +
>> +/**
>> + * struct optee_rpmi_call_req - start a yielding command
>> + * @op: OPTEE_RPMI_YIELDING_CALL_WITH_ARG
>> + * @parcel_id: argument parcel identity
>> + * @nonce: nonce associated with the parcel
>> + * @flags: zero in version 1
>> + * @arg_offset: command argument byte offset from the parcel's first byte
>> + * @rpc_offset: RPC argument byte offset from the parcel's first byte
>> + * @arg_size: command argument extent in bytes
>> + * @rpc_size: RPC argument capacity in bytes
>> + *
>> + * Firmware validates ownership, RW access, alignment and disjoint ranges.
>> + * RPMI_ERR_BUSY rejects an initial request without acquiring call ownership.
>> + */
>> +struct optee_rpmi_call_req {
>> +       __le32 op;
>> +       __le32 parcel_id;
>> +       __le32 nonce;
>> +       __le32 flags;
>> +       __le64 arg_offset;
>> +       __le64 rpc_offset;
>> +       __le32 arg_size;
>> +       __le32 rpc_size;
>> +} __packed;
>> +
>> +/**
>> + * struct optee_rpmi_call_resp - yielding command response
>> + * @status: signed RPMI error code, distinct from the command's GP result
>> + * @result: OPTEE_RPMI_YIELDING_CALL_RETURN_* value when status is success
>> + * @resume_token: zero for DONE, nonzero opaque token for a suspended command
>> + *
>> + * DONE releases all access to the call's argument and RPC ranges. RPC_CMD
>> + * requests RPC command handling; INTERRUPT requests resumption without an
>> + * RPC command. Tokens belong to one accepted call, service and caller.
>> + */
>> +struct optee_rpmi_call_resp {
>> +       __le32 status;
>> +       __le32 result;
>> +       __le64 resume_token;
>> +} __packed;
>> +
>> +/*
>> + * Resume a suspended yielding command.
>> + *
>> + * Request:  struct optee_rpmi_resume_req
>> + * Response: struct optee_rpmi_call_resp
>> + */
>> +#define OPTEE_RPMI_YIELDING_CALL_RESUME        6
>> +
>> +/**
>> + * struct optee_rpmi_resume_req - resume a suspended command
>> + * @op: OPTEE_RPMI_YIELDING_CALL_RESUME
>> + * @reserved: must be zero
>> + * @resume_token: token from the preceding response for this call
>> + *
>> + * Resume must not return RPMI_ERR_BUSY.
>> + */
>> +struct optee_rpmi_resume_req {
>> +       __le32 op;
>> +       __le32 reserved;
>> +       __le64 resume_token;
>> +} __packed;
>> +
>> +#endif /* OPTEE_RPMI_H */
>>
>> --
>> 2.34.1
>>

Re: [PATCH RFC v2 2/8] tee: optee: define the RPMI control and parcel-reference ABI

[Jens Wiklander]

Hi Amir,

On Fri, Oct 9, 2026 at 12:03 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> Hi Jens,
>
> On 10/8/2026 5:51 PM, Jens Wiklander wrote:
> > Hi Amir,
> >
> > On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> > <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> Define the OP-TEE service protocol carried by RPMI TEE_CALL payloads.
> >> Add operations for version and capability queries, shared-memory
> >> unregistration, asynchronous notification enablement, and yielding-call
> >> start and resume. Use fixed-width little-endian control fields and RPMI
> >> error codes for control responses.
> >>
> >> Add a parcel memory-reference layout to the common message parameter
> >> union. Identify shared memory by its parcel ID and nonce, with a 64-bit
> >> byte offset and size, without changing the message parameter size.
> >> Encode NULL references with a zero parcel ID and nonce, leaving parcel
> >> ID zero usable with a nonzero nonce.
> >>
> >> Document the wire layouts and ownership rules for matching Linux and
> >> OP-TEE implementations.
> >>
> >> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> >> ---
> >>  drivers/tee/optee/optee_msg.h  |  38 +++++--
> >>  drivers/tee/optee/optee_rpmi.h | 234 +++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 264 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> >> index 6c3043f8da33..028f8cd95fdd 100644
> >> --- a/drivers/tee/optee/optee_msg.h
> >> +++ b/drivers/tee/optee/optee_msg.h
> >> @@ -31,6 +31,9 @@
> >>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
> >>  #define OPTEE_MSG_ATTR_TYPE_FMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
> >>  #define OPTEE_MSG_ATTR_TYPE_FMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
> >> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INPUT         OPTEE_MSG_ATTR_TYPE_RMEM_INPUT
> >> +#define OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT                OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT
> >> +#define OPTEE_MSG_ATTR_TYPE_PMEM_INOUT         OPTEE_MSG_ATTR_TYPE_RMEM_INOUT
> >>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INPUT         0x9
> >>  #define OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT                0xa
> >>  #define OPTEE_MSG_ATTR_TYPE_TMEM_INOUT         0xb
> >> @@ -149,6 +152,23 @@ struct optee_msg_param_fmem {
> >>         u64 global_id;
> >>  };
> >>
> >> +/**
> >> + * struct optee_msg_param_pmem - RPMI parcel memory reference
> >> + * @offs: full-width byte offset from the parcel's first byte
> >> + * @size: logical reference size, or required size for a short-buffer response
> >> + * @parcel_id: firmware-assigned parcel identifier
> >> + * @nonce: nonzero REE nonce supplied when sharing the parcel
> >> + *
> >> + * A zero parcel ID and nonce encode a NULL reference. Its offset must be zero;
> >> + * its size is preserved. Parcel ID zero remains valid with a nonzero nonce.
> >> + */
> >> +struct optee_msg_param_pmem {
> >
> > Without an internal offset like in struct optee_msg_param_fmem, an
> > explicit register SHM call into OP-TEE is needed before it can be
> > used. You'd still need the TEE_MEMORY_PARCEL_CREATE, but we can save
> > one round-trip into the secure world.
> >
>
> The original intenation was to use parcel-relative offsets, with the
> secure-side memory object covers the entire parcel. OP-TEE can retrieve it
> lazily and apply the supplied offset directly, so this design does not require
> an explicit SHM registration call or an additional round trip.
>
> Since we are implementing the RPMI parcel memory-object cache independently of
> FF-A's cache in OP-TEE, this seems simpler: it avoids a separate
> initial-offset property, preserves a full-width offset reference, while
> keeping OP-TEE ignorant from the SHM concept which is a Linux side concept.
>
> That said, I can separate the offsets to matching FF-A's memory-object
> model. But I am not sure what we achive?

I think it's needed, but I don't mind being proven wrong. :-)

IIRC, this stems from TEE_IOC_SHM_REGISTER. In hindsight, I regret
that offset. We should have kept it in userspace, but that's too late
now.

>
> >> +       u64 offs;
> >> +       u64 size;
> >> +       u32 parcel_id;
> >> +       u32 nonce;
> >
> > I wonder if parcel_id and nonce wouldn't be better combined into a
> > single field. They need to be separate when preparing arguments for
> > the TEE_MEMORY_* calls. Everywhere else, it's only an opaque memory
> > handle, and one handle is easier to keep track of than two.
>
> I thought about that before. This would push the conversion to the
> firmware boundary. I was not sure if it is acceptable. I'll do that :).

Sounds good to me.

>
> >
> >> +};
> >> +
> >>  /**
> >>   * struct optee_msg_param_value - opaque value parameter
> >>   * @a: first opaque value
> >> @@ -166,18 +186,19 @@ struct optee_msg_param_value {
> >>  /**
> >>   * struct optee_msg_param - parameter used together with struct optee_msg_arg
> >>   * @attr:      attributes
> >> - * @tmem:      parameter by temporary memory reference
> >> - * @rmem:      parameter by registered memory reference
> >> - * @fmem:      parameter by FF-A registered memory reference
> >> - * @value:     parameter by opaque value
> >> - * @octets:    parameter by octet string
> >> + * @u.tmem:    parameter by temporary memory reference
> >> + * @u.rmem:    parameter by registered memory reference
> >> + * @u.fmem:    parameter by FF-A registered memory reference
> >> + * @u.pmem:    parameter by RPMI parcel memory reference
> >> + * @u.value:   parameter by opaque value
> >> + * @u.octets:  parameter by octet string
> >>   * @u:         union holding OP-TEE msg parameter
> >>   *
> >>   * @attr & OPTEE_MSG_ATTR_TYPE_MASK indicates if tmem, rmem or value is used in
> >>   * the union. OPTEE_MSG_ATTR_TYPE_VALUE_* indicates value or octets,
> >> - * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @tmem and
> >> - * OPTEE_MSG_ATTR_TYPE_RMEM_* or the alias PTEE_MSG_ATTR_TYPE_FMEM_* indicates
> >> - * @rmem or @fmem depending on the conduit.
> >> + * OPTEE_MSG_ATTR_TYPE_TMEM_* indicates @u.tmem. OPTEE_MSG_ATTR_TYPE_RMEM_*
> >> + * and its FMEM/PMEM aliases indicate @u.rmem, @u.fmem or @u.pmem depending
> >> + * on the conduit.
> >>   * OPTEE_MSG_ATTR_TYPE_NONE indicates that none of the members are used.
> >>   */
> >>  struct optee_msg_param {
> >> @@ -186,6 +207,7 @@ struct optee_msg_param {
> >>                 struct optee_msg_param_tmem tmem;
> >>                 struct optee_msg_param_rmem rmem;
> >>                 struct optee_msg_param_fmem fmem;
> >> +               struct optee_msg_param_pmem pmem;
> >>                 struct optee_msg_param_value value;
> >>                 u8 octets[24];
> >>         } u;
> >> diff --git a/drivers/tee/optee/optee_rpmi.h b/drivers/tee/optee/optee_rpmi.h
> >> new file mode 100644
> >> index 000000000000..252216aad09b
> >> --- /dev/null
> >> +++ b/drivers/tee/optee/optee_rpmi.h
> >> @@ -0,0 +1,234 @@
> >> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> >> +/*
> >> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> >> + */
> >> +#ifndef OPTEE_RPMI_H
> >> +#define OPTEE_RPMI_H
> >> +
> >> +#include <linux/bitops.h>
> >> +#include <linux/types.h>
> >> +#include <linux/uuid.h>
> >> +
> >> +/*
> >> + * OP-TEE service ABI over RPMI TEE_CALL.
> >> + *
> >> + * Requests and responses are carried in the TEE_CALL service payload.
> >> + * Control fields are little-endian. Response status fields contain signed
> >> + * RPMI error codes, distinct from the outer TEE_CALL status and the GP
> >> + * command result in optee_msg_arg.ret.
> >> + */
> >> +#define OPTEE_RPMI_SERVICE_UUID \
> >> +       UUID_INIT(0x486178e0, 0xe7f8, 0x11e3, \
> >> +                 0xbc, 0x5e, 0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b)
> >> +
> >> +#define OPTEE_RPMI_VERSION_MAJOR               1
> >> +#define OPTEE_RPMI_VERSION_MINOR               0
> >> +
> >> +/**
> >> + * struct optee_rpmi_probe_req - request without operation-specific arguments
> >> + * @op: GET_API_VERSION, GET_OS_VERSION or EXCHANGE_CAPABILITIES
> >> + */
> >> +struct optee_rpmi_probe_req {
> >> +       __le32 op;
> >> +} __packed;
> >> +
> >> +/**
> >> + * struct optee_rpmi_status_resp - response carrying only an RPMI status
> >> + * @status: signed RPMI error code
> >> + */
> >> +struct optee_rpmi_status_resp {
> >> +       __le32 status;
> >> +} __packed;
> >> +
> >> +/*
> >> + * Return the service API version.
> >> + *
> >> + * Request:  struct optee_rpmi_probe_req
> >> + * Response: struct optee_rpmi_api_resp
> >> + */
> >> +#define OPTEE_RPMI_GET_API_VERSION     0
> >> +
> >> +/**
> >> + * struct optee_rpmi_api_resp - GET_API_VERSION response
> >> + * @status: signed RPMI error code
> >> + * @major: incompatible protocol revision
> >> + * @minor: compatible protocol revision
> >> + */
> >> +struct optee_rpmi_api_resp {
> >> +       __le32 status;
> >> +       __le32 major;
> >> +       __le32 minor;
> >> +} __packed;
> >
> > Why do these communication structs have to be packed? With careful
> > design of the layout, padding, alignment, etc, it shouldn't be an
> > issue.
>
> Agreed. The structures already have naturally aligned fields and
> explicit reserved fields where needed, so `__packed` is unnecessary for
> their current layouts. I'll remove it and keep the wire layout explicitly
> defined, retaining unaligned access helpers where transport-buffer alignment
> is not guaranteed.

Good.

Cheers,
Jens

[PATCH RFC v2 3/8] tee: optee: add RPMI shared-memory and parameter support

[Amirreza Zarrabi]

OP-TEE commands and RPCs need shared memory that secure world can
identify through RPMI parcels.

Register normal-world pages as read-write parcels shared with the
OP-TEE endpoint. Track each parcel ID and nonce in a hash table and
store the combined identity in tee_shm.sec_world_id. Use a fixed
nonzero nonce to distinguish registered memory from NULL references.

Add conversions between TEE parameters and parcel memory references.

On client memory unregistration, ask OP-TEE to release its mapping
before reclaiming the parcel. Supplicant memory has already been
released by OP-TEE through its SHM_FREE RPC and only needs reclaiming.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/Makefile        |   1 +
 drivers/tee/optee/optee_private.h |  27 +++
 drivers/tee/optee/rpmi_abi.c      | 417 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 445 insertions(+)

diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
index 183cdde1ac04..8576cc73a922 100644
--- a/drivers/tee/optee/Makefile
+++ b/drivers/tee/optee/Makefile
@@ -9,6 +9,7 @@ optee-objs += supp.o
 optee-objs += device.o
 optee-$(CONFIG_HAVE_ARM_SMCCC) += smc_abi.o
 optee-$(CONFIG_ARM_FFA_TRANSPORT) += ffa_abi.o
+optee-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) += rpmi_abi.o
 
 # for tracing framework to find optee_trace.h
 CFLAGS_smc_abi.o := -I$(src)
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 02d6f79df407..2422caf3c883 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -180,6 +180,28 @@ struct optee_ffa {
 };
 #endif
 
+#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
+struct rpmi_tee_device;
+
+/**
+ * struct optee_rpmi - RPMI shared-memory identity state
+ * @rdev: owning RPMI service device
+ * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
+ * @shm_rht: lookup by the host-endian parcel ID and nonce pair
+ *
+ * Callers keep their tee_shm alive while using its registration. Lookup
+ * returns a raw pointer; the mutex does not protect its lifetime after
+ * unlocking. Never hold @shm_rht_lock across a transport operation, RPC or
+ * thread-availability wait.
+ */
+struct optee_rpmi {
+	struct rpmi_tee_device *rdev;
+	/* Protects parcel lookup, insertion, removal and publication. */
+	struct mutex shm_rht_lock;
+	struct rhashtable shm_rht;
+};
+#endif
+
 struct optee;
 
 /**
@@ -240,6 +262,7 @@ struct optee_ops {
  * @ctx:			driver internal TEE context
  * @smc:			specific to SMC ABI
  * @ffa:			specific to FF-A ABI
+ * @rpmi:			specific to RPMI ABI
  * @shm_arg_cache:		shared memory cache argument
  * @call_queue:			queue of threads waiting to call @invoke_fn
  * @notif:			notification synchronization struct
@@ -271,6 +294,9 @@ struct optee {
 #endif
 #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
 		struct optee_ffa ffa;
+#endif
+#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
+		struct optee_rpmi rpmi;
 #endif
 	};
 	struct optee_shm_arg_cache shm_arg_cache;
@@ -464,4 +490,5 @@ static inline void optee_ffa_abi_unregister(void)
 }
 #endif
 
+
 #endif /*OPTEE_PRIVATE_H*/
diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
new file mode 100644
index 000000000000..e7fc853cfb15
--- /dev/null
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cleanup.h>
+#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/overflow.h>
+#include <linux/rpmi_tee.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+#include "optee_private.h"
+#include "optee_rpmi.h"
+
+/* Nonzero nonce keeps parcel ID zero distinct from a null reference. */
+#define OPTEE_RPMI_SHM_NONCE	1
+
+struct optee_rpmi_parcel_key {
+	u32 parcel_id;
+	u32 nonce;
+};
+
+struct optee_rpmi_shm_rht_entry {
+	struct rhash_head node;
+	struct optee_rpmi_parcel_key key;
+	struct tee_shm *shm;
+};
+
+static const struct rhashtable_params optee_rpmi_shm_rht_params = {
+	.head_offset = offsetof(struct optee_rpmi_shm_rht_entry, node),
+	.key_offset = offsetof(struct optee_rpmi_shm_rht_entry, key),
+	.key_len = sizeof(struct optee_rpmi_parcel_key),
+	.automatic_shrinking = true,
+};
+
+/* Keep transport errors separate from the control status in a response. */
+static int optee_rpmi_call_with_status(struct optee *optee,
+				       const void *req, size_t req_len,
+				       void *resp, size_t resp_size,
+				       s32 *status)
+{
+	struct rpmi_tee_device *rdev = optee->rpmi.rdev;
+	size_t received = resp_size;
+	int ret;
+
+	ret = rdev->ops->msg_ops->call(rdev, req, req_len, resp, &received);
+	if (ret)
+		return ret;
+
+	if (received != resp_size)
+		return -EPROTO;
+
+	*status = get_unaligned_le32(resp);
+
+	return 0;
+}
+
+/**
+ * optee_rpmi_call - Send a control request and decode its RPMI status
+ * @optee: OP-TEE instance.
+ * @req: Control request, including the operation number.
+ * @req_len: Request size in bytes.
+ * @resp: Response buffer, beginning with a little-endian RPMI status.
+ * @resp_size: Exact expected response size, including the status field.
+ *
+ * Return: 0 on success, a transport error, -EPROTO for an unexpected response
+ * size, or the control status converted to a Linux error code.
+ */
+static int optee_rpmi_call(struct optee *optee, const void *req, size_t req_len,
+			   void *resp, size_t resp_size)
+{
+	s32 status;
+	int ret;
+
+	ret = optee_rpmi_call_with_status(optee, req, req_len, resp, resp_size,
+					  &status);
+	if (ret)
+		return ret;
+
+	return rpmi_to_linux_error(status);
+}
+
+static int optee_rpmi_shm_rht_init(struct optee *optee)
+{
+	int ret;
+
+	mutex_init(&optee->rpmi.shm_rht_lock);
+	ret = rhashtable_init(&optee->rpmi.shm_rht, &optee_rpmi_shm_rht_params);
+	if (ret)
+		mutex_destroy(&optee->rpmi.shm_rht_lock);
+
+	return ret;
+}
+
+static void optee_rpmi_shm_rht_free(void *ptr, void *arg)
+{
+	kfree(ptr);
+}
+
+static void optee_rpmi_shm_rht_uninit(struct optee *optee)
+{
+	rhashtable_free_and_destroy(&optee->rpmi.shm_rht,
+				    optee_rpmi_shm_rht_free, NULL);
+	mutex_destroy(&optee->rpmi.shm_rht_lock);
+}
+
+/* Allocate and publish a parcel-to-SHM mapping. */
+static int optee_rpmi_shm_rht_add(struct optee *optee, struct tee_shm *shm,
+				  u32 parcel_id, u32 nonce)
+{
+	struct optee_rpmi_shm_rht_entry *entry;
+	int ret;
+
+	entry = kzalloc_obj(*entry);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->shm = shm;
+	entry->key.parcel_id = parcel_id;
+	entry->key.nonce = nonce;
+
+	scoped_guard(mutex, &optee->rpmi.shm_rht_lock)
+		ret = rhashtable_lookup_insert_fast(&optee->rpmi.shm_rht,
+						    &entry->node,
+						    optee_rpmi_shm_rht_params);
+	if (ret)
+		kfree(entry);
+
+	return ret;
+}
+
+/* Remove and free a parcel-to-SHM mapping. */
+static int optee_rpmi_shm_rht_rm(struct optee *optee, u32 parcel_id, u32 nonce)
+{
+	struct optee_rpmi_shm_rht_entry *entry;
+	struct optee_rpmi_parcel_key key = {
+		.parcel_id = parcel_id,
+		.nonce = nonce,
+	};
+	int ret = -ENOENT;
+
+	scoped_guard(mutex, &optee->rpmi.shm_rht_lock) {
+		entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
+					       optee_rpmi_shm_rht_params);
+		if (entry)
+			ret = rhashtable_remove_fast(&optee->rpmi.shm_rht,
+						     &entry->node,
+						     optee_rpmi_shm_rht_params);
+	}
+
+	if (!ret)
+		kfree(entry);
+
+	return ret;
+}
+
+/* Return a raw pointer; the surrounding call or RPC owns the SHM lifetime. */
+static struct tee_shm *
+optee_rpmi_get_shm_for_parcel(struct optee *optee, u32 parcel_id, u32 nonce)
+{
+	struct optee_rpmi_shm_rht_entry *entry;
+	struct optee_rpmi_parcel_key key = {
+		.parcel_id = parcel_id,
+		.nonce = nonce,
+	};
+
+	guard(mutex)(&optee->rpmi.shm_rht_lock);
+	entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
+				       optee_rpmi_shm_rht_params);
+
+	return entry ? entry->shm : NULL;
+}
+
+/* Extract the parcel ID and nonce stored in the SHM identity. */
+static void optee_rpmi_shm_get_identity(const struct tee_shm *shm,
+					u32 *parcel_id, u32 *nonce)
+{
+	*parcel_id = lower_32_bits(shm->sec_world_id);
+	*nonce = upper_32_bits(shm->sec_world_id);
+}
+
+static int optee_rpmi_shm_register(struct tee_context *ctx, struct tee_shm *shm,
+				   struct page **pages, size_t num_pages,
+				   unsigned long start)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+	struct rpmi_tee_device *rdev = optee->rpmi.rdev;
+	struct rpmi_tee_mem_receiver receiver = {
+		.endpoint_id = rdev->endpoint_id,
+		.access = RPMI_TEE_MEM_ACCESS_READ | RPMI_TEE_MEM_ACCESS_WRITE,
+	};
+	struct rpmi_tee_mem_args args = {
+		.nonce = OPTEE_RPMI_SHM_NONCE,
+		.receivers = &receiver,
+		.receiver_count = 1,
+		.creator_access = RPMI_TEE_MEM_ACCESS_READ |
+				  RPMI_TEE_MEM_ACCESS_WRITE,
+	};
+	struct sg_table sgt;
+	int ret;
+
+	ret = optee_check_mem_type(start, num_pages);
+	if (ret)
+		return ret;
+
+	ret = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
+					num_pages * PAGE_SIZE, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	args.sg = sgt.sgl;
+	ret = rdev->ops->mem_ops->memory_share(rdev, &args);
+	sg_free_table(&sgt);
+	if (ret)
+		return ret;
+
+	ret = optee_rpmi_shm_rht_add(optee, shm, args.parcel_id, args.nonce);
+	if (ret) {
+		int reclaim_ret;
+
+		reclaim_ret = rdev->ops->mem_ops->memory_reclaim(rdev, args.parcel_id);
+		if (reclaim_ret)
+			dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
+				args.parcel_id, reclaim_ret);
+		return ret;
+	}
+
+	shm->sec_world_id = ((u64)args.nonce << 32) | args.parcel_id;
+
+	return 0;
+}
+
+static int optee_rpmi_shm_unregister(struct tee_context *ctx,
+				     struct tee_shm *shm)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+	struct rpmi_tee_device *rdev = optee->rpmi.rdev;
+	struct optee_rpmi_unregister_req req;
+	struct optee_rpmi_status_resp resp;
+	u32 parcel_id, nonce;
+	int ret;
+
+	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
+	optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
+	shm->sec_world_id = 0;
+
+	req.op = cpu_to_le32(OPTEE_RPMI_UNREGISTER_SHM);
+	req.parcel_id = cpu_to_le32(parcel_id);
+	req.nonce = cpu_to_le32(nonce);
+	ret = optee_rpmi_call(optee, &req, sizeof(req), &resp, sizeof(resp));
+	if (ret)
+		dev_err(&rdev->dev, "unregister parcel %#x failed: %d\n",
+			parcel_id, ret);
+
+	ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
+	if (ret)
+		dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
+			parcel_id, ret);
+
+	return ret;
+}
+
+static int optee_rpmi_shm_unregister_supp(struct tee_context *ctx,
+					  struct tee_shm *shm)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+	struct rpmi_tee_device *rdev = optee->rpmi.rdev;
+	u32 parcel_id, nonce;
+	int ret;
+
+	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
+	optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
+	shm->sec_world_id = 0;
+	/* OP-TEE has already retired the parcel through SHM_FREE RPC. */
+	ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
+	if (ret)
+		dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
+			parcel_id, ret);
+
+	return ret;
+}
+
+/* Convert a memory reference to an OP-TEE RPMI parcel reference. */
+static int to_msg_param_rpmi_mem(struct optee_msg_param *mp,
+				 const struct tee_param *p)
+{
+	struct tee_shm *shm = p->u.memref.shm;
+
+	mp->attr = OPTEE_MSG_ATTR_TYPE_PMEM_INPUT + p->attr -
+		   TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
+	memset(&mp->u, 0, sizeof(mp->u));
+	/* For !shm, return parcel_id = 0 and nonce = 0 to represent NULL. */
+	if (shm) {
+		if (check_add_overflow((u64)shm->offset,
+				       (u64)p->u.memref.shm_offs,
+				       &mp->u.pmem.offs))
+			return -EINVAL;
+
+		optee_rpmi_shm_get_identity(shm, &mp->u.pmem.parcel_id,
+					    &mp->u.pmem.nonce);
+	}
+
+	mp->u.pmem.size = p->u.memref.size;
+
+	return 0;
+}
+
+static int optee_rpmi_to_msg_param(struct optee *optee,
+				   struct optee_msg_param *msg_params,
+				   size_t num_params,
+				   const struct tee_param *params)
+{
+	size_t n;
+
+	for (n = 0; n < num_params; n++) {
+		const struct tee_param *p = params + n;
+		struct optee_msg_param *mp = msg_params + n;
+		switch (p->attr) {
+		case TEE_IOCTL_PARAM_ATTR_TYPE_NONE:
+			mp->attr = OPTEE_MSG_ATTR_TYPE_NONE;
+			memset(&mp->u, 0, sizeof(mp->u));
+			break;
+		case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT:
+		case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT:
+		case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT:
+			optee_to_msg_param_value(mp, p);
+			break;
+		case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
+		case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
+		case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
+			if (to_msg_param_rpmi_mem(mp, p))
+				return -EINVAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+/* Convert an RPMI parcel reference to a memref; callers own SHM lifetime. */
+static int from_msg_param_rpmi_mem(struct optee *optee, struct tee_param *p,
+				   u32 attr, const struct optee_msg_param *mp)
+{
+	struct tee_shm *shm;
+	u64 offset;
+
+	p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT + attr -
+		  OPTEE_MSG_ATTR_TYPE_PMEM_INPUT;
+
+	if (mp->u.pmem.size > SIZE_MAX)
+		return -EOVERFLOW;
+	p->u.memref.size = mp->u.pmem.size;
+
+	if (!mp->u.pmem.nonce) {
+		/* Return NULL shm. */
+		if (mp->u.pmem.offs || mp->u.pmem.parcel_id)
+			return -EINVAL;
+		p->u.memref.shm = NULL;
+		p->u.memref.shm_offs = 0;
+		return 0;
+	}
+
+	shm = optee_rpmi_get_shm_for_parcel(optee, mp->u.pmem.parcel_id,
+					    mp->u.pmem.nonce);
+	if (!shm || mp->u.pmem.offs < shm->offset)
+		return -EINVAL;
+
+	offset = mp->u.pmem.offs - shm->offset;
+	if (offset > SIZE_MAX)
+		return -EOVERFLOW;
+
+	p->u.memref.shm = shm;
+	p->u.memref.shm_offs = offset;
+
+	return 0;
+}
+
+static int optee_rpmi_from_msg_param(struct optee *optee,
+				     struct tee_param *params,
+				     size_t num_params,
+				     const struct optee_msg_param *msg_params)
+{
+	size_t n;
+
+	for (n = 0; n < num_params; n++) {
+		const struct optee_msg_param *mp = msg_params + n;
+		struct tee_param *p = params + n;
+		u32 attr = mp->attr & OPTEE_MSG_ATTR_TYPE_MASK;
+		int ret;
+
+		switch (attr) {
+		case OPTEE_MSG_ATTR_TYPE_NONE:
+			p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_NONE;
+			memset(&p->u, 0, sizeof(p->u));
+			break;
+		case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
+		case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
+		case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
+			optee_from_msg_param_value(p, attr, mp);
+			break;
+		case OPTEE_MSG_ATTR_TYPE_PMEM_INPUT:
+		case OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT:
+		case OPTEE_MSG_ATTR_TYPE_PMEM_INOUT:
+			ret = from_msg_param_rpmi_mem(optee, p, attr, mp);
+			if (ret)
+				return ret;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
+}

-- 
2.34.1

Re: [PATCH RFC v2 3/8] tee: optee: add RPMI shared-memory and parameter support

[Jens Wiklander]

Hi Amir,

On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> OP-TEE commands and RPCs need shared memory that secure world can
> identify through RPMI parcels.
>
> Register normal-world pages as read-write parcels shared with the
> OP-TEE endpoint. Track each parcel ID and nonce in a hash table and
> store the combined identity in tee_shm.sec_world_id. Use a fixed
> nonzero nonce to distinguish registered memory from NULL references.
>
> Add conversions between TEE parameters and parcel memory references.
>
> On client memory unregistration, ask OP-TEE to release its mapping
> before reclaiming the parcel. Supplicant memory has already been
> released by OP-TEE through its SHM_FREE RPC and only needs reclaiming.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
>  drivers/tee/optee/Makefile        |   1 +
>  drivers/tee/optee/optee_private.h |  27 +++
>  drivers/tee/optee/rpmi_abi.c      | 417 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 445 insertions(+)
>
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index 183cdde1ac04..8576cc73a922 100644
> --- a/drivers/tee/optee/Makefile
> +++ b/drivers/tee/optee/Makefile
> @@ -9,6 +9,7 @@ optee-objs += supp.o
>  optee-objs += device.o
>  optee-$(CONFIG_HAVE_ARM_SMCCC) += smc_abi.o
>  optee-$(CONFIG_ARM_FFA_TRANSPORT) += ffa_abi.o
> +optee-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) += rpmi_abi.o
>
>  # for tracing framework to find optee_trace.h
>  CFLAGS_smc_abi.o := -I$(src)
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 02d6f79df407..2422caf3c883 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -180,6 +180,28 @@ struct optee_ffa {
>  };
>  #endif
>
> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
> +struct rpmi_tee_device;
> +
> +/**
> + * struct optee_rpmi - RPMI shared-memory identity state
> + * @rdev: owning RPMI service device
> + * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
> + * @shm_rht: lookup by the host-endian parcel ID and nonce pair
> + *
> + * Callers keep their tee_shm alive while using its registration. Lookup
> + * returns a raw pointer; the mutex does not protect its lifetime after
> + * unlocking. Never hold @shm_rht_lock across a transport operation, RPC or
> + * thread-availability wait.
> + */
> +struct optee_rpmi {
> +       struct rpmi_tee_device *rdev;
> +       /* Protects parcel lookup, insertion, removal and publication. */
> +       struct mutex shm_rht_lock;
> +       struct rhashtable shm_rht;
> +};
> +#endif
> +
>  struct optee;
>
>  /**
> @@ -240,6 +262,7 @@ struct optee_ops {
>   * @ctx:                       driver internal TEE context
>   * @smc:                       specific to SMC ABI
>   * @ffa:                       specific to FF-A ABI
> + * @rpmi:                      specific to RPMI ABI
>   * @shm_arg_cache:             shared memory cache argument
>   * @call_queue:                        queue of threads waiting to call @invoke_fn
>   * @notif:                     notification synchronization struct
> @@ -271,6 +294,9 @@ struct optee {
>  #endif
>  #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
>                 struct optee_ffa ffa;
> +#endif
> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
> +               struct optee_rpmi rpmi;
>  #endif
>         };
>         struct optee_shm_arg_cache shm_arg_cache;
> @@ -464,4 +490,5 @@ static inline void optee_ffa_abi_unregister(void)
>  }
>  #endif
>
> +
>  #endif /*OPTEE_PRIVATE_H*/
> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> new file mode 100644
> index 000000000000..e7fc853cfb15
> --- /dev/null
> +++ b/drivers/tee/optee/rpmi_abi.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/cleanup.h>
> +#include <linux/mailbox/riscv-rpmi-message.h>
> +#include <linux/overflow.h>
> +#include <linux/rpmi_tee.h>
> +#include <linux/slab.h>
> +#include <linux/unaligned.h>
> +#include "optee_private.h"
> +#include "optee_rpmi.h"
> +
> +/* Nonzero nonce keeps parcel ID zero distinct from a null reference. */
> +#define OPTEE_RPMI_SHM_NONCE   1
The spec describes this as:

A token nonce which receivers will need to present to the framework,
together with MEM_PARCEL_ID, to accept the memory.
It is intended as a way to reduce the likelihood of accidental
collisions on MEM_PARCEL_ID values, which can be reused by the
framework after they have been destroyed.

Why don't we change the nonce with each new parcel to live up to that?

> +
> +struct optee_rpmi_parcel_key {
> +       u32 parcel_id;
> +       u32 nonce;
> +};
> +
> +struct optee_rpmi_shm_rht_entry {
> +       struct rhash_head node;
> +       struct optee_rpmi_parcel_key key;
> +       struct tee_shm *shm;
> +};
> +
> +static const struct rhashtable_params optee_rpmi_shm_rht_params = {
> +       .head_offset = offsetof(struct optee_rpmi_shm_rht_entry, node),
> +       .key_offset = offsetof(struct optee_rpmi_shm_rht_entry, key),
> +       .key_len = sizeof(struct optee_rpmi_parcel_key),
> +       .automatic_shrinking = true,
> +};
> +
> +/* Keep transport errors separate from the control status in a response. */
> +static int optee_rpmi_call_with_status(struct optee *optee,
> +                                      const void *req, size_t req_len,
> +                                      void *resp, size_t resp_size,
> +                                      s32 *status)
> +{
> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
> +       size_t received = resp_size;
> +       int ret;
> +
> +       ret = rdev->ops->msg_ops->call(rdev, req, req_len, resp, &received);
> +       if (ret)
> +               return ret;
> +
> +       if (received != resp_size)
> +               return -EPROTO;
> +
> +       *status = get_unaligned_le32(resp);
> +
> +       return 0;
> +}
> +
> +/**
> + * optee_rpmi_call - Send a control request and decode its RPMI status
> + * @optee: OP-TEE instance.
> + * @req: Control request, including the operation number.
> + * @req_len: Request size in bytes.
> + * @resp: Response buffer, beginning with a little-endian RPMI status.
> + * @resp_size: Exact expected response size, including the status field.
> + *
> + * Return: 0 on success, a transport error, -EPROTO for an unexpected response
> + * size, or the control status converted to a Linux error code.
> + */
> +static int optee_rpmi_call(struct optee *optee, const void *req, size_t req_len,
> +                          void *resp, size_t resp_size)
> +{
> +       s32 status;
> +       int ret;
> +
> +       ret = optee_rpmi_call_with_status(optee, req, req_len, resp, resp_size,
> +                                         &status);
> +       if (ret)
> +               return ret;
> +
> +       return rpmi_to_linux_error(status);
> +}
> +
> +static int optee_rpmi_shm_rht_init(struct optee *optee)
> +{
> +       int ret;
> +
> +       mutex_init(&optee->rpmi.shm_rht_lock);
> +       ret = rhashtable_init(&optee->rpmi.shm_rht, &optee_rpmi_shm_rht_params);
> +       if (ret)
> +               mutex_destroy(&optee->rpmi.shm_rht_lock);
> +
> +       return ret;
> +}
> +
> +static void optee_rpmi_shm_rht_free(void *ptr, void *arg)
> +{
> +       kfree(ptr);
> +}
> +
> +static void optee_rpmi_shm_rht_uninit(struct optee *optee)
> +{
> +       rhashtable_free_and_destroy(&optee->rpmi.shm_rht,
> +                                   optee_rpmi_shm_rht_free, NULL);
> +       mutex_destroy(&optee->rpmi.shm_rht_lock);
> +}
> +
> +/* Allocate and publish a parcel-to-SHM mapping. */
> +static int optee_rpmi_shm_rht_add(struct optee *optee, struct tee_shm *shm,
> +                                 u32 parcel_id, u32 nonce)
> +{
> +       struct optee_rpmi_shm_rht_entry *entry;
> +       int ret;
> +
> +       entry = kzalloc_obj(*entry);
> +       if (!entry)
> +               return -ENOMEM;
> +
> +       entry->shm = shm;
> +       entry->key.parcel_id = parcel_id;
> +       entry->key.nonce = nonce;
> +
> +       scoped_guard(mutex, &optee->rpmi.shm_rht_lock)
> +               ret = rhashtable_lookup_insert_fast(&optee->rpmi.shm_rht,
> +                                                   &entry->node,
> +                                                   optee_rpmi_shm_rht_params);
> +       if (ret)
> +               kfree(entry);
> +
> +       return ret;
> +}
> +
> +/* Remove and free a parcel-to-SHM mapping. */
> +static int optee_rpmi_shm_rht_rm(struct optee *optee, u32 parcel_id, u32 nonce)
> +{
> +       struct optee_rpmi_shm_rht_entry *entry;
> +       struct optee_rpmi_parcel_key key = {
> +               .parcel_id = parcel_id,
> +               .nonce = nonce,
> +       };
> +       int ret = -ENOENT;
> +
> +       scoped_guard(mutex, &optee->rpmi.shm_rht_lock) {
> +               entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
> +                                              optee_rpmi_shm_rht_params);
> +               if (entry)
> +                       ret = rhashtable_remove_fast(&optee->rpmi.shm_rht,
> +                                                    &entry->node,
> +                                                    optee_rpmi_shm_rht_params);
> +       }
> +
> +       if (!ret)
> +               kfree(entry);
> +
> +       return ret;
> +}
> +
> +/* Return a raw pointer; the surrounding call or RPC owns the SHM lifetime. */
> +static struct tee_shm *
> +optee_rpmi_get_shm_for_parcel(struct optee *optee, u32 parcel_id, u32 nonce)
> +{
> +       struct optee_rpmi_shm_rht_entry *entry;
> +       struct optee_rpmi_parcel_key key = {
> +               .parcel_id = parcel_id,
> +               .nonce = nonce,
> +       };
> +
> +       guard(mutex)(&optee->rpmi.shm_rht_lock);
> +       entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
> +                                      optee_rpmi_shm_rht_params);
> +
> +       return entry ? entry->shm : NULL;

Please use a full if statement instead of the ternary operator

> +}
> +
> +/* Extract the parcel ID and nonce stored in the SHM identity. */
> +static void optee_rpmi_shm_get_identity(const struct tee_shm *shm,
> +                                       u32 *parcel_id, u32 *nonce)
> +{
> +       *parcel_id = lower_32_bits(shm->sec_world_id);
> +       *nonce = upper_32_bits(shm->sec_world_id);

By keeping parcel_id and nonce in separate fields, we add quite a bit
of code only to handle u64 -> u32 + u32 and vice versa. I wonder if it
wouldn't be easier always to keep them in a u64 and say that the upper
32 bits are a nonce, or something. With that, we could make the
optee_shm_rem_ffa_handle() function and friends common helpers in the
optee driver.

> +}
> +
> +static int optee_rpmi_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> +                                  struct page **pages, size_t num_pages,
> +                                  unsigned long start)
> +{
> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
> +       struct rpmi_tee_mem_receiver receiver = {
> +               .endpoint_id = rdev->endpoint_id,
> +               .access = RPMI_TEE_MEM_ACCESS_READ | RPMI_TEE_MEM_ACCESS_WRITE,
> +       };
> +       struct rpmi_tee_mem_args args = {
> +               .nonce = OPTEE_RPMI_SHM_NONCE,
> +               .receivers = &receiver,
> +               .receiver_count = 1,
> +               .creator_access = RPMI_TEE_MEM_ACCESS_READ |
> +                                 RPMI_TEE_MEM_ACCESS_WRITE,
> +       };
> +       struct sg_table sgt;
> +       int ret;
> +
> +       ret = optee_check_mem_type(start, num_pages);
> +       if (ret)
> +               return ret;
> +
> +       ret = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
> +                                       num_pages * PAGE_SIZE, GFP_KERNEL);
> +       if (ret)
> +               return ret;
> +
> +       args.sg = sgt.sgl;
> +       ret = rdev->ops->mem_ops->memory_share(rdev, &args);
> +       sg_free_table(&sgt);
> +       if (ret)
> +               return ret;
> +
> +       ret = optee_rpmi_shm_rht_add(optee, shm, args.parcel_id, args.nonce);
> +       if (ret) {
> +               int reclaim_ret;
> +
> +               reclaim_ret = rdev->ops->mem_ops->memory_reclaim(rdev, args.parcel_id);
> +               if (reclaim_ret)
> +                       dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
> +                               args.parcel_id, reclaim_ret);
> +               return ret;
> +       }
> +
> +       shm->sec_world_id = ((u64)args.nonce << 32) | args.parcel_id;
> +
> +       return 0;
> +}
> +
> +static int optee_rpmi_shm_unregister(struct tee_context *ctx,
> +                                    struct tee_shm *shm)
> +{
> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
> +       struct optee_rpmi_unregister_req req;
> +       struct optee_rpmi_status_resp resp;
> +       u32 parcel_id, nonce;
> +       int ret;
> +
> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
> +       optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
> +       shm->sec_world_id = 0;
> +
> +       req.op = cpu_to_le32(OPTEE_RPMI_UNREGISTER_SHM);
> +       req.parcel_id = cpu_to_le32(parcel_id);
> +       req.nonce = cpu_to_le32(nonce);
> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &resp, sizeof(resp));
> +       if (ret)
> +               dev_err(&rdev->dev, "unregister parcel %#x failed: %d\n",
> +                       parcel_id, ret);
> +
> +       ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
> +       if (ret)
> +               dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
> +                       parcel_id, ret);
> +
> +       return ret;
> +}
> +
> +static int optee_rpmi_shm_unregister_supp(struct tee_context *ctx,
> +                                         struct tee_shm *shm)
> +{
> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
> +       u32 parcel_id, nonce;
> +       int ret;
> +
> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
> +       optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
> +       shm->sec_world_id = 0;
> +       /* OP-TEE has already retired the parcel through SHM_FREE RPC. */
> +       ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
> +       if (ret)
> +               dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
> +                       parcel_id, ret);
> +
> +       return ret;
> +}
> +
> +/* Convert a memory reference to an OP-TEE RPMI parcel reference. */
> +static int to_msg_param_rpmi_mem(struct optee_msg_param *mp,
> +                                const struct tee_param *p)
> +{
> +       struct tee_shm *shm = p->u.memref.shm;
> +
> +       mp->attr = OPTEE_MSG_ATTR_TYPE_PMEM_INPUT + p->attr -
> +                  TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
> +       memset(&mp->u, 0, sizeof(mp->u));
> +       /* For !shm, return parcel_id = 0 and nonce = 0 to represent NULL. */
> +       if (shm) {
> +               if (check_add_overflow((u64)shm->offset,
> +                                      (u64)p->u.memref.shm_offs,
> +                                      &mp->u.pmem.offs))
> +                       return -EINVAL;

By combining the shm->offset with p->u.memref.shm_offs, OP-TEE
requires an explicit call to register the shared memory so it knows
the initial page offset. Compared with the FF-A ABI, which can tell
the initial page offset from mp->u.fmem.internal_offs. See also the
mobj_ffa_get_by_cookie() call in set_fmem_param() in
core/tee/entry_std.c in optee_os.git

> +
> +               optee_rpmi_shm_get_identity(shm, &mp->u.pmem.parcel_id,
> +                                           &mp->u.pmem.nonce);
> +       }
> +
> +       mp->u.pmem.size = p->u.memref.size;
> +
> +       return 0;
> +}
> +
> +static int optee_rpmi_to_msg_param(struct optee *optee,
> +                                  struct optee_msg_param *msg_params,
> +                                  size_t num_params,
> +                                  const struct tee_param *params)
> +{
> +       size_t n;
> +
> +       for (n = 0; n < num_params; n++) {
> +               const struct tee_param *p = params + n;
> +               struct optee_msg_param *mp = msg_params + n;

Please add an empty line after the variables.

Cheers,
Jens

> +               switch (p->attr) {
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_NONE:
> +                       mp->attr = OPTEE_MSG_ATTR_TYPE_NONE;
> +                       memset(&mp->u, 0, sizeof(mp->u));
> +                       break;
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT:
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT:
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT:
> +                       optee_to_msg_param_value(mp, p);
> +                       break;
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
> +                       if (to_msg_param_rpmi_mem(mp, p))
> +                               return -EINVAL;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +/* Convert an RPMI parcel reference to a memref; callers own SHM lifetime. */
> +static int from_msg_param_rpmi_mem(struct optee *optee, struct tee_param *p,
> +                                  u32 attr, const struct optee_msg_param *mp)
> +{
> +       struct tee_shm *shm;
> +       u64 offset;
> +
> +       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT + attr -
> +                 OPTEE_MSG_ATTR_TYPE_PMEM_INPUT;
> +
> +       if (mp->u.pmem.size > SIZE_MAX)
> +               return -EOVERFLOW;
> +       p->u.memref.size = mp->u.pmem.size;
> +
> +       if (!mp->u.pmem.nonce) {
> +               /* Return NULL shm. */
> +               if (mp->u.pmem.offs || mp->u.pmem.parcel_id)
> +                       return -EINVAL;
> +               p->u.memref.shm = NULL;
> +               p->u.memref.shm_offs = 0;
> +               return 0;
> +       }
> +
> +       shm = optee_rpmi_get_shm_for_parcel(optee, mp->u.pmem.parcel_id,
> +                                           mp->u.pmem.nonce);
> +       if (!shm || mp->u.pmem.offs < shm->offset)
> +               return -EINVAL;
> +
> +       offset = mp->u.pmem.offs - shm->offset;
> +       if (offset > SIZE_MAX)
> +               return -EOVERFLOW;
> +
> +       p->u.memref.shm = shm;
> +       p->u.memref.shm_offs = offset;
> +
> +       return 0;
> +}
> +
> +static int optee_rpmi_from_msg_param(struct optee *optee,
> +                                    struct tee_param *params,
> +                                    size_t num_params,
> +                                    const struct optee_msg_param *msg_params)
> +{
> +       size_t n;
> +
> +       for (n = 0; n < num_params; n++) {
> +               const struct optee_msg_param *mp = msg_params + n;
> +               struct tee_param *p = params + n;
> +               u32 attr = mp->attr & OPTEE_MSG_ATTR_TYPE_MASK;
> +               int ret;
> +
> +               switch (attr) {
> +               case OPTEE_MSG_ATTR_TYPE_NONE:
> +                       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_NONE;
> +                       memset(&p->u, 0, sizeof(p->u));
> +                       break;
> +               case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
> +               case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
> +               case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
> +                       optee_from_msg_param_value(p, attr, mp);
> +                       break;
> +               case OPTEE_MSG_ATTR_TYPE_PMEM_INPUT:
> +               case OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT:
> +               case OPTEE_MSG_ATTR_TYPE_PMEM_INOUT:
> +                       ret = from_msg_param_rpmi_mem(optee, p, attr, mp);
> +                       if (ret)
> +                               return ret;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +       }
> +       return 0;
> +}
>
> --
> 2.34.1
>

Re: [PATCH RFC v2 3/8] tee: optee: add RPMI shared-memory and parameter support

[Amirreza Zarrabi]

Hi Jens,

On 10/8/2026 6:10 PM, Jens Wiklander wrote:
> Hi Amir,
> 
> On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> OP-TEE commands and RPCs need shared memory that secure world can
>> identify through RPMI parcels.
>>
>> Register normal-world pages as read-write parcels shared with the
>> OP-TEE endpoint. Track each parcel ID and nonce in a hash table and
>> store the combined identity in tee_shm.sec_world_id. Use a fixed
>> nonzero nonce to distinguish registered memory from NULL references.
>>
>> Add conversions between TEE parameters and parcel memory references.
>>
>> On client memory unregistration, ask OP-TEE to release its mapping
>> before reclaiming the parcel. Supplicant memory has already been
>> released by OP-TEE through its SHM_FREE RPC and only needs reclaiming.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>>  drivers/tee/optee/Makefile        |   1 +
>>  drivers/tee/optee/optee_private.h |  27 +++
>>  drivers/tee/optee/rpmi_abi.c      | 417 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 445 insertions(+)
>>
>> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
>> index 183cdde1ac04..8576cc73a922 100644
>> --- a/drivers/tee/optee/Makefile
>> +++ b/drivers/tee/optee/Makefile
>> @@ -9,6 +9,7 @@ optee-objs += supp.o
>>  optee-objs += device.o
>>  optee-$(CONFIG_HAVE_ARM_SMCCC) += smc_abi.o
>>  optee-$(CONFIG_ARM_FFA_TRANSPORT) += ffa_abi.o
>> +optee-$(CONFIG_RISCV_RPMI_TEE_TRANSPORT) += rpmi_abi.o
>>
>>  # for tracing framework to find optee_trace.h
>>  CFLAGS_smc_abi.o := -I$(src)
>> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
>> index 02d6f79df407..2422caf3c883 100644
>> --- a/drivers/tee/optee/optee_private.h
>> +++ b/drivers/tee/optee/optee_private.h
>> @@ -180,6 +180,28 @@ struct optee_ffa {
>>  };
>>  #endif
>>
>> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
>> +struct rpmi_tee_device;
>> +
>> +/**
>> + * struct optee_rpmi - RPMI shared-memory identity state
>> + * @rdev: owning RPMI service device
>> + * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
>> + * @shm_rht: lookup by the host-endian parcel ID and nonce pair
>> + *
>> + * Callers keep their tee_shm alive while using its registration. Lookup
>> + * returns a raw pointer; the mutex does not protect its lifetime after
>> + * unlocking. Never hold @shm_rht_lock across a transport operation, RPC or
>> + * thread-availability wait.
>> + */
>> +struct optee_rpmi {
>> +       struct rpmi_tee_device *rdev;
>> +       /* Protects parcel lookup, insertion, removal and publication. */
>> +       struct mutex shm_rht_lock;
>> +       struct rhashtable shm_rht;
>> +};
>> +#endif
>> +
>>  struct optee;
>>
>>  /**
>> @@ -240,6 +262,7 @@ struct optee_ops {
>>   * @ctx:                       driver internal TEE context
>>   * @smc:                       specific to SMC ABI
>>   * @ffa:                       specific to FF-A ABI
>> + * @rpmi:                      specific to RPMI ABI
>>   * @shm_arg_cache:             shared memory cache argument
>>   * @call_queue:                        queue of threads waiting to call @invoke_fn
>>   * @notif:                     notification synchronization struct
>> @@ -271,6 +294,9 @@ struct optee {
>>  #endif
>>  #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
>>                 struct optee_ffa ffa;
>> +#endif
>> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
>> +               struct optee_rpmi rpmi;
>>  #endif
>>         };
>>         struct optee_shm_arg_cache shm_arg_cache;
>> @@ -464,4 +490,5 @@ static inline void optee_ffa_abi_unregister(void)
>>  }
>>  #endif
>>
>> +
>>  #endif /*OPTEE_PRIVATE_H*/
>> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
>> new file mode 100644
>> index 000000000000..e7fc853cfb15
>> --- /dev/null
>> +++ b/drivers/tee/optee/rpmi_abi.c
>> @@ -0,0 +1,417 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/cleanup.h>
>> +#include <linux/mailbox/riscv-rpmi-message.h>
>> +#include <linux/overflow.h>
>> +#include <linux/rpmi_tee.h>
>> +#include <linux/slab.h>
>> +#include <linux/unaligned.h>
>> +#include "optee_private.h"
>> +#include "optee_rpmi.h"
>> +
>> +/* Nonzero nonce keeps parcel ID zero distinct from a null reference. */
>> +#define OPTEE_RPMI_SHM_NONCE   1
> The spec describes this as:
> 
> A token nonce which receivers will need to present to the framework,
> together with MEM_PARCEL_ID, to accept the memory.
> It is intended as a way to reduce the likelihood of accidental
> collisions on MEM_PARCEL_ID values, which can be reused by the
> framework after they have been destroyed.
> 
> Why don't we change the nonce with each new parcel to live up to that?
> 

True, That's the intention. I just set it to a constsnt to reduce the
code size so we can concentrate on the ABI. I'll include the nonce
allocation in the next version.

>> +
>> +struct optee_rpmi_parcel_key {
>> +       u32 parcel_id;
>> +       u32 nonce;
>> +};
>> +
>> +struct optee_rpmi_shm_rht_entry {
>> +       struct rhash_head node;
>> +       struct optee_rpmi_parcel_key key;
>> +       struct tee_shm *shm;
>> +};
>> +
>> +static const struct rhashtable_params optee_rpmi_shm_rht_params = {
>> +       .head_offset = offsetof(struct optee_rpmi_shm_rht_entry, node),
>> +       .key_offset = offsetof(struct optee_rpmi_shm_rht_entry, key),
>> +       .key_len = sizeof(struct optee_rpmi_parcel_key),
>> +       .automatic_shrinking = true,
>> +};
>> +
>> +/* Keep transport errors separate from the control status in a response. */
>> +static int optee_rpmi_call_with_status(struct optee *optee,
>> +                                      const void *req, size_t req_len,
>> +                                      void *resp, size_t resp_size,
>> +                                      s32 *status)
>> +{
>> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
>> +       size_t received = resp_size;
>> +       int ret;
>> +
>> +       ret = rdev->ops->msg_ops->call(rdev, req, req_len, resp, &received);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (received != resp_size)
>> +               return -EPROTO;
>> +
>> +       *status = get_unaligned_le32(resp);
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * optee_rpmi_call - Send a control request and decode its RPMI status
>> + * @optee: OP-TEE instance.
>> + * @req: Control request, including the operation number.
>> + * @req_len: Request size in bytes.
>> + * @resp: Response buffer, beginning with a little-endian RPMI status.
>> + * @resp_size: Exact expected response size, including the status field.
>> + *
>> + * Return: 0 on success, a transport error, -EPROTO for an unexpected response
>> + * size, or the control status converted to a Linux error code.
>> + */
>> +static int optee_rpmi_call(struct optee *optee, const void *req, size_t req_len,
>> +                          void *resp, size_t resp_size)
>> +{
>> +       s32 status;
>> +       int ret;
>> +
>> +       ret = optee_rpmi_call_with_status(optee, req, req_len, resp, resp_size,
>> +                                         &status);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return rpmi_to_linux_error(status);
>> +}
>> +
>> +static int optee_rpmi_shm_rht_init(struct optee *optee)
>> +{
>> +       int ret;
>> +
>> +       mutex_init(&optee->rpmi.shm_rht_lock);
>> +       ret = rhashtable_init(&optee->rpmi.shm_rht, &optee_rpmi_shm_rht_params);
>> +       if (ret)
>> +               mutex_destroy(&optee->rpmi.shm_rht_lock);
>> +
>> +       return ret;
>> +}
>> +
>> +static void optee_rpmi_shm_rht_free(void *ptr, void *arg)
>> +{
>> +       kfree(ptr);
>> +}
>> +
>> +static void optee_rpmi_shm_rht_uninit(struct optee *optee)
>> +{
>> +       rhashtable_free_and_destroy(&optee->rpmi.shm_rht,
>> +                                   optee_rpmi_shm_rht_free, NULL);
>> +       mutex_destroy(&optee->rpmi.shm_rht_lock);
>> +}
>> +
>> +/* Allocate and publish a parcel-to-SHM mapping. */
>> +static int optee_rpmi_shm_rht_add(struct optee *optee, struct tee_shm *shm,
>> +                                 u32 parcel_id, u32 nonce)
>> +{
>> +       struct optee_rpmi_shm_rht_entry *entry;
>> +       int ret;
>> +
>> +       entry = kzalloc_obj(*entry);
>> +       if (!entry)
>> +               return -ENOMEM;
>> +
>> +       entry->shm = shm;
>> +       entry->key.parcel_id = parcel_id;
>> +       entry->key.nonce = nonce;
>> +
>> +       scoped_guard(mutex, &optee->rpmi.shm_rht_lock)
>> +               ret = rhashtable_lookup_insert_fast(&optee->rpmi.shm_rht,
>> +                                                   &entry->node,
>> +                                                   optee_rpmi_shm_rht_params);
>> +       if (ret)
>> +               kfree(entry);
>> +
>> +       return ret;
>> +}
>> +
>> +/* Remove and free a parcel-to-SHM mapping. */
>> +static int optee_rpmi_shm_rht_rm(struct optee *optee, u32 parcel_id, u32 nonce)
>> +{
>> +       struct optee_rpmi_shm_rht_entry *entry;
>> +       struct optee_rpmi_parcel_key key = {
>> +               .parcel_id = parcel_id,
>> +               .nonce = nonce,
>> +       };
>> +       int ret = -ENOENT;
>> +
>> +       scoped_guard(mutex, &optee->rpmi.shm_rht_lock) {
>> +               entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
>> +                                              optee_rpmi_shm_rht_params);
>> +               if (entry)
>> +                       ret = rhashtable_remove_fast(&optee->rpmi.shm_rht,
>> +                                                    &entry->node,
>> +                                                    optee_rpmi_shm_rht_params);
>> +       }
>> +
>> +       if (!ret)
>> +               kfree(entry);
>> +
>> +       return ret;
>> +}
>> +
>> +/* Return a raw pointer; the surrounding call or RPC owns the SHM lifetime. */
>> +static struct tee_shm *
>> +optee_rpmi_get_shm_for_parcel(struct optee *optee, u32 parcel_id, u32 nonce)
>> +{
>> +       struct optee_rpmi_shm_rht_entry *entry;
>> +       struct optee_rpmi_parcel_key key = {
>> +               .parcel_id = parcel_id,
>> +               .nonce = nonce,
>> +       };
>> +
>> +       guard(mutex)(&optee->rpmi.shm_rht_lock);
>> +       entry = rhashtable_lookup_fast(&optee->rpmi.shm_rht, &key,
>> +                                      optee_rpmi_shm_rht_params);
>> +
>> +       return entry ? entry->shm : NULL;
> 
> Please use a full if statement instead of the ternary operator
> 

Ack.

>> +}
>> +
>> +/* Extract the parcel ID and nonce stored in the SHM identity. */
>> +static void optee_rpmi_shm_get_identity(const struct tee_shm *shm,
>> +                                       u32 *parcel_id, u32 *nonce)
>> +{
>> +       *parcel_id = lower_32_bits(shm->sec_world_id);
>> +       *nonce = upper_32_bits(shm->sec_world_id);
> 
> By keeping parcel_id and nonce in separate fields, we add quite a bit
> of code only to handle u64 -> u32 + u32 and vice versa. I wonder if it
> wouldn't be easier always to keep them in a u64 and say that the upper
> 32 bits are a nonce, or something. With that, we could make the
> optee_shm_rem_ffa_handle() function and friends common helpers in the
> optee driver.

Yes. I'll do that.
I'll also try to extract the sharable functions from FFA in the next version.

> 
>> +}
>> +
>> +static int optee_rpmi_shm_register(struct tee_context *ctx, struct tee_shm *shm,
>> +                                  struct page **pages, size_t num_pages,
>> +                                  unsigned long start)
>> +{
>> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
>> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
>> +       struct rpmi_tee_mem_receiver receiver = {
>> +               .endpoint_id = rdev->endpoint_id,
>> +               .access = RPMI_TEE_MEM_ACCESS_READ | RPMI_TEE_MEM_ACCESS_WRITE,
>> +       };
>> +       struct rpmi_tee_mem_args args = {
>> +               .nonce = OPTEE_RPMI_SHM_NONCE,
>> +               .receivers = &receiver,
>> +               .receiver_count = 1,
>> +               .creator_access = RPMI_TEE_MEM_ACCESS_READ |
>> +                                 RPMI_TEE_MEM_ACCESS_WRITE,
>> +       };
>> +       struct sg_table sgt;
>> +       int ret;
>> +
>> +       ret = optee_check_mem_type(start, num_pages);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0,
>> +                                       num_pages * PAGE_SIZE, GFP_KERNEL);
>> +       if (ret)
>> +               return ret;
>> +
>> +       args.sg = sgt.sgl;
>> +       ret = rdev->ops->mem_ops->memory_share(rdev, &args);
>> +       sg_free_table(&sgt);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = optee_rpmi_shm_rht_add(optee, shm, args.parcel_id, args.nonce);
>> +       if (ret) {
>> +               int reclaim_ret;
>> +
>> +               reclaim_ret = rdev->ops->mem_ops->memory_reclaim(rdev, args.parcel_id);
>> +               if (reclaim_ret)
>> +                       dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
>> +                               args.parcel_id, reclaim_ret);
>> +               return ret;
>> +       }
>> +
>> +       shm->sec_world_id = ((u64)args.nonce << 32) | args.parcel_id;
>> +
>> +       return 0;
>> +}
>> +
>> +static int optee_rpmi_shm_unregister(struct tee_context *ctx,
>> +                                    struct tee_shm *shm)
>> +{
>> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
>> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
>> +       struct optee_rpmi_unregister_req req;
>> +       struct optee_rpmi_status_resp resp;
>> +       u32 parcel_id, nonce;
>> +       int ret;
>> +
>> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
>> +       optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
>> +       shm->sec_world_id = 0;
>> +
>> +       req.op = cpu_to_le32(OPTEE_RPMI_UNREGISTER_SHM);
>> +       req.parcel_id = cpu_to_le32(parcel_id);
>> +       req.nonce = cpu_to_le32(nonce);
>> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &resp, sizeof(resp));
>> +       if (ret)
>> +               dev_err(&rdev->dev, "unregister parcel %#x failed: %d\n",
>> +                       parcel_id, ret);
>> +
>> +       ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
>> +       if (ret)
>> +               dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
>> +                       parcel_id, ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int optee_rpmi_shm_unregister_supp(struct tee_context *ctx,
>> +                                         struct tee_shm *shm)
>> +{
>> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
>> +       struct rpmi_tee_device *rdev = optee->rpmi.rdev;
>> +       u32 parcel_id, nonce;
>> +       int ret;
>> +
>> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
>> +       optee_rpmi_shm_rht_rm(optee, parcel_id, nonce);
>> +       shm->sec_world_id = 0;
>> +       /* OP-TEE has already retired the parcel through SHM_FREE RPC. */
>> +       ret = rdev->ops->mem_ops->memory_reclaim(rdev, parcel_id);
>> +       if (ret)
>> +               dev_err(&rdev->dev, "reclaim parcel %#x failed: %d\n",
>> +                       parcel_id, ret);
>> +
>> +       return ret;
>> +}
>> +
>> +/* Convert a memory reference to an OP-TEE RPMI parcel reference. */
>> +static int to_msg_param_rpmi_mem(struct optee_msg_param *mp,
>> +                                const struct tee_param *p)
>> +{
>> +       struct tee_shm *shm = p->u.memref.shm;
>> +
>> +       mp->attr = OPTEE_MSG_ATTR_TYPE_PMEM_INPUT + p->attr -
>> +                  TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
>> +       memset(&mp->u, 0, sizeof(mp->u));
>> +       /* For !shm, return parcel_id = 0 and nonce = 0 to represent NULL. */
>> +       if (shm) {
>> +               if (check_add_overflow((u64)shm->offset,
>> +                                      (u64)p->u.memref.shm_offs,
>> +                                      &mp->u.pmem.offs))
>> +                       return -EINVAL;
> 
> By combining the shm->offset with p->u.memref.shm_offs, OP-TEE
> requires an explicit call to register the shared memory so it knows
> the initial page offset. Compared with the FF-A ABI, which can tell
> the initial page offset from mp->u.fmem.internal_offs. See also the
> mobj_ffa_get_by_cookie() call in set_fmem_param() in
> core/tee/entry_std.c in optee_os.git
> 

I addressed this in my reply to the earlier commit. The intended
secure-side memory object covers the entire parcel, so the supplied
offset is parcel-relative. OP-TEE can retrieve the parcel lazily and
apply that offset directly, without a separate SHM registration call.

Is there a requirement for OP-TEE to know the initial page offset
separately that I am overlooking? I understand it helps with a
logical buffer representation and to match with Linux SHM.
The whole-parcel memory-object model appears to work without it.

But if you feel, having seperate offsets is better or may help
for some unification with FFA in future. I can do that :).

>> +
>> +               optee_rpmi_shm_get_identity(shm, &mp->u.pmem.parcel_id,
>> +                                           &mp->u.pmem.nonce);
>> +       }
>> +
>> +       mp->u.pmem.size = p->u.memref.size;
>> +
>> +       return 0;
>> +}
>> +
>> +static int optee_rpmi_to_msg_param(struct optee *optee,
>> +                                  struct optee_msg_param *msg_params,
>> +                                  size_t num_params,
>> +                                  const struct tee_param *params)
>> +{
>> +       size_t n;
>> +
>> +       for (n = 0; n < num_params; n++) {
>> +               const struct tee_param *p = params + n;
>> +               struct optee_msg_param *mp = msg_params + n;
> 
> Please add an empty line after the variables.
> 

Ack.

Thanks Jens for the review.

Best Regards,
Amir

> Cheers,
> Jens
> 
>> +               switch (p->attr) {
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_NONE:
>> +                       mp->attr = OPTEE_MSG_ATTR_TYPE_NONE;
>> +                       memset(&mp->u, 0, sizeof(mp->u));
>> +                       break;
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT:
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT:
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT:
>> +                       optee_to_msg_param_value(mp, p);
>> +                       break;
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT:
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
>> +               case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
>> +                       if (to_msg_param_rpmi_mem(mp, p))
>> +                               return -EINVAL;
>> +                       break;
>> +               default:
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +/* Convert an RPMI parcel reference to a memref; callers own SHM lifetime. */
>> +static int from_msg_param_rpmi_mem(struct optee *optee, struct tee_param *p,
>> +                                  u32 attr, const struct optee_msg_param *mp)
>> +{
>> +       struct tee_shm *shm;
>> +       u64 offset;
>> +
>> +       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT + attr -
>> +                 OPTEE_MSG_ATTR_TYPE_PMEM_INPUT;
>> +
>> +       if (mp->u.pmem.size > SIZE_MAX)
>> +               return -EOVERFLOW;
>> +       p->u.memref.size = mp->u.pmem.size;
>> +
>> +       if (!mp->u.pmem.nonce) {
>> +               /* Return NULL shm. */
>> +               if (mp->u.pmem.offs || mp->u.pmem.parcel_id)
>> +                       return -EINVAL;
>> +               p->u.memref.shm = NULL;
>> +               p->u.memref.shm_offs = 0;
>> +               return 0;
>> +       }
>> +
>> +       shm = optee_rpmi_get_shm_for_parcel(optee, mp->u.pmem.parcel_id,
>> +                                           mp->u.pmem.nonce);
>> +       if (!shm || mp->u.pmem.offs < shm->offset)
>> +               return -EINVAL;
>> +
>> +       offset = mp->u.pmem.offs - shm->offset;
>> +       if (offset > SIZE_MAX)
>> +               return -EOVERFLOW;
>> +
>> +       p->u.memref.shm = shm;
>> +       p->u.memref.shm_offs = offset;
>> +
>> +       return 0;
>> +}
>> +
>> +static int optee_rpmi_from_msg_param(struct optee *optee,
>> +                                    struct tee_param *params,
>> +                                    size_t num_params,
>> +                                    const struct optee_msg_param *msg_params)
>> +{
>> +       size_t n;
>> +
>> +       for (n = 0; n < num_params; n++) {
>> +               const struct optee_msg_param *mp = msg_params + n;
>> +               struct tee_param *p = params + n;
>> +               u32 attr = mp->attr & OPTEE_MSG_ATTR_TYPE_MASK;
>> +               int ret;
>> +
>> +               switch (attr) {
>> +               case OPTEE_MSG_ATTR_TYPE_NONE:
>> +                       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_NONE;
>> +                       memset(&p->u, 0, sizeof(p->u));
>> +                       break;
>> +               case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
>> +               case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
>> +               case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
>> +                       optee_from_msg_param_value(p, attr, mp);
>> +                       break;
>> +               case OPTEE_MSG_ATTR_TYPE_PMEM_INPUT:
>> +               case OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT:
>> +               case OPTEE_MSG_ATTR_TYPE_PMEM_INOUT:
>> +                       ret = from_msg_param_rpmi_mem(optee, p, attr, mp);
>> +                       if (ret)
>> +                               return ret;
>> +                       break;
>> +               default:
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +       return 0;
>> +}
>>
>> --
>> 2.34.1
>>

[PATCH RFC v2 4/8] tee: optee: add RPMI dynamic shared-memory pool

[Amirreza Zarrabi]

Add a page-based dynamic shared-memory pool for the RPMI backend using
the common TEE allocation and free helpers.

Register allocated pages as RPMI parcels so OP-TEE can access them.
Unregister the parcels before freeing their backing pages.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/rpmi_abi.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
index e7fc853cfb15..c29b7ac4552a 100644
--- a/drivers/tee/optee/rpmi_abi.c
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -282,6 +282,42 @@ static int optee_rpmi_shm_unregister_supp(struct tee_context *ctx,
 	return ret;
 }
 
+static int optee_rpmi_pool_alloc(struct tee_shm_pool *pool, struct tee_shm *shm,
+				 size_t size, size_t align)
+{
+	return tee_dyn_shm_alloc_helper(shm, size, align,
+					optee_rpmi_shm_register);
+}
+
+static void optee_rpmi_pool_free(struct tee_shm_pool *pool, struct tee_shm *shm)
+{
+	tee_dyn_shm_free_helper(shm, optee_rpmi_shm_unregister);
+}
+
+static void optee_rpmi_pool_destroy(struct tee_shm_pool *pool)
+{
+	kfree(pool);
+}
+
+static const struct tee_shm_pool_ops optee_rpmi_pool_ops = {
+	.alloc = optee_rpmi_pool_alloc,
+	.free = optee_rpmi_pool_free,
+	.destroy_pool = optee_rpmi_pool_destroy,
+};
+
+static struct tee_shm_pool *optee_rpmi_shm_pool_alloc(void)
+{
+	struct tee_shm_pool *pool;
+
+	pool = kzalloc_obj(*pool);
+	if (!pool)
+		return ERR_PTR(-ENOMEM);
+
+	pool->ops = &optee_rpmi_pool_ops;
+
+	return pool;
+}
+
 /* Convert a memory reference to an OP-TEE RPMI parcel reference. */
 static int to_msg_param_rpmi_mem(struct optee_msg_param *mp,
 				 const struct tee_param *p)

-- 
2.34.1

Re: [PATCH RFC v2 4/8] tee: optee: add RPMI dynamic shared-memory pool

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] tee: optee: UAF in optee_rpmi_shm_rht_rm() due to synchronous kfree()
- [Low] build: unused static function optee_rpmi_shm_pool_alloc breaks bisect
--

commit c8c597113db02649177403939240e1d7feefbedd
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

tee: optee: add RPMI dynamic shared-memory pool

This patch adds a page-based dynamic shared-memory pool for the RPMI backend
using the common TEE allocation and free helpers. It registers allocated
pages as RPMI parcels so OP-TEE can access them, and unregisters the parcels
before freeing their backing pages.

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index e7fc853cfb150..c29b7ac4552a9 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c

[ ... ]

> +static void optee_rpmi_pool_free(struct tee_shm_pool *pool, struct tee_shm *shm)
> +{
> +	tee_dyn_shm_free_helper(shm, optee_rpmi_shm_unregister);
> +}

[Severity: High]
Is there a risk of a use-after-free during bucket traversal here?

When tee_dyn_shm_free_helper() executes optee_rpmi_shm_unregister() in
drivers/tee/optee/rpmi_abi.c, it subsequently calls optee_rpmi_shm_rht_rm()
to remove the hash table entry. This same path is also triggered via
optee_rpmi_shm_unregister_supp().

In optee_rpmi_shm_rht_rm():

	if (entry)
		ret = rhashtable_remove_fast(&optee->rpmi.shm_rht,
					     &entry->node,
					     optee_rpmi_shm_rht_params);
	if (!ret)
		kfree(entry);

Because rhashtable uses RCU-based worker threads (rht_deferred_worker) to
traverse the hash table concurrently during automatic resizing, freeing
the entry synchronously via kfree() instead of kfree_rcu() might cause a
use-after-free if a rehash worker is actively traversing the removed node's
bucket list.

Note that this issue is reachable from unprivileged user space by repeatedly
registering and unregistering memory, driving the size of the table up and
down to trigger rehashing.

[ ... ]

> +static struct tee_shm_pool *optee_rpmi_shm_pool_alloc(void)
> +{
> +	struct tee_shm_pool *pool;
> +
> +	pool = kzalloc_obj(*pool);
> +	if (!pool)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pool->ops = &optee_rpmi_pool_ops;
> +
> +	return pool;
> +}

[Severity: Low]
Could this unused function break git bisect?

The static function optee_rpmi_shm_pool_alloc() in
drivers/tee/optee/rpmi_abi.c is defined here but is never invoked in this
patch. Under CONFIG_WERROR=y, the compiler warning for an unused function
promotes to a build error.

While this is fixed later in the series by commit
3ef22394684ff6d8b2126c91fa4c8bb2d1960d68 (tee: optee: bind RPMI services and
negotiate backend capabilities), each individual patch in a series
should compile cleanly to ensure git bisect continues to work.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=4

[PATCH RFC v2 5/8] tee: optee: add RPMI RPC handling

[Amirreza Zarrabi]

OP-TEE can suspend a yielding call to request services from normal
world through RPC.

Add handlers for shared-memory allocation and free requests. Use the
existing kernel and supplicant allocation helpers, return RPMI parcel
references for allocated memory, and resolve those references when
freeing it.

Route other commands through the common OP-TEE RPC dispatcher. Store
RPC results in the shared arguments for OP-TEE to consume when the
yielding call resumes.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/rpmi_abi.c | 109 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
index c29b7ac4552a..58d82678be98 100644
--- a/drivers/tee/optee/rpmi_abi.c
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -13,6 +13,7 @@
 #include <linux/unaligned.h>
 #include "optee_private.h"
 #include "optee_rpmi.h"
+#include "optee_rpc_cmd.h"
 
 /* Nonzero nonce keeps parcel ID zero distinct from a null reference. */
 #define OPTEE_RPMI_SHM_NONCE	1
@@ -451,3 +452,111 @@ static int optee_rpmi_from_msg_param(struct optee *optee,
 	}
 	return 0;
 }
+
+static void optee_rpmi_handle_rpc_shm_alloc(struct tee_context *ctx,
+					    struct optee *optee,
+					    struct optee_msg_arg *arg)
+{
+	u32 parcel_id, nonce;
+	struct tee_shm *shm;
+	u64 type, size;
+
+	if (arg->num_params != 1 ||
+	    arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
+		goto err_bad_param;
+
+	type = arg->params[0].u.value.a;
+	size = arg->params[0].u.value.b;
+	if (!size || size > SIZE_MAX - PAGE_SIZE + 1)
+		goto err_bad_param;
+
+	switch (type) {
+	case OPTEE_RPC_SHM_TYPE_APPL:
+		shm = optee_rpc_cmd_alloc_suppl(ctx, size);
+		break;
+	case OPTEE_RPC_SHM_TYPE_KERNEL:
+		shm = tee_shm_alloc_priv_buf(optee->ctx, size);
+		break;
+	default:
+		goto err_bad_param;
+	}
+
+	if (IS_ERR(shm)) {
+		arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
+		return;
+	}
+
+	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
+	arg->params[0] = (struct optee_msg_param) {
+		.attr = OPTEE_MSG_ATTR_TYPE_PMEM_OUTPUT,
+		.u.pmem = {
+			.offs = shm->offset,
+			.size = tee_shm_get_size(shm),
+			.parcel_id = parcel_id,
+			.nonce = nonce,
+		},
+	};
+
+	arg->ret = TEEC_SUCCESS;
+	return;
+
+err_bad_param:
+	arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+}
+
+static void optee_rpmi_handle_rpc_shm_free(struct tee_context *ctx,
+					   struct optee *optee,
+					   struct optee_msg_arg *arg)
+{
+	struct tee_shm *shm;
+	u64 type, parcel_id, nonce;
+
+	if (arg->num_params != 1 ||
+	    arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
+		goto err_bad_param;
+
+	type = arg->params[0].u.value.a;
+	parcel_id = arg->params[0].u.value.b;
+	nonce = arg->params[0].u.value.c;
+	if (parcel_id > U32_MAX || nonce > U32_MAX)
+		goto err_bad_param;
+
+	shm = optee_rpmi_get_shm_for_parcel(optee, parcel_id, nonce);
+	if (!shm)
+		goto err_bad_param;
+
+	switch (type) {
+	case OPTEE_RPC_SHM_TYPE_APPL:
+		optee_rpc_cmd_free_suppl(ctx, shm);
+		break;
+	case OPTEE_RPC_SHM_TYPE_KERNEL:
+		tee_shm_free(shm);
+		break;
+	default:
+		goto err_bad_param;
+	}
+
+	arg->ret = TEEC_SUCCESS;
+	return;
+
+err_bad_param:
+	arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+}
+
+/* OP-TEE leaves the shared arguments unchanged while Linux handles the RPC. */
+static void optee_rpmi_handle_rpc_cmd(struct tee_context *ctx,
+				      struct optee *optee,
+				      struct optee_msg_arg *arg)
+{
+	arg->ret_origin = TEEC_ORIGIN_COMMS;
+	switch (arg->cmd) {
+	case OPTEE_RPC_CMD_SHM_ALLOC:
+		optee_rpmi_handle_rpc_shm_alloc(ctx, optee, arg);
+		break;
+	case OPTEE_RPC_CMD_SHM_FREE:
+		optee_rpmi_handle_rpc_shm_free(ctx, optee, arg);
+		break;
+	default:
+		optee_rpc_cmd(ctx, optee, arg);
+	}
+}

-- 
2.34.1

[PATCH RFC v2 6/8] tee: optee: execute yielding RPMI calls

[Amirreza Zarrabi]

OP-TEE commands can span multiple exchanges with normal world. A call
may yield to request an RPC service or allow interrupt processing
before continuing execution in secure world.

Add the RPMI yielding-call path used by the common OP-TEE session
operations. Submit the command and RPC argument buffers as ranges
within a shared memory parcel.

Handle RPC requests while the call is suspended and resume execution
using the token returned by OP-TEE until the command completes.

Use the common OP-TEE call queue to wait when an initial request is
rejected with RPMI_ERR_BUSY, allowing another active call to complete
before retrying.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/rpmi_abi.c | 124 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
index 58d82678be98..6e76316794c1 100644
--- a/drivers/tee/optee/rpmi_abi.c
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -9,6 +9,7 @@
 #include <linux/mailbox/riscv-rpmi-message.h>
 #include <linux/overflow.h>
 #include <linux/rpmi_tee.h>
+#include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/unaligned.h>
 #include "optee_private.h"
@@ -560,3 +561,126 @@ static void optee_rpmi_handle_rpc_cmd(struct tee_context *ctx,
 		optee_rpc_cmd(ctx, optee, arg);
 	}
 }
+
+/* Handle RPC command or interrupt returns from a yielding call. */
+static void optee_rpmi_handle_rpc(struct tee_context *ctx, struct optee *optee,
+				  u32 result, struct optee_msg_arg *arg)
+{
+	switch (result) {
+	case OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD:
+		optee_rpmi_handle_rpc_cmd(ctx, optee, arg);
+		break;
+	case OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT:
+		break;
+	default:
+		pr_warn("Unknown RPC func 0x%x\n", result);
+		break;
+	}
+}
+
+/**
+ * optee_rpmi_yielding_call() - submit and resume a yielding RPMI command
+ * @ctx: calling context
+ * @req: initial command request
+ * @rpc_arg: shared RPC argument buffer
+ * @system_thread: caller requests TEE system thread support
+ *
+ * Only RPMI_ERR_BUSY rejection of the initial command permits retry.
+ *
+ * Return: zero on completion, or a negative error.
+ */
+static int optee_rpmi_yielding_call(struct tee_context *ctx,
+				    const struct optee_rpmi_call_req *req,
+				    struct optee_msg_arg *rpc_arg,
+				    bool system_thread)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+	struct optee_rpmi_resume_req resume = {
+		.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_RESUME),
+		/* resume_token is nonzero after OP-TEE suspends the call. */
+		.resume_token = 0,
+	};
+	struct optee_rpmi_call_resp resp;
+	struct optee_call_waiter waiter;
+	u32 result;
+	s32 status;
+	int ret;
+
+	optee_cq_wait_init(&optee->call_queue, &waiter, system_thread);
+	while (true) {
+		if (resume.resume_token)
+			ret = optee_rpmi_call_with_status(optee, &resume,
+							  sizeof(resume), &resp,
+							  sizeof(resp), &status);
+		else
+			ret = optee_rpmi_call_with_status(optee, req, sizeof(*req),
+							  &resp, sizeof(resp),
+							  &status);
+		if (ret)
+			goto done;
+
+		switch (status) {
+		case RPMI_SUCCESS:
+			break;
+		case RPMI_ERR_BUSY:
+			if (!resume.resume_token) {
+				optee_cq_wait_for_completion(&optee->call_queue,
+							     &waiter);
+				continue;
+			}
+
+			fallthrough;
+		default:
+			ret = rpmi_to_linux_error(status);
+			goto done;
+		}
+
+		result = get_unaligned_le32(&resp.result);
+		if (result == OPTEE_RPMI_YIELDING_CALL_RETURN_DONE)
+			goto done;
+
+		cond_resched();
+		optee_rpmi_handle_rpc(ctx, optee, result, rpc_arg);
+
+		resume.resume_token = resp.resume_token;
+	}
+done:
+	optee_cq_wait_final(&optee->call_queue, &waiter);
+
+	return ret;
+}
+
+/* The caller supplies SHM with room for command and RPC args. */
+static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
+				       struct tee_shm *shm, u_int offs,
+				       bool system_thread)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+	struct optee_msg_arg *arg, *rpc_arg;
+	struct optee_rpmi_call_req req;
+	size_t arg_size, rpc_size, rpc_offset;
+	u32 parcel_id, nonce;
+
+	arg = tee_shm_get_va(shm, offs);
+	if (IS_ERR(arg))
+		return PTR_ERR(arg);
+
+	arg_size = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
+	rpc_size = OPTEE_MSG_GET_ARG_SIZE(optee->rpc_param_count);
+	rpc_offset = offs + arg_size;
+	rpc_arg = tee_shm_get_va(shm, rpc_offset);
+	if (IS_ERR(rpc_arg))
+		return PTR_ERR(rpc_arg);
+
+	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
+
+	req.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_WITH_ARG);
+	req.parcel_id = cpu_to_le32(parcel_id);
+	req.nonce = cpu_to_le32(nonce);
+	req.arg_offset = cpu_to_le64((u64)shm->offset + offs);
+	req.rpc_offset = cpu_to_le64((u64)shm->offset + rpc_offset);
+	req.arg_size = cpu_to_le32(arg_size);
+	req.rpc_size = cpu_to_le32(rpc_size);
+
+	return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
+}

-- 
2.34.1

Re: [PATCH RFC v2 6/8] tee: optee: execute yielding RPMI calls

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] optee: unhandled unknown RPC result causes infinite loop in yielding call
- [High] optee: deadlock on RPMI_ERR_BUSY when no local threads are active
- [High] optee: uninitialized req.flags leaks stack memory to Secure World
- [Medium] optee: zero resume_token from suspended call causes infinite retry loop
--

commit e5243042ee3a40ae18df933976be9641326c5d08
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Date:   Tue Oct 6 00:39:50 2026 +0000

    tee: optee: execute yielding RPMI calls
    
    OP-TEE commands can span multiple exchanges with normal world. A call
    may yield to request an RPC service or allow interrupt processing
    before continuing execution in secure world.
    
    Add the RPMI yielding-call path used by the common OP-TEE session
    operations. Submit the command and RPC argument buffers as ranges
    within a shared memory parcel.
    
    Handle RPC requests while the call is suspended and resume execution
    using the token returned by OP-TEE until the command completes.
    
    Use the common OP-TEE call queue to wait when an initial request is
    rejected with RPMI_ERR_BUSY, allowing another active call to complete
    before retrying.
    
    Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index 58d82678be98f..6e76316794c18 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c

[ ... ]

> +/* Handle RPC command or interrupt returns from a yielding call. */
> +static void optee_rpmi_handle_rpc(struct tee_context *ctx, struct optee *optee,
> +				  u32 result, struct optee_msg_arg *arg)
> +{
> +	switch (result) {
> +	case OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD:
> +		optee_rpmi_handle_rpc_cmd(ctx, optee, arg);
> +		break;
> +	case OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT:
> +		break;
> +	default:
> +		pr_warn("Unknown RPC func 0x%x\n", result);
> +		break;
> +	}
> +}

[Severity: High]
Does this code risk entering an infinite loop? If the result is unknown,
optee_rpmi_handle_rpc() logs a warning but doesn't return an error code to
the caller.

The loop in optee_rpmi_yielding_call() below will then blindly update the
token and continue infinitely polling the TEE.

> +
> +/**
> + * optee_rpmi_yielding_call() - submit and resume a yielding RPMI command
> + * @ctx: calling context
> + * @req: initial command request
> + * @rpc_arg: shared RPC argument buffer
> + * @system_thread: caller requests TEE system thread support
> + *
> + * Only RPMI_ERR_BUSY rejection of the initial command permits retry.
> + *
> + * Return: zero on completion, or a negative error.
> + */
> +static int optee_rpmi_yielding_call(struct tee_context *ctx,
> +				    const struct optee_rpmi_call_req *req,
> +				    struct optee_msg_arg *rpc_arg,
> +				    bool system_thread)
> +{
> +	struct optee *optee = tee_get_drvdata(ctx->teedev);
> +	struct optee_rpmi_resume_req resume = {
> +		.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_RESUME),
> +		/* resume_token is nonzero after OP-TEE suspends the call. */
> +		.resume_token = 0,
> +	};
> +	struct optee_rpmi_call_resp resp;
> +	struct optee_call_waiter waiter;
> +	u32 result;
> +	s32 status;
> +	int ret;
> +
> +	optee_cq_wait_init(&optee->call_queue, &waiter, system_thread);
> +	while (true) {
> +		if (resume.resume_token)
> +			ret = optee_rpmi_call_with_status(optee, &resume,
> +							  sizeof(resume), &resp,
> +							  sizeof(resp), &status);
> +		else
> +			ret = optee_rpmi_call_with_status(optee, req, sizeof(*req),
> +							  &resp, sizeof(resp),
> +							  &status);

[Severity: Medium]
Will this correctly handle a zero resume_token from a suspended call?

If the firmware suspends the call but incorrectly provides a resume_token
of 0, resume.resume_token will be 0 on the next iteration. This if
statement will evaluate to false, falling back to resending the initial
request instead of the resume command, which could cause an infinite retry
loop.

> +		if (ret)
> +			goto done;
> +
> +		switch (status) {
> +		case RPMI_SUCCESS:
> +			break;
> +		case RPMI_ERR_BUSY:
> +			if (!resume.resume_token) {
> +				optee_cq_wait_for_completion(&optee->call_queue,
> +							     &waiter);
> +				continue;
> +			}

[Severity: High]
Can this cause a deadlock if the BUSY state was triggered by an external
client?

If RPMI_ERR_BUSY is returned on the initial command, the task blocks
indefinitely in optee_cq_wait_for_completion() waiting for a local
completion. If the shared mailbox or service is busy processing requests
from another VM and the local kernel has no other active threads, no thread
will ever call optee_cq_wait_final() to wake up this task.

> +
> +			fallthrough;

[ ... ]

> +/* The caller supplies SHM with room for command and RPC args. */
> +static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
> +				       struct tee_shm *shm, u_int offs,
> +				       bool system_thread)
> +{
> +	struct optee *optee = tee_get_drvdata(ctx->teedev);
> +	struct optee_msg_arg *arg, *rpc_arg;
> +	struct optee_rpmi_call_req req;

[Severity: High]
Does this leak uninitialized kernel stack memory to the Secure World?

The struct optee_rpmi_call_req req is allocated on the stack here but is
not zero-initialized.

> +	size_t arg_size, rpc_size, rpc_offset;
> +	u32 parcel_id, nonce;
> +
> +	arg = tee_shm_get_va(shm, offs);
> +	if (IS_ERR(arg))
> +		return PTR_ERR(arg);
> +
> +	arg_size = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
> +	rpc_size = OPTEE_MSG_GET_ARG_SIZE(optee->rpc_param_count);
> +	rpc_offset = offs + arg_size;
> +	rpc_arg = tee_shm_get_va(shm, rpc_offset);
> +	if (IS_ERR(rpc_arg))
> +		return PTR_ERR(rpc_arg);
> +
> +	optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
> +
> +	req.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_WITH_ARG);
> +	req.parcel_id = cpu_to_le32(parcel_id);
> +	req.nonce = cpu_to_le32(nonce);
> +	req.arg_offset = cpu_to_le64((u64)shm->offset + offs);
> +	req.rpc_offset = cpu_to_le64((u64)shm->offset + rpc_offset);
> +	req.arg_size = cpu_to_le32(arg_size);
> +	req.rpc_size = cpu_to_le32(rpc_size);

[Severity: High]
The code explicitly initializes most fields but misses req.flags. This
leaves req.flags containing uninitialized stack memory when req is passed
to optee_rpmi_yielding_call() and sent over the RPMI mailbox.

Could we initialize the whole struct to zero?

> +
> +	return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=6

Re: [PATCH RFC v2 6/8] tee: optee: execute yielding RPMI calls

[Jens Wiklander]

On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> OP-TEE commands can span multiple exchanges with normal world. A call
> may yield to request an RPC service or allow interrupt processing
> before continuing execution in secure world.
>
> Add the RPMI yielding-call path used by the common OP-TEE session
> operations. Submit the command and RPC argument buffers as ranges
> within a shared memory parcel.
>
> Handle RPC requests while the call is suspended and resume execution
> using the token returned by OP-TEE until the command completes.
>
> Use the common OP-TEE call queue to wait when an initial request is
> rejected with RPMI_ERR_BUSY, allowing another active call to complete
> before retrying.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
>  drivers/tee/optee/rpmi_abi.c | 124 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
>
> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index 58d82678be98..6e76316794c1 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c
> @@ -9,6 +9,7 @@
>  #include <linux/mailbox/riscv-rpmi-message.h>
>  #include <linux/overflow.h>
>  #include <linux/rpmi_tee.h>
> +#include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/unaligned.h>
>  #include "optee_private.h"
> @@ -560,3 +561,126 @@ static void optee_rpmi_handle_rpc_cmd(struct tee_context *ctx,
>                 optee_rpc_cmd(ctx, optee, arg);
>         }
>  }
> +
> +/* Handle RPC command or interrupt returns from a yielding call. */
> +static void optee_rpmi_handle_rpc(struct tee_context *ctx, struct optee *optee,
> +                                 u32 result, struct optee_msg_arg *arg)
> +{
> +       switch (result) {
> +       case OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD:
> +               optee_rpmi_handle_rpc_cmd(ctx, optee, arg);
> +               break;
> +       case OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT:
> +               break;
> +       default:
> +               pr_warn("Unknown RPC func 0x%x\n", result);
> +               break;
> +       }
> +}
> +
> +/**
> + * optee_rpmi_yielding_call() - submit and resume a yielding RPMI command
> + * @ctx: calling context
> + * @req: initial command request
> + * @rpc_arg: shared RPC argument buffer
> + * @system_thread: caller requests TEE system thread support
> + *
> + * Only RPMI_ERR_BUSY rejection of the initial command permits retry.
> + *
> + * Return: zero on completion, or a negative error.
> + */
> +static int optee_rpmi_yielding_call(struct tee_context *ctx,
> +                                   const struct optee_rpmi_call_req *req,
> +                                   struct optee_msg_arg *rpc_arg,
> +                                   bool system_thread)
> +{
> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> +       struct optee_rpmi_resume_req resume = {
> +               .op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_RESUME),
> +               /* resume_token is nonzero after OP-TEE suspends the call. */
> +               .resume_token = 0,

I missed this when reviewing "tee: optee: define the RPMI control and
parcel-reference ABI". I'd prefer if the resume_token was a truly
opaque value, just as for the SMC and FF-A ABIs. Any reason why it's a
64-bit value instead of 32-bit, as is used for the other ABI?

> +       };
> +       struct optee_rpmi_call_resp resp;
> +       struct optee_call_waiter waiter;
> +       u32 result;
> +       s32 status;
> +       int ret;
> +
> +       optee_cq_wait_init(&optee->call_queue, &waiter, system_thread);
> +       while (true) {
> +               if (resume.resume_token)
> +                       ret = optee_rpmi_call_with_status(optee, &resume,
> +                                                         sizeof(resume), &resp,
> +                                                         sizeof(resp), &status);
> +               else
> +                       ret = optee_rpmi_call_with_status(optee, req, sizeof(*req),
> +                                                         &resp, sizeof(resp),
> +                                                         &status);

Please fix the too-long lines above.

> +               if (ret)
> +                       goto done;
> +
> +               switch (status) {
> +               case RPMI_SUCCESS:

Any particular reason why we aren't using TEE error codes here?

> +                       break;
> +               case RPMI_ERR_BUSY:
> +                       if (!resume.resume_token) {
> +                               optee_cq_wait_for_completion(&optee->call_queue,
> +                                                            &waiter);
> +                               continue;
> +                       }
> +
> +                       fallthrough;
> +               default:
> +                       ret = rpmi_to_linux_error(status);
> +                       goto done;
> +               }
> +
> +               result = get_unaligned_le32(&resp.result);

Why not le32_to_cpu(resp.result)?

Cheers,
Jens

> +               if (result == OPTEE_RPMI_YIELDING_CALL_RETURN_DONE)
> +                       goto done;
> +
> +               cond_resched();
> +               optee_rpmi_handle_rpc(ctx, optee, result, rpc_arg);
> +
> +               resume.resume_token = resp.resume_token;
> +       }
> +done:
> +       optee_cq_wait_final(&optee->call_queue, &waiter);
> +
> +       return ret;
> +}
> +
> +/* The caller supplies SHM with room for command and RPC args. */
> +static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
> +                                      struct tee_shm *shm, u_int offs,
> +                                      bool system_thread)
> +{
> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> +       struct optee_msg_arg *arg, *rpc_arg;
> +       struct optee_rpmi_call_req req;
> +       size_t arg_size, rpc_size, rpc_offset;
> +       u32 parcel_id, nonce;
> +
> +       arg = tee_shm_get_va(shm, offs);
> +       if (IS_ERR(arg))
> +               return PTR_ERR(arg);
> +
> +       arg_size = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
> +       rpc_size = OPTEE_MSG_GET_ARG_SIZE(optee->rpc_param_count);
> +       rpc_offset = offs + arg_size;
> +       rpc_arg = tee_shm_get_va(shm, rpc_offset);
> +       if (IS_ERR(rpc_arg))
> +               return PTR_ERR(rpc_arg);
> +
> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
> +
> +       req.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_WITH_ARG);
> +       req.parcel_id = cpu_to_le32(parcel_id);
> +       req.nonce = cpu_to_le32(nonce);
> +       req.arg_offset = cpu_to_le64((u64)shm->offset + offs);
> +       req.rpc_offset = cpu_to_le64((u64)shm->offset + rpc_offset);
> +       req.arg_size = cpu_to_le32(arg_size);
> +       req.rpc_size = cpu_to_le32(rpc_size);
> +
> +       return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
> +}
>
> --
> 2.34.1
>

Re: [PATCH RFC v2 6/8] tee: optee: execute yielding RPMI calls

[Amirreza Zarrabi]

Hi Jens,

On 10/8/2026 7:11 PM, Jens Wiklander wrote:
> On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> OP-TEE commands can span multiple exchanges with normal world. A call
>> may yield to request an RPC service or allow interrupt processing
>> before continuing execution in secure world.
>>
>> Add the RPMI yielding-call path used by the common OP-TEE session
>> operations. Submit the command and RPC argument buffers as ranges
>> within a shared memory parcel.
>>
>> Handle RPC requests while the call is suspended and resume execution
>> using the token returned by OP-TEE until the command completes.
>>
>> Use the common OP-TEE call queue to wait when an initial request is
>> rejected with RPMI_ERR_BUSY, allowing another active call to complete
>> before retrying.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>>  drivers/tee/optee/rpmi_abi.c | 124 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 124 insertions(+)
>>
>> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
>> index 58d82678be98..6e76316794c1 100644
>> --- a/drivers/tee/optee/rpmi_abi.c
>> +++ b/drivers/tee/optee/rpmi_abi.c
>> @@ -9,6 +9,7 @@
>>  #include <linux/mailbox/riscv-rpmi-message.h>
>>  #include <linux/overflow.h>
>>  #include <linux/rpmi_tee.h>
>> +#include <linux/sched.h>
>>  #include <linux/slab.h>
>>  #include <linux/unaligned.h>
>>  #include "optee_private.h"
>> @@ -560,3 +561,126 @@ static void optee_rpmi_handle_rpc_cmd(struct tee_context *ctx,
>>                 optee_rpc_cmd(ctx, optee, arg);
>>         }
>>  }
>> +
>> +/* Handle RPC command or interrupt returns from a yielding call. */
>> +static void optee_rpmi_handle_rpc(struct tee_context *ctx, struct optee *optee,
>> +                                 u32 result, struct optee_msg_arg *arg)
>> +{
>> +       switch (result) {
>> +       case OPTEE_RPMI_YIELDING_CALL_RETURN_RPC_CMD:
>> +               optee_rpmi_handle_rpc_cmd(ctx, optee, arg);
>> +               break;
>> +       case OPTEE_RPMI_YIELDING_CALL_RETURN_INTERRUPT:
>> +               break;
>> +       default:
>> +               pr_warn("Unknown RPC func 0x%x\n", result);
>> +               break;
>> +       }
>> +}
>> +
>> +/**
>> + * optee_rpmi_yielding_call() - submit and resume a yielding RPMI command
>> + * @ctx: calling context
>> + * @req: initial command request
>> + * @rpc_arg: shared RPC argument buffer
>> + * @system_thread: caller requests TEE system thread support
>> + *
>> + * Only RPMI_ERR_BUSY rejection of the initial command permits retry.
>> + *
>> + * Return: zero on completion, or a negative error.
>> + */
>> +static int optee_rpmi_yielding_call(struct tee_context *ctx,
>> +                                   const struct optee_rpmi_call_req *req,
>> +                                   struct optee_msg_arg *rpc_arg,
>> +                                   bool system_thread)
>> +{
>> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
>> +       struct optee_rpmi_resume_req resume = {
>> +               .op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_RESUME),
>> +               /* resume_token is nonzero after OP-TEE suspends the call. */
>> +               .resume_token = 0,
> 
> I missed this when reviewing "tee: optee: define the RPMI control and
> parcel-reference ABI". I'd prefer if the resume_token was a truly
> opaque value, just as for the SMC and FF-A ABIs. Any reason why it's a
> 64-bit value instead of 32-bit, as is used for the other ABI?
> 

Agreed. I currently use a zero token to distinguish the initial request
from a resume. I'll track that separately with a boolean and treat the token
as opaque, only copying it from the response into the resume request.

There is no specific reason for it to be 64-bit; I overlooked the
existing ABI convention. I'll change it to 32-bit.

>> +       };
>> +       struct optee_rpmi_call_resp resp;
>> +       struct optee_call_waiter waiter;
>> +       u32 result;
>> +       s32 status;
>> +       int ret;
>> +
>> +       optee_cq_wait_init(&optee->call_queue, &waiter, system_thread);
>> +       while (true) {
>> +               if (resume.resume_token)
>> +                       ret = optee_rpmi_call_with_status(optee, &resume,
>> +                                                         sizeof(resume), &resp,
>> +                                                         sizeof(resp), &status);
>> +               else
>> +                       ret = optee_rpmi_call_with_status(optee, req, sizeof(*req),
>> +                                                         &resp, sizeof(resp),
>> +                                                         &status);
> 
> Please fix the too-long lines above.
> 

Ack.

>> +               if (ret)
>> +                       goto done;
>> +
>> +               switch (status) {
>> +               case RPMI_SUCCESS:
> 
> Any particular reason why we aren't using TEE error codes here?
> 

I defined RPMI error codes consistently for all control responses in optee_rpmi.h.
However, these statuses belong to the OP-TEE service ABI rather than the transport.
I'll change them to TEE error codes and keep RPMI errors at the transport layer.

>> +                       break;
>> +               case RPMI_ERR_BUSY:
>> +                       if (!resume.resume_token) {
>> +                               optee_cq_wait_for_completion(&optee->call_queue,
>> +                                                            &waiter);
>> +                               continue;
>> +                       }
>> +
>> +                       fallthrough;
>> +               default:
>> +                       ret = rpmi_to_linux_error(status);
>> +                       goto done;
>> +               }
>> +
>> +               result = get_unaligned_le32(&resp.result);
> 
> Why not le32_to_cpu(resp.result)?

You are right, there are a couple of more of this that I missed.
I'll fix all in the next version.

Thanks Jens for the review.

Best Regards,
Amir

> 
> Cheers,
> Jens
> 
>> +               if (result == OPTEE_RPMI_YIELDING_CALL_RETURN_DONE)
>> +                       goto done;
>> +
>> +               cond_resched();
>> +               optee_rpmi_handle_rpc(ctx, optee, result, rpc_arg);
>> +
>> +               resume.resume_token = resp.resume_token;
>> +       }
>> +done:
>> +       optee_cq_wait_final(&optee->call_queue, &waiter);
>> +
>> +       return ret;
>> +}
>> +
>> +/* The caller supplies SHM with room for command and RPC args. */
>> +static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
>> +                                      struct tee_shm *shm, u_int offs,
>> +                                      bool system_thread)
>> +{
>> +       struct optee *optee = tee_get_drvdata(ctx->teedev);
>> +       struct optee_msg_arg *arg, *rpc_arg;
>> +       struct optee_rpmi_call_req req;
>> +       size_t arg_size, rpc_size, rpc_offset;
>> +       u32 parcel_id, nonce;
>> +
>> +       arg = tee_shm_get_va(shm, offs);
>> +       if (IS_ERR(arg))
>> +               return PTR_ERR(arg);
>> +
>> +       arg_size = OPTEE_MSG_GET_ARG_SIZE(arg->num_params);
>> +       rpc_size = OPTEE_MSG_GET_ARG_SIZE(optee->rpc_param_count);
>> +       rpc_offset = offs + arg_size;
>> +       rpc_arg = tee_shm_get_va(shm, rpc_offset);
>> +       if (IS_ERR(rpc_arg))
>> +               return PTR_ERR(rpc_arg);
>> +
>> +       optee_rpmi_shm_get_identity(shm, &parcel_id, &nonce);
>> +
>> +       req.op = cpu_to_le32(OPTEE_RPMI_YIELDING_CALL_WITH_ARG);
>> +       req.parcel_id = cpu_to_le32(parcel_id);
>> +       req.nonce = cpu_to_le32(nonce);
>> +       req.arg_offset = cpu_to_le64((u64)shm->offset + offs);
>> +       req.rpc_offset = cpu_to_le64((u64)shm->offset + rpc_offset);
>> +       req.arg_size = cpu_to_le32(arg_size);
>> +       req.rpc_size = cpu_to_le32(rpc_size);
>> +
>> +       return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
>> +}
>>
>> --
>> 2.34.1
>>

[PATCH RFC v2 7/8] tee: optee: bind RPMI services and negotiate backend capabilities

[Amirreza Zarrabi]

Register an RPMI service driver matching the OP-TEE service UUID and
integrate it with OP-TEE module initialization and removal.

Check the service API version, query the trusted OS revision, and
obtain the RPC parameter and logical notification counts. Initialize
shared-memory tracking, the call queue, supplicant state and internal
context before publishing the client and supplicant TEE devices.

Connect the RPMI backend to the common OP-TEE operations and enumerate
trusted application devices. Enable in-kernel RPMB routing when the
RPMB subsystem is reachable.

Add removal and probe failure cleanup for the backend resources.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/core.c          |  10 +-
 drivers/tee/optee/optee_private.h |  21 ++-
 drivers/tee/optee/rpmi_abi.c      | 286 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+), 5 deletions(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index a52c1f498b99..8a44a25ebc66 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -220,6 +220,7 @@ void optee_remove_common(struct optee *optee)
 
 static int smc_abi_rc;
 static int ffa_abi_rc;
+static int rpmi_abi_rc;
 static bool intf_is_regged;
 
 static int __init optee_core_init(void)
@@ -245,14 +246,15 @@ static int __init optee_core_init(void)
 
 	smc_abi_rc = optee_smc_abi_register();
 	ffa_abi_rc = optee_ffa_abi_register();
+	rpmi_abi_rc = optee_rpmi_abi_register();
 
-	/* If both failed there's no point with this module */
-	if (smc_abi_rc && ffa_abi_rc) {
+	/* Keep the module if any supported transport registered successfully. */
+	if (smc_abi_rc && ffa_abi_rc && rpmi_abi_rc) {
 		if (IS_REACHABLE(CONFIG_RPMB)) {
 			rpmb_interface_unregister(&rpmb_class_intf);
 			intf_is_regged = false;
 		}
-		return smc_abi_rc;
+		return -EOPNOTSUPP;
 	}
 
 	return 0;
@@ -270,6 +272,8 @@ static void __exit optee_core_exit(void)
 		optee_smc_abi_unregister();
 	if (!ffa_abi_rc)
 		optee_ffa_abi_unregister();
+	if (!rpmi_abi_rc)
+		optee_rpmi_abi_unregister();
 }
 module_exit(optee_core_exit);
 
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 2422caf3c883..07c27e322a71 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -188,6 +188,8 @@ struct rpmi_tee_device;
  * @rdev: owning RPMI service device
  * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
  * @shm_rht: lookup by the host-endian parcel ID and nonce pair
+ * @sec_caps: negotiated optional OPTEE_RPMI_CAP_* features
+ * @notification_count: negotiated nonzero number of logical notification keys
  *
  * Callers keep their tee_shm alive while using its registration. Lookup
  * returns a raw pointer; the mutex does not protect its lifetime after
@@ -199,6 +201,8 @@ struct optee_rpmi {
 	/* Protects parcel lookup, insertion, removal and publication. */
 	struct mutex shm_rht_lock;
 	struct rhashtable shm_rht;
+	u32 sec_caps;
+	u32 notification_count;
 };
 #endif
 
@@ -211,8 +215,8 @@ struct optee;
  * @os_build_id:	OP-TEE OS build identifier (0 if unspecified)
  *
  * Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
- * OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
- * FF-A ABI version.
+ * OPTEE_FFA_GET_OS_VERSION (FF-A ABI) or OPTEE_RPMI_GET_OS_VERSION (RPMI ABI).
+ * This is the trusted OS revision, not a transport ABI version.
  */
 struct optee_revision {
 	u32 os_major;
@@ -490,5 +494,18 @@ static inline void optee_ffa_abi_unregister(void)
 }
 #endif
 
+#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
+int optee_rpmi_abi_register(void);
+void optee_rpmi_abi_unregister(void);
+#else
+static inline int optee_rpmi_abi_register(void)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void optee_rpmi_abi_unregister(void)
+{
+}
+#endif
 
 #endif /*OPTEE_PRIVATE_H*/
diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
index 6e76316794c1..db541f3de425 100644
--- a/drivers/tee/optee/rpmi_abi.c
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -684,3 +684,289 @@ static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
 
 	return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
 }
+
+/* Query and store the trusted OS revision. */
+static int optee_rpmi_get_os_version(struct optee *optee)
+{
+	struct optee_rpmi_probe_req req = {
+		.op = cpu_to_le32(OPTEE_RPMI_GET_OS_VERSION),
+	};
+	struct optee_rpmi_os_resp os;
+	int ret;
+
+	ret = optee_rpmi_call(optee, &req, sizeof(req), &os, sizeof(os));
+	if (ret)
+		return ret;
+
+	optee->revision.os_major = get_unaligned_le32(&os.major);
+	optee->revision.os_minor = get_unaligned_le32(&os.minor);
+	optee->revision.os_build_id = get_unaligned_le64(&os.build_id);
+
+	if (optee->revision.os_build_id)
+		pr_info("revision %u.%u (%016llx)\n",
+			optee->revision.os_major, optee->revision.os_minor,
+			optee->revision.os_build_id);
+	else
+		pr_info("revision %u.%u\n", optee->revision.os_major,
+			optee->revision.os_minor);
+
+	return 0;
+}
+
+/* Query and store secure-world capabilities and buffer limits. */
+static int optee_rpmi_exchange_caps(struct optee *optee)
+{
+	struct optee_rpmi_probe_req req = {
+		.op = cpu_to_le32(OPTEE_RPMI_EXCHANGE_CAPABILITIES),
+	};
+	struct optee_rpmi_caps_resp caps;
+	u32 rpc_count, sec_caps, notif_count;
+	int ret;
+
+	ret = optee_rpmi_call(optee, &req, sizeof(req), &caps, sizeof(caps));
+	if (ret)
+		return ret;
+
+	sec_caps = get_unaligned_le32(&caps.secure_caps);
+	rpc_count = get_unaligned_le32(&caps.rpc_param_count);
+	notif_count = get_unaligned_le32(&caps.notification_count);
+	if (!notif_count || !rpc_count)
+		return -EPROTO;
+
+	optee->rpc_param_count = rpc_count;
+	optee->rpmi.sec_caps = sec_caps;
+	optee->rpmi.notification_count = notif_count;
+	optee->in_kernel_rpmb_routing = IS_REACHABLE(CONFIG_RPMB);
+
+	return 0;
+}
+
+static int optee_rpmi_api_is_compatible(struct optee *optee)
+{
+	struct optee_rpmi_probe_req req = {
+		.op = cpu_to_le32(OPTEE_RPMI_GET_API_VERSION),
+	};
+	struct optee_rpmi_api_resp api;
+	int ret;
+
+	ret = optee_rpmi_call(optee, &req, sizeof(req), &api, sizeof(api));
+	if (ret)
+		return ret;
+
+	if (get_unaligned_le32(&api.major) != OPTEE_RPMI_VERSION_MAJOR)
+		return -EPROTONOSUPPORT;
+
+	/* Version 1.0 has no minimum minor revision beyond zero. */
+	return 0;
+}
+
+static void optee_rpmi_get_version(struct tee_device *teedev,
+				   struct tee_ioctl_version_data *vers)
+{
+	*vers = (struct tee_ioctl_version_data) {
+		.impl_id = TEE_IMPL_ID_OPTEE,
+		.gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_REG_MEM |
+			    TEE_GEN_CAP_MEMREF_NULL,
+	};
+}
+
+static int optee_rpmi_open(struct tee_context *ctx)
+{
+	return optee_open(ctx, true);
+}
+
+static const struct tee_driver_ops optee_rpmi_clnt_ops = {
+	.get_version = optee_rpmi_get_version,
+	.get_tee_revision = optee_get_revision,
+	.open = optee_rpmi_open,
+	.release = optee_release,
+	.open_session = optee_open_session,
+	.close_session = optee_close_session,
+	.invoke_func = optee_invoke_func,
+	.cancel_req = optee_cancel_req,
+	.shm_register = optee_rpmi_shm_register,
+	.shm_unregister = optee_rpmi_shm_unregister,
+};
+
+static const struct tee_driver_ops optee_rpmi_supp_ops = {
+	.get_version = optee_rpmi_get_version,
+	.get_tee_revision = optee_get_revision,
+	.open = optee_rpmi_open,
+	.release = optee_release_supp,
+	.supp_recv = optee_supp_recv,
+	.supp_send = optee_supp_send,
+	.shm_register = optee_rpmi_shm_register,
+	.shm_unregister = optee_rpmi_shm_unregister_supp,
+};
+
+static const struct tee_desc optee_rpmi_clnt_desc = {
+	.name = DRIVER_NAME "-rpmi-clnt",
+	.ops = &optee_rpmi_clnt_ops,
+	.owner = THIS_MODULE,
+};
+
+static const struct tee_desc optee_rpmi_supp_desc = {
+	.name = DRIVER_NAME "-rpmi-supp",
+	.ops = &optee_rpmi_supp_ops,
+	.owner = THIS_MODULE,
+	.flags = TEE_DESC_PRIVILEGED,
+};
+
+static const struct optee_ops optee_rpmi_ops = {
+	.do_call_with_arg = optee_rpmi_do_call_with_arg,
+	.to_msg_param = optee_rpmi_to_msg_param,
+	.from_msg_param = optee_rpmi_from_msg_param,
+};
+
+/* Keep callback state and memory tables alive until all TEE users release. */
+static void optee_rpmi_remove(struct rpmi_tee_device *rdev)
+{
+	struct optee *optee = dev_get_drvdata(&rdev->dev);
+
+	optee_remove_common(optee);
+	optee_rpmi_shm_rht_uninit(optee);
+	kfree(optee);
+}
+
+static int optee_rpmi_probe(struct rpmi_tee_device *rdev)
+{
+	struct tee_device *teedev;
+	struct tee_context *ctx;
+	int ret;
+
+	struct optee *optee __free(kfree) = kzalloc_obj(*optee);
+	if (!optee)
+		return -ENOMEM;
+
+	optee->rpmi.rdev = rdev;
+	optee->ops = &optee_rpmi_ops;
+
+	ret = optee_rpmi_api_is_compatible(optee);
+	if (ret)
+		return ret;
+
+	ret = optee_rpmi_get_os_version(optee);
+	if (ret)
+		return ret;
+
+	ret = optee_rpmi_exchange_caps(optee);
+	if (ret)
+		return ret;
+
+	optee->pool = optee_rpmi_shm_pool_alloc();
+	if (IS_ERR(optee->pool))
+		return PTR_ERR(optee->pool);
+
+	ret = optee_rpmi_shm_rht_init(optee);
+	if (ret)
+		goto err_pool;
+
+	optee_cq_init(&optee->call_queue, 0);
+	optee_supp_init(&optee->supp);
+	optee_shm_arg_cache_init(optee, OPTEE_SHM_ARG_SHARED);
+	mutex_init(&optee->rpmb_dev_mutex);
+	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
+	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
+	ret = optee_notif_init(optee, optee->rpmi.notification_count);
+	if (ret)
+		goto err_common;
+
+	/* Allocate all keys, then restrict the inclusive bound to the last key. */
+	optee->notif.max_key = optee->rpmi.notification_count - 1;
+
+	teedev = tee_device_alloc(&optee_rpmi_clnt_desc, &rdev->dev,
+				  optee->pool, optee);
+	if (IS_ERR(teedev)) {
+		ret = PTR_ERR(teedev);
+		goto err_notif;
+	}
+	optee->teedev = teedev;
+
+	teedev = tee_device_alloc(&optee_rpmi_supp_desc, &rdev->dev,
+				  optee->pool, optee);
+	if (IS_ERR(teedev)) {
+		ret = PTR_ERR(teedev);
+		goto err_devices;
+	}
+	optee->supp_teedev = teedev;
+
+	optee_set_dev_group(optee);
+
+	/* Internal RPC allocation must be ready before userspace can enter. */
+	ctx = teedev_open(optee->teedev);
+	if (IS_ERR(ctx)) {
+		ret = PTR_ERR(ctx);
+		goto err_devices;
+	}
+
+	optee->ctx = ctx;
+	dev_set_drvdata(&rdev->dev, optee);
+	if (optee->in_kernel_rpmb_routing)
+		blocking_notifier_chain_register(&optee_rpmb_intf_added,
+						 &optee->rpmb_intf);
+
+	ret = tee_device_register(optee->teedev);
+	if (ret)
+		goto err_initialized;
+
+	ret = tee_device_register(optee->supp_teedev);
+	if (ret)
+		goto err_initialized;
+
+	ret = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
+	if (ret)
+		goto err_initialized;
+
+	dev_info(&rdev->dev, "OP-TEE RPMI %u.%u initialized\n",
+		 optee->revision.os_major, optee->revision.os_minor);
+	retain_and_null_ptr(optee);
+
+	return 0;
+
+err_initialized:
+	/* The remove path owns and frees the published backend state. */
+	retain_and_null_ptr(optee);
+	optee_rpmi_remove(rdev);
+
+	return ret;
+err_devices:
+	tee_device_unregister(optee->supp_teedev);
+	tee_device_unregister(optee->teedev);
+	optee_shm_arg_cache_uninit(optee);
+err_notif:
+	optee_notif_uninit(optee);
+err_common:
+	optee_supp_uninit(&optee->supp);
+	mutex_destroy(&optee->call_queue.mutex);
+	rpmb_dev_put(optee->rpmb_dev);
+	mutex_destroy(&optee->rpmb_dev_mutex);
+	optee_rpmi_shm_rht_uninit(optee);
+err_pool:
+	tee_shm_pool_free(optee->pool);
+
+	return ret;
+}
+
+static const struct rpmi_tee_device_id optee_rpmi_device_ids[] = {
+	{ OPTEE_RPMI_SERVICE_UUID },
+	{}
+};
+
+static struct rpmi_tee_driver optee_rpmi_driver = {
+	.name = DRIVER_NAME "-rpmi",
+	.probe = optee_rpmi_probe,
+	.remove = optee_rpmi_remove,
+	.id_table = optee_rpmi_device_ids,
+};
+
+int optee_rpmi_abi_register(void)
+{
+	return rpmi_tee_register(&optee_rpmi_driver);
+}
+
+void optee_rpmi_abi_unregister(void)
+{
+	rpmi_tee_unregister(&optee_rpmi_driver);
+}
+
+MODULE_ALIAS("rpmi_tee:486178e0-e7f8-11e3-bc5e-0002a5d5c51b");

-- 
2.34.1

Re: [PATCH RFC v2 7/8] tee: optee: bind RPMI services and negotiate backend capabilities

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] tee: optee: mixing goto and __free() violates cleanup guidelines
- [Low] tee: optee: missing cleanup of shm_arg_cache on probe error paths
--

commit 3ef22394684ff6d8b2126c91fa4c8bb2d1960d68
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

tee: optee: bind RPMI services and negotiate backend capabilities

Register an RPMI service driver matching the OP-TEE service UUID and integrate
it with OP-TEE module initialization and removal. It checks the service API
version, queries the trusted OS revision, initializes shared-memory tracking,
and connects the RPMI backend to the common OP-TEE operations.

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index 6e76316794c18..db541f3de4250 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c
[ ... ]
> +static int optee_rpmi_probe(struct rpmi_tee_device *rdev)
> +{
> +	struct tee_device *teedev;
> +	struct tee_context *ctx;
> +	int ret;
> +
> +	struct optee *optee __free(kfree) = kzalloc_obj(*optee);

[Severity: Medium]
Does this function mix goto-based error handling and scope-based cleanup?

According to the kernel cleanup subsystem guidelines, mixing goto and
scope-based cleanups like __free() in the same function creates confusing
ownership semantics and raises the risk of missed cleanups. The guidelines
suggest converting all resources to scope-based cleanup or none at all.

> +	if (!optee)
> +		return -ENOMEM;
> +
[ ... ]
> +	optee_cq_init(&optee->call_queue, 0);
> +	optee_supp_init(&optee->supp);
> +	optee_shm_arg_cache_init(optee, OPTEE_SHM_ARG_SHARED);
> +	mutex_init(&optee->rpmb_dev_mutex);
> +	INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
> +	optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
> +	ret = optee_notif_init(optee, optee->rpmi.notification_count);
> +	if (ret)
> +		goto err_common;
[ ... ]
> +err_initialized:
> +	/* The remove path owns and frees the published backend state. */
> +	retain_and_null_ptr(optee);
> +	optee_rpmi_remove(rdev);
> +
> +	return ret;
> +err_devices:
> +	tee_device_unregister(optee->supp_teedev);
> +	tee_device_unregister(optee->teedev);
> +	optee_shm_arg_cache_uninit(optee);
> +err_notif:
> +	optee_notif_uninit(optee);
> +err_common:

[Severity: Low]
Are we missing the cleanup of the argument cache on the err_common path?

If optee_notif_init() fails, the code jumps to err_common, bypassing
err_devices where optee_shm_arg_cache_uninit() is called. This skips the
mutex_destroy() for the argument cache. The actual memory for the optee
struct is freed via the scope-based cleanup, but the logical teardown
is bypassed.

> +	optee_supp_uninit(&optee->supp);
> +	mutex_destroy(&optee->call_queue.mutex);
> +	rpmb_dev_put(optee->rpmb_dev);
> +	mutex_destroy(&optee->rpmb_dev_mutex);
> +	optee_rpmi_shm_rht_uninit(optee);
> +err_pool:
> +	tee_shm_pool_free(optee->pool);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=7

Re: [PATCH RFC v2 7/8] tee: optee: bind RPMI services and negotiate backend capabilities

[Jens Wiklander]

Hi Amir,

On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> Register an RPMI service driver matching the OP-TEE service UUID and
> integrate it with OP-TEE module initialization and removal.
>
> Check the service API version, query the trusted OS revision, and
> obtain the RPC parameter and logical notification counts. Initialize
> shared-memory tracking, the call queue, supplicant state and internal
> context before publishing the client and supplicant TEE devices.
>
> Connect the RPMI backend to the common OP-TEE operations and enumerate
> trusted application devices. Enable in-kernel RPMB routing when the
> RPMB subsystem is reachable.
>
> Add removal and probe failure cleanup for the backend resources.
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
>  drivers/tee/optee/core.c          |  10 +-
>  drivers/tee/optee/optee_private.h |  21 ++-
>  drivers/tee/optee/rpmi_abi.c      | 286 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 312 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index a52c1f498b99..8a44a25ebc66 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -220,6 +220,7 @@ void optee_remove_common(struct optee *optee)
>
>  static int smc_abi_rc;
>  static int ffa_abi_rc;
> +static int rpmi_abi_rc;
>  static bool intf_is_regged;
>
>  static int __init optee_core_init(void)
> @@ -245,14 +246,15 @@ static int __init optee_core_init(void)
>
>         smc_abi_rc = optee_smc_abi_register();
>         ffa_abi_rc = optee_ffa_abi_register();
> +       rpmi_abi_rc = optee_rpmi_abi_register();
>
> -       /* If both failed there's no point with this module */
> -       if (smc_abi_rc && ffa_abi_rc) {
> +       /* Keep the module if any supported transport registered successfully. */
> +       if (smc_abi_rc && ffa_abi_rc && rpmi_abi_rc) {
>                 if (IS_REACHABLE(CONFIG_RPMB)) {
>                         rpmb_interface_unregister(&rpmb_class_intf);
>                         intf_is_regged = false;
>                 }
> -               return smc_abi_rc;
> +               return -EOPNOTSUPP;
>         }
>
>         return 0;
> @@ -270,6 +272,8 @@ static void __exit optee_core_exit(void)
>                 optee_smc_abi_unregister();
>         if (!ffa_abi_rc)
>                 optee_ffa_abi_unregister();
> +       if (!rpmi_abi_rc)
> +               optee_rpmi_abi_unregister();
>  }
>  module_exit(optee_core_exit);
>
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 2422caf3c883..07c27e322a71 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -188,6 +188,8 @@ struct rpmi_tee_device;
>   * @rdev: owning RPMI service device
>   * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
>   * @shm_rht: lookup by the host-endian parcel ID and nonce pair
> + * @sec_caps: negotiated optional OPTEE_RPMI_CAP_* features
> + * @notification_count: negotiated nonzero number of logical notification keys
>   *
>   * Callers keep their tee_shm alive while using its registration. Lookup
>   * returns a raw pointer; the mutex does not protect its lifetime after
> @@ -199,6 +201,8 @@ struct optee_rpmi {
>         /* Protects parcel lookup, insertion, removal and publication. */
>         struct mutex shm_rht_lock;
>         struct rhashtable shm_rht;
> +       u32 sec_caps;
> +       u32 notification_count;

Why are these two needed?

>  };
>  #endif
>
> @@ -211,8 +215,8 @@ struct optee;
>   * @os_build_id:       OP-TEE OS build identifier (0 if unspecified)
>   *
>   * Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
> - * OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
> - * FF-A ABI version.
> + * OPTEE_FFA_GET_OS_VERSION (FF-A ABI) or OPTEE_RPMI_GET_OS_VERSION (RPMI ABI).
> + * This is the trusted OS revision, not a transport ABI version.
>   */
>  struct optee_revision {
>         u32 os_major;
> @@ -490,5 +494,18 @@ static inline void optee_ffa_abi_unregister(void)
>  }
>  #endif
>
> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
> +int optee_rpmi_abi_register(void);
> +void optee_rpmi_abi_unregister(void);
> +#else
> +static inline int optee_rpmi_abi_register(void)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline void optee_rpmi_abi_unregister(void)
> +{
> +}
> +#endif
>
>  #endif /*OPTEE_PRIVATE_H*/
> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index 6e76316794c1..db541f3de425 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c
> @@ -684,3 +684,289 @@ static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
>
>         return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
>  }
> +
> +/* Query and store the trusted OS revision. */
> +static int optee_rpmi_get_os_version(struct optee *optee)
> +{
> +       struct optee_rpmi_probe_req req = {
> +               .op = cpu_to_le32(OPTEE_RPMI_GET_OS_VERSION),
> +       };
> +       struct optee_rpmi_os_resp os;
> +       int ret;
> +
> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &os, sizeof(os));
> +       if (ret)
> +               return ret;
> +
> +       optee->revision.os_major = get_unaligned_le32(&os.major);
> +       optee->revision.os_minor = get_unaligned_le32(&os.minor);
> +       optee->revision.os_build_id = get_unaligned_le64(&os.build_id);
> +
> +       if (optee->revision.os_build_id)
> +               pr_info("revision %u.%u (%016llx)\n",
> +                       optee->revision.os_major, optee->revision.os_minor,
> +                       optee->revision.os_build_id);
> +       else
> +               pr_info("revision %u.%u\n", optee->revision.os_major,
> +                       optee->revision.os_minor);
> +
> +       return 0;
> +}
> +
> +/* Query and store secure-world capabilities and buffer limits. */
> +static int optee_rpmi_exchange_caps(struct optee *optee)
> +{
> +       struct optee_rpmi_probe_req req = {
> +               .op = cpu_to_le32(OPTEE_RPMI_EXCHANGE_CAPABILITIES),
> +       };
> +       struct optee_rpmi_caps_resp caps;
> +       u32 rpc_count, sec_caps, notif_count;
> +       int ret;
> +
> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &caps, sizeof(caps));
> +       if (ret)
> +               return ret;
> +
> +       sec_caps = get_unaligned_le32(&caps.secure_caps);
> +       rpc_count = get_unaligned_le32(&caps.rpc_param_count);
> +       notif_count = get_unaligned_le32(&caps.notification_count);
> +       if (!notif_count || !rpc_count)
> +               return -EPROTO;
> +
> +       optee->rpc_param_count = rpc_count;
> +       optee->rpmi.sec_caps = sec_caps;
> +       optee->rpmi.notification_count = notif_count;
> +       optee->in_kernel_rpmb_routing = IS_REACHABLE(CONFIG_RPMB);

What if OP-TEE is built without RPMB support?

> +
> +       return 0;
> +}
> +
> +static int optee_rpmi_api_is_compatible(struct optee *optee)
> +{
> +       struct optee_rpmi_probe_req req = {
> +               .op = cpu_to_le32(OPTEE_RPMI_GET_API_VERSION),
> +       };
> +       struct optee_rpmi_api_resp api;
> +       int ret;
> +
> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &api, sizeof(api));
> +       if (ret)
> +               return ret;
> +
> +       if (get_unaligned_le32(&api.major) != OPTEE_RPMI_VERSION_MAJOR)
> +               return -EPROTONOSUPPORT;
> +
> +       /* Version 1.0 has no minimum minor revision beyond zero. */
> +       return 0;
> +}
> +
> +static void optee_rpmi_get_version(struct tee_device *teedev,
> +                                  struct tee_ioctl_version_data *vers)
> +{
> +       *vers = (struct tee_ioctl_version_data) {
> +               .impl_id = TEE_IMPL_ID_OPTEE,
> +               .gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_REG_MEM |
> +                           TEE_GEN_CAP_MEMREF_NULL,
> +       };
> +}
> +
> +static int optee_rpmi_open(struct tee_context *ctx)
> +{
> +       return optee_open(ctx, true);
> +}
> +
> +static const struct tee_driver_ops optee_rpmi_clnt_ops = {
> +       .get_version = optee_rpmi_get_version,
> +       .get_tee_revision = optee_get_revision,
> +       .open = optee_rpmi_open,
> +       .release = optee_release,
> +       .open_session = optee_open_session,
> +       .close_session = optee_close_session,
> +       .invoke_func = optee_invoke_func,
> +       .cancel_req = optee_cancel_req,
> +       .shm_register = optee_rpmi_shm_register,
> +       .shm_unregister = optee_rpmi_shm_unregister,
> +};
> +
> +static const struct tee_driver_ops optee_rpmi_supp_ops = {
> +       .get_version = optee_rpmi_get_version,
> +       .get_tee_revision = optee_get_revision,
> +       .open = optee_rpmi_open,
> +       .release = optee_release_supp,
> +       .supp_recv = optee_supp_recv,
> +       .supp_send = optee_supp_send,
> +       .shm_register = optee_rpmi_shm_register,
> +       .shm_unregister = optee_rpmi_shm_unregister_supp,
> +};
> +
> +static const struct tee_desc optee_rpmi_clnt_desc = {
> +       .name = DRIVER_NAME "-rpmi-clnt",
> +       .ops = &optee_rpmi_clnt_ops,
> +       .owner = THIS_MODULE,
> +};
> +
> +static const struct tee_desc optee_rpmi_supp_desc = {
> +       .name = DRIVER_NAME "-rpmi-supp",
> +       .ops = &optee_rpmi_supp_ops,
> +       .owner = THIS_MODULE,
> +       .flags = TEE_DESC_PRIVILEGED,
> +};
> +
> +static const struct optee_ops optee_rpmi_ops = {
> +       .do_call_with_arg = optee_rpmi_do_call_with_arg,
> +       .to_msg_param = optee_rpmi_to_msg_param,
> +       .from_msg_param = optee_rpmi_from_msg_param,
> +};
> +
> +/* Keep callback state and memory tables alive until all TEE users release. */
> +static void optee_rpmi_remove(struct rpmi_tee_device *rdev)
> +{
> +       struct optee *optee = dev_get_drvdata(&rdev->dev);
> +
> +       optee_remove_common(optee);
> +       optee_rpmi_shm_rht_uninit(optee);
> +       kfree(optee);
> +}
> +
> +static int optee_rpmi_probe(struct rpmi_tee_device *rdev)
> +{
> +       struct tee_device *teedev;
> +       struct tee_context *ctx;
> +       int ret;
> +
> +       struct optee *optee __free(kfree) = kzalloc_obj(*optee);

The cleanup macros should, if I understand it correctly, not be used
in functions using gotos for cleanup.

> +       if (!optee)
> +               return -ENOMEM;
> +
> +       optee->rpmi.rdev = rdev;
> +       optee->ops = &optee_rpmi_ops;
> +
> +       ret = optee_rpmi_api_is_compatible(optee);
> +       if (ret)
> +               return ret;
> +
> +       ret = optee_rpmi_get_os_version(optee);
> +       if (ret)
> +               return ret;
> +
> +       ret = optee_rpmi_exchange_caps(optee);
> +       if (ret)
> +               return ret;
> +
> +       optee->pool = optee_rpmi_shm_pool_alloc();

Perhaps it's just me, but it seems a bit odd to store an err pointer
in a struct like this.

> +       if (IS_ERR(optee->pool))
> +               return PTR_ERR(optee->pool);
> +
> +       ret = optee_rpmi_shm_rht_init(optee);
> +       if (ret)
> +               goto err_pool;
> +
> +       optee_cq_init(&optee->call_queue, 0);
> +       optee_supp_init(&optee->supp);
> +       optee_shm_arg_cache_init(optee, OPTEE_SHM_ARG_SHARED);
> +       mutex_init(&optee->rpmb_dev_mutex);
> +       INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
> +       optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
> +       ret = optee_notif_init(optee, optee->rpmi.notification_count);
> +       if (ret)
> +               goto err_common;
> +
> +       /* Allocate all keys, then restrict the inclusive bound to the last key. */
> +       optee->notif.max_key = optee->rpmi.notification_count - 1;

Why? Do you have any plans for that?

Cheers,
Jens

> +
> +       teedev = tee_device_alloc(&optee_rpmi_clnt_desc, &rdev->dev,
> +                                 optee->pool, optee);
> +       if (IS_ERR(teedev)) {
> +               ret = PTR_ERR(teedev);
> +               goto err_notif;
> +       }
> +       optee->teedev = teedev;
> +
> +       teedev = tee_device_alloc(&optee_rpmi_supp_desc, &rdev->dev,
> +                                 optee->pool, optee);
> +       if (IS_ERR(teedev)) {
> +               ret = PTR_ERR(teedev);
> +               goto err_devices;
> +       }
> +       optee->supp_teedev = teedev;
> +
> +       optee_set_dev_group(optee);
> +
> +       /* Internal RPC allocation must be ready before userspace can enter. */
> +       ctx = teedev_open(optee->teedev);
> +       if (IS_ERR(ctx)) {
> +               ret = PTR_ERR(ctx);
> +               goto err_devices;
> +       }
> +
> +       optee->ctx = ctx;
> +       dev_set_drvdata(&rdev->dev, optee);
> +       if (optee->in_kernel_rpmb_routing)
> +               blocking_notifier_chain_register(&optee_rpmb_intf_added,
> +                                                &optee->rpmb_intf);
> +
> +       ret = tee_device_register(optee->teedev);
> +       if (ret)
> +               goto err_initialized;
> +
> +       ret = tee_device_register(optee->supp_teedev);
> +       if (ret)
> +               goto err_initialized;
> +
> +       ret = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
> +       if (ret)
> +               goto err_initialized;
> +
> +       dev_info(&rdev->dev, "OP-TEE RPMI %u.%u initialized\n",
> +                optee->revision.os_major, optee->revision.os_minor);
> +       retain_and_null_ptr(optee);
> +
> +       return 0;
> +
> +err_initialized:
> +       /* The remove path owns and frees the published backend state. */
> +       retain_and_null_ptr(optee);
> +       optee_rpmi_remove(rdev);
> +
> +       return ret;
> +err_devices:
> +       tee_device_unregister(optee->supp_teedev);
> +       tee_device_unregister(optee->teedev);
> +       optee_shm_arg_cache_uninit(optee);
> +err_notif:
> +       optee_notif_uninit(optee);
> +err_common:
> +       optee_supp_uninit(&optee->supp);
> +       mutex_destroy(&optee->call_queue.mutex);
> +       rpmb_dev_put(optee->rpmb_dev);
> +       mutex_destroy(&optee->rpmb_dev_mutex);
> +       optee_rpmi_shm_rht_uninit(optee);
> +err_pool:
> +       tee_shm_pool_free(optee->pool);
> +
> +       return ret;
> +}
> +
> +static const struct rpmi_tee_device_id optee_rpmi_device_ids[] = {
> +       { OPTEE_RPMI_SERVICE_UUID },
> +       {}
> +};
> +
> +static struct rpmi_tee_driver optee_rpmi_driver = {
> +       .name = DRIVER_NAME "-rpmi",
> +       .probe = optee_rpmi_probe,
> +       .remove = optee_rpmi_remove,
> +       .id_table = optee_rpmi_device_ids,
> +};
> +
> +int optee_rpmi_abi_register(void)
> +{
> +       return rpmi_tee_register(&optee_rpmi_driver);
> +}
> +
> +void optee_rpmi_abi_unregister(void)
> +{
> +       rpmi_tee_unregister(&optee_rpmi_driver);
> +}
> +
> +MODULE_ALIAS("rpmi_tee:486178e0-e7f8-11e3-bc5e-0002a5d5c51b");
>
> --
> 2.34.1
>

Re: [PATCH RFC v2 7/8] tee: optee: bind RPMI services and negotiate backend capabilities

[Amirreza Zarrabi]

Hi jens,

On 10/8/2026 7:20 PM, Jens Wiklander wrote:
> Hi Amir,
> 
> On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> Register an RPMI service driver matching the OP-TEE service UUID and
>> integrate it with OP-TEE module initialization and removal.
>>
>> Check the service API version, query the trusted OS revision, and
>> obtain the RPC parameter and logical notification counts. Initialize
>> shared-memory tracking, the call queue, supplicant state and internal
>> context before publishing the client and supplicant TEE devices.
>>
>> Connect the RPMI backend to the common OP-TEE operations and enumerate
>> trusted application devices. Enable in-kernel RPMB routing when the
>> RPMB subsystem is reachable.
>>
>> Add removal and probe failure cleanup for the backend resources.
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>>  drivers/tee/optee/core.c          |  10 +-
>>  drivers/tee/optee/optee_private.h |  21 ++-
>>  drivers/tee/optee/rpmi_abi.c      | 286 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 312 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
>> index a52c1f498b99..8a44a25ebc66 100644
>> --- a/drivers/tee/optee/core.c
>> +++ b/drivers/tee/optee/core.c
>> @@ -220,6 +220,7 @@ void optee_remove_common(struct optee *optee)
>>
>>  static int smc_abi_rc;
>>  static int ffa_abi_rc;
>> +static int rpmi_abi_rc;
>>  static bool intf_is_regged;
>>
>>  static int __init optee_core_init(void)
>> @@ -245,14 +246,15 @@ static int __init optee_core_init(void)
>>
>>         smc_abi_rc = optee_smc_abi_register();
>>         ffa_abi_rc = optee_ffa_abi_register();
>> +       rpmi_abi_rc = optee_rpmi_abi_register();
>>
>> -       /* If both failed there's no point with this module */
>> -       if (smc_abi_rc && ffa_abi_rc) {
>> +       /* Keep the module if any supported transport registered successfully. */
>> +       if (smc_abi_rc && ffa_abi_rc && rpmi_abi_rc) {
>>                 if (IS_REACHABLE(CONFIG_RPMB)) {
>>                         rpmb_interface_unregister(&rpmb_class_intf);
>>                         intf_is_regged = false;
>>                 }
>> -               return smc_abi_rc;
>> +               return -EOPNOTSUPP;
>>         }
>>
>>         return 0;
>> @@ -270,6 +272,8 @@ static void __exit optee_core_exit(void)
>>                 optee_smc_abi_unregister();
>>         if (!ffa_abi_rc)
>>                 optee_ffa_abi_unregister();
>> +       if (!rpmi_abi_rc)
>> +               optee_rpmi_abi_unregister();
>>  }
>>  module_exit(optee_core_exit);
>>
>> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
>> index 2422caf3c883..07c27e322a71 100644
>> --- a/drivers/tee/optee/optee_private.h
>> +++ b/drivers/tee/optee/optee_private.h
>> @@ -188,6 +188,8 @@ struct rpmi_tee_device;
>>   * @rdev: owning RPMI service device
>>   * @shm_rht_lock: protects parcel lookup, insertion, removal and publication
>>   * @shm_rht: lookup by the host-endian parcel ID and nonce pair
>> + * @sec_caps: negotiated optional OPTEE_RPMI_CAP_* features
>> + * @notification_count: negotiated nonzero number of logical notification keys
>>   *
>>   * Callers keep their tee_shm alive while using its registration. Lookup
>>   * returns a raw pointer; the mutex does not protect its lifetime after
>> @@ -199,6 +201,8 @@ struct optee_rpmi {
>>         /* Protects parcel lookup, insertion, removal and publication. */
>>         struct mutex shm_rht_lock;
>>         struct rhashtable shm_rht;
>> +       u32 sec_caps;
>> +       u32 notification_count;
> 
> Why are these two needed?
> 

Neither needs to be retained in the instance state: sec_caps is currently unused,
and notification_count is consumed during probe. I'll remove both fields.
I thought it would be nice to keep them.

>>  };
>>  #endif
>>
>> @@ -211,8 +215,8 @@ struct optee;
>>   * @os_build_id:       OP-TEE OS build identifier (0 if unspecified)
>>   *
>>   * Values come from OPTEE_SMC_CALL_GET_OS_REVISION (SMC ABI) or
>> - * OPTEE_FFA_GET_OS_VERSION (FF-A ABI); this is the trusted OS revision, not an
>> - * FF-A ABI version.
>> + * OPTEE_FFA_GET_OS_VERSION (FF-A ABI) or OPTEE_RPMI_GET_OS_VERSION (RPMI ABI).
>> + * This is the trusted OS revision, not a transport ABI version.
>>   */
>>  struct optee_revision {
>>         u32 os_major;
>> @@ -490,5 +494,18 @@ static inline void optee_ffa_abi_unregister(void)
>>  }
>>  #endif
>>
>> +#if IS_REACHABLE(CONFIG_RISCV_RPMI_TEE_TRANSPORT)
>> +int optee_rpmi_abi_register(void);
>> +void optee_rpmi_abi_unregister(void);
>> +#else
>> +static inline int optee_rpmi_abi_register(void)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>> +static inline void optee_rpmi_abi_unregister(void)
>> +{
>> +}
>> +#endif
>>
>>  #endif /*OPTEE_PRIVATE_H*/
>> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
>> index 6e76316794c1..db541f3de425 100644
>> --- a/drivers/tee/optee/rpmi_abi.c
>> +++ b/drivers/tee/optee/rpmi_abi.c
>> @@ -684,3 +684,289 @@ static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
>>
>>         return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
>>  }
>> +
>> +/* Query and store the trusted OS revision. */
>> +static int optee_rpmi_get_os_version(struct optee *optee)
>> +{
>> +       struct optee_rpmi_probe_req req = {
>> +               .op = cpu_to_le32(OPTEE_RPMI_GET_OS_VERSION),
>> +       };
>> +       struct optee_rpmi_os_resp os;
>> +       int ret;
>> +
>> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &os, sizeof(os));
>> +       if (ret)
>> +               return ret;
>> +
>> +       optee->revision.os_major = get_unaligned_le32(&os.major);
>> +       optee->revision.os_minor = get_unaligned_le32(&os.minor);
>> +       optee->revision.os_build_id = get_unaligned_le64(&os.build_id);
>> +
>> +       if (optee->revision.os_build_id)
>> +               pr_info("revision %u.%u (%016llx)\n",
>> +                       optee->revision.os_major, optee->revision.os_minor,
>> +                       optee->revision.os_build_id);
>> +       else
>> +               pr_info("revision %u.%u\n", optee->revision.os_major,
>> +                       optee->revision.os_minor);
>> +
>> +       return 0;
>> +}
>> +
>> +/* Query and store secure-world capabilities and buffer limits. */
>> +static int optee_rpmi_exchange_caps(struct optee *optee)
>> +{
>> +       struct optee_rpmi_probe_req req = {
>> +               .op = cpu_to_le32(OPTEE_RPMI_EXCHANGE_CAPABILITIES),
>> +       };
>> +       struct optee_rpmi_caps_resp caps;
>> +       u32 rpc_count, sec_caps, notif_count;
>> +       int ret;
>> +
>> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &caps, sizeof(caps));
>> +       if (ret)
>> +               return ret;
>> +
>> +       sec_caps = get_unaligned_le32(&caps.secure_caps);
>> +       rpc_count = get_unaligned_le32(&caps.rpc_param_count);
>> +       notif_count = get_unaligned_le32(&caps.notification_count);
>> +       if (!notif_count || !rpc_count)
>> +               return -EPROTO;
>> +
>> +       optee->rpc_param_count = rpc_count;
>> +       optee->rpmi.sec_caps = sec_caps;
>> +       optee->rpmi.notification_count = notif_count;
>> +       optee->in_kernel_rpmb_routing = IS_REACHABLE(CONFIG_RPMB);
> 
> What if OP-TEE is built without RPMB support?
> 

I discussed this with Sumit, and the intention was to make RPMB probing
part of the baseline for the new ABI rather than negotiate it separately.
I can enable the capability bit if it should remain optional.

>> +
>> +       return 0;
>> +}
>> +
>> +static int optee_rpmi_api_is_compatible(struct optee *optee)
>> +{
>> +       struct optee_rpmi_probe_req req = {
>> +               .op = cpu_to_le32(OPTEE_RPMI_GET_API_VERSION),
>> +       };
>> +       struct optee_rpmi_api_resp api;
>> +       int ret;
>> +
>> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &api, sizeof(api));
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (get_unaligned_le32(&api.major) != OPTEE_RPMI_VERSION_MAJOR)
>> +               return -EPROTONOSUPPORT;
>> +
>> +       /* Version 1.0 has no minimum minor revision beyond zero. */
>> +       return 0;
>> +}
>> +
>> +static void optee_rpmi_get_version(struct tee_device *teedev,
>> +                                  struct tee_ioctl_version_data *vers)
>> +{
>> +       *vers = (struct tee_ioctl_version_data) {
>> +               .impl_id = TEE_IMPL_ID_OPTEE,
>> +               .gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_REG_MEM |
>> +                           TEE_GEN_CAP_MEMREF_NULL,
>> +       };
>> +}
>> +
>> +static int optee_rpmi_open(struct tee_context *ctx)
>> +{
>> +       return optee_open(ctx, true);
>> +}
>> +
>> +static const struct tee_driver_ops optee_rpmi_clnt_ops = {
>> +       .get_version = optee_rpmi_get_version,
>> +       .get_tee_revision = optee_get_revision,
>> +       .open = optee_rpmi_open,
>> +       .release = optee_release,
>> +       .open_session = optee_open_session,
>> +       .close_session = optee_close_session,
>> +       .invoke_func = optee_invoke_func,
>> +       .cancel_req = optee_cancel_req,
>> +       .shm_register = optee_rpmi_shm_register,
>> +       .shm_unregister = optee_rpmi_shm_unregister,
>> +};
>> +
>> +static const struct tee_driver_ops optee_rpmi_supp_ops = {
>> +       .get_version = optee_rpmi_get_version,
>> +       .get_tee_revision = optee_get_revision,
>> +       .open = optee_rpmi_open,
>> +       .release = optee_release_supp,
>> +       .supp_recv = optee_supp_recv,
>> +       .supp_send = optee_supp_send,
>> +       .shm_register = optee_rpmi_shm_register,
>> +       .shm_unregister = optee_rpmi_shm_unregister_supp,
>> +};
>> +
>> +static const struct tee_desc optee_rpmi_clnt_desc = {
>> +       .name = DRIVER_NAME "-rpmi-clnt",
>> +       .ops = &optee_rpmi_clnt_ops,
>> +       .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct tee_desc optee_rpmi_supp_desc = {
>> +       .name = DRIVER_NAME "-rpmi-supp",
>> +       .ops = &optee_rpmi_supp_ops,
>> +       .owner = THIS_MODULE,
>> +       .flags = TEE_DESC_PRIVILEGED,
>> +};
>> +
>> +static const struct optee_ops optee_rpmi_ops = {
>> +       .do_call_with_arg = optee_rpmi_do_call_with_arg,
>> +       .to_msg_param = optee_rpmi_to_msg_param,
>> +       .from_msg_param = optee_rpmi_from_msg_param,
>> +};
>> +
>> +/* Keep callback state and memory tables alive until all TEE users release. */
>> +static void optee_rpmi_remove(struct rpmi_tee_device *rdev)
>> +{
>> +       struct optee *optee = dev_get_drvdata(&rdev->dev);
>> +
>> +       optee_remove_common(optee);
>> +       optee_rpmi_shm_rht_uninit(optee);
>> +       kfree(optee);
>> +}
>> +
>> +static int optee_rpmi_probe(struct rpmi_tee_device *rdev)
>> +{
>> +       struct tee_device *teedev;
>> +       struct tee_context *ctx;
>> +       int ret;
>> +
>> +       struct optee *optee __free(kfree) = kzalloc_obj(*optee);
> 
> The cleanup macros should, if I understand it correctly, not be used
> in functions using gotos for cleanup.

I'll remove all cleanup.h related macros as requested.

> 
>> +       if (!optee)
>> +               return -ENOMEM;
>> +
>> +       optee->rpmi.rdev = rdev;
>> +       optee->ops = &optee_rpmi_ops;
>> +
>> +       ret = optee_rpmi_api_is_compatible(optee);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = optee_rpmi_get_os_version(optee);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = optee_rpmi_exchange_caps(optee);
>> +       if (ret)
>> +               return ret;
>> +
>> +       optee->pool = optee_rpmi_shm_pool_alloc();
> 
> Perhaps it's just me, but it seems a bit odd to store an err pointer
> in a struct like this.
> 

Ack.

>> +       if (IS_ERR(optee->pool))
>> +               return PTR_ERR(optee->pool);
>> +
>> +       ret = optee_rpmi_shm_rht_init(optee);
>> +       if (ret)
>> +               goto err_pool;
>> +
>> +       optee_cq_init(&optee->call_queue, 0);
>> +       optee_supp_init(&optee->supp);
>> +       optee_shm_arg_cache_init(optee, OPTEE_SHM_ARG_SHARED);
>> +       mutex_init(&optee->rpmb_dev_mutex);
>> +       INIT_WORK(&optee->rpmb_scan_bus_work, optee_bus_scan_rpmb);
>> +       optee->rpmb_intf.notifier_call = optee_rpmb_intf_rdev;
>> +       ret = optee_notif_init(optee, optee->rpmi.notification_count);
>> +       if (ret)
>> +               goto err_common;
>> +
>> +       /* Allocate all keys, then restrict the inclusive bound to the last key. */
>> +       optee->notif.max_key = optee->rpmi.notification_count - 1;
> 
> Why? Do you have any plans for that?
> 

The comment is misleading; there is no additional restriction intended.
The ABI currently reports a key count, while the common notification code uses an
inclusive maximum key. I'll change the ABI to report the maximum key instead
and remove this adjustment.

Thanks Jens for the review.

Best Regards,
Amir

> Cheers,
> Jens
> 
>> +
>> +       teedev = tee_device_alloc(&optee_rpmi_clnt_desc, &rdev->dev,
>> +                                 optee->pool, optee);
>> +       if (IS_ERR(teedev)) {
>> +               ret = PTR_ERR(teedev);
>> +               goto err_notif;
>> +       }
>> +       optee->teedev = teedev;
>> +
>> +       teedev = tee_device_alloc(&optee_rpmi_supp_desc, &rdev->dev,
>> +                                 optee->pool, optee);
>> +       if (IS_ERR(teedev)) {
>> +               ret = PTR_ERR(teedev);
>> +               goto err_devices;
>> +       }
>> +       optee->supp_teedev = teedev;
>> +
>> +       optee_set_dev_group(optee);
>> +
>> +       /* Internal RPC allocation must be ready before userspace can enter. */
>> +       ctx = teedev_open(optee->teedev);
>> +       if (IS_ERR(ctx)) {
>> +               ret = PTR_ERR(ctx);
>> +               goto err_devices;
>> +       }
>> +
>> +       optee->ctx = ctx;
>> +       dev_set_drvdata(&rdev->dev, optee);
>> +       if (optee->in_kernel_rpmb_routing)
>> +               blocking_notifier_chain_register(&optee_rpmb_intf_added,
>> +                                                &optee->rpmb_intf);
>> +
>> +       ret = tee_device_register(optee->teedev);
>> +       if (ret)
>> +               goto err_initialized;
>> +
>> +       ret = tee_device_register(optee->supp_teedev);
>> +       if (ret)
>> +               goto err_initialized;
>> +
>> +       ret = optee_enumerate_devices(PTA_CMD_GET_DEVICES);
>> +       if (ret)
>> +               goto err_initialized;
>> +
>> +       dev_info(&rdev->dev, "OP-TEE RPMI %u.%u initialized\n",
>> +                optee->revision.os_major, optee->revision.os_minor);
>> +       retain_and_null_ptr(optee);
>> +
>> +       return 0;
>> +
>> +err_initialized:
>> +       /* The remove path owns and frees the published backend state. */
>> +       retain_and_null_ptr(optee);
>> +       optee_rpmi_remove(rdev);
>> +
>> +       return ret;
>> +err_devices:
>> +       tee_device_unregister(optee->supp_teedev);
>> +       tee_device_unregister(optee->teedev);
>> +       optee_shm_arg_cache_uninit(optee);
>> +err_notif:
>> +       optee_notif_uninit(optee);
>> +err_common:
>> +       optee_supp_uninit(&optee->supp);
>> +       mutex_destroy(&optee->call_queue.mutex);
>> +       rpmb_dev_put(optee->rpmb_dev);
>> +       mutex_destroy(&optee->rpmb_dev_mutex);
>> +       optee_rpmi_shm_rht_uninit(optee);
>> +err_pool:
>> +       tee_shm_pool_free(optee->pool);
>> +
>> +       return ret;
>> +}
>> +
>> +static const struct rpmi_tee_device_id optee_rpmi_device_ids[] = {
>> +       { OPTEE_RPMI_SERVICE_UUID },
>> +       {}
>> +};
>> +
>> +static struct rpmi_tee_driver optee_rpmi_driver = {
>> +       .name = DRIVER_NAME "-rpmi",
>> +       .probe = optee_rpmi_probe,
>> +       .remove = optee_rpmi_remove,
>> +       .id_table = optee_rpmi_device_ids,
>> +};
>> +
>> +int optee_rpmi_abi_register(void)
>> +{
>> +       return rpmi_tee_register(&optee_rpmi_driver);
>> +}
>> +
>> +void optee_rpmi_abi_unregister(void)
>> +{
>> +       rpmi_tee_unregister(&optee_rpmi_driver);
>> +}
>> +
>> +MODULE_ALIAS("rpmi_tee:486178e0-e7f8-11e3-bc5e-0002a5d5c51b");
>>
>> --
>> 2.34.1
>>

Re: [PATCH RFC v2 7/8] tee: optee: bind RPMI services and negotiate backend capabilities

[Jens Wiklander]

Hi Amir,

On Fri, Oct 9, 2026 at 1:10 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> Hi jens,
>
> On 10/8/2026 7:20 PM, Jens Wiklander wrote:
> > Hi Amir,
> >
> > On Tue, Oct 6, 2026 at 2:40 AM Amirreza Zarrabi
> > <amirreza.zarrabi@oss.qualcomm.com> wrote:
> >>
> >> Register an RPMI service driver matching the OP-TEE service UUID and
> >> integrate it with OP-TEE module initialization and removal.
> >>
> >> Check the service API version, query the trusted OS revision, and
> >> obtain the RPC parameter and logical notification counts. Initialize
> >> shared-memory tracking, the call queue, supplicant state and internal
> >> context before publishing the client and supplicant TEE devices.
> >>
> >> Connect the RPMI backend to the common OP-TEE operations and enumerate
> >> trusted application devices. Enable in-kernel RPMB routing when the
> >> RPMB subsystem is reachable.
> >>
> >> Add removal and probe failure cleanup for the backend resources.
> >>
> >> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> >> ---
[snip]
> >> +
> >> +/* Query and store secure-world capabilities and buffer limits. */
> >> +static int optee_rpmi_exchange_caps(struct optee *optee)
> >> +{
> >> +       struct optee_rpmi_probe_req req = {
> >> +               .op = cpu_to_le32(OPTEE_RPMI_EXCHANGE_CAPABILITIES),
> >> +       };
> >> +       struct optee_rpmi_caps_resp caps;
> >> +       u32 rpc_count, sec_caps, notif_count;
> >> +       int ret;
> >> +
> >> +       ret = optee_rpmi_call(optee, &req, sizeof(req), &caps, sizeof(caps));
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       sec_caps = get_unaligned_le32(&caps.secure_caps);
> >> +       rpc_count = get_unaligned_le32(&caps.rpc_param_count);
> >> +       notif_count = get_unaligned_le32(&caps.notification_count);
> >> +       if (!notif_count || !rpc_count)
> >> +               return -EPROTO;
> >> +
> >> +       optee->rpc_param_count = rpc_count;
> >> +       optee->rpmi.sec_caps = sec_caps;
> >> +       optee->rpmi.notification_count = notif_count;
> >> +       optee->in_kernel_rpmb_routing = IS_REACHABLE(CONFIG_RPMB);
> >
> > What if OP-TEE is built without RPMB support?
> >
>
> I discussed this with Sumit, and the intention was to make RPMB probing
> part of the baseline for the new ABI rather than negotiate it separately.
> I can enable the capability bit if it should remain optional.

You have a point. Let's keep it, but we may want to move the
assignment down into optee_rpmi_probe(), as the other probe functions
have it.

Cheers,
Jens

[PATCH RFC v2 8/8] tee: optee: support RPMI asynchronous notification doorbells

[Amirreza Zarrabi]

OP-TEE uses asynchronous notifications to request bottom-half
processing in normal world without waiting for a client call.

Allocate a TEE-to-REE RPMI signal and pass its ID to OP-TEE when
enabling asynchronous notifications. Queue bottom-half processing on
a private workqueue.

Initialize notification delivery before publishing the TEE devices.
Allow probe to continue if setup fails, keeping synchronous RPC
notifications available.

On cleanup, request that OP-TEE stop asynchronous notifications,
relinquish the signal and drain the bottom-half workqueue. Transport
relinquishment does not synchronize an already-selected callback.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
---
 drivers/tee/optee/optee_private.h |  6 +++
 drivers/tee/optee/rpmi_abi.c      | 87 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 07c27e322a71..e5c6381bb872 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -190,6 +190,9 @@ struct rpmi_tee_device;
  * @shm_rht: lookup by the host-endian parcel ID and nonce pair
  * @sec_caps: negotiated optional OPTEE_RPMI_CAP_* features
  * @notification_count: negotiated nonzero number of logical notification keys
+ * @signal: allocated TEE-to-REE doorbell, independent of logical keys
+ * @notif_wq: private bottom-half workqueue, NULL when notifications are inactive
+ * @notif_work: processes bottom halves requested by the RPMI signal
  *
  * Callers keep their tee_shm alive while using its registration. Lookup
  * returns a raw pointer; the mutex does not protect its lifetime after
@@ -203,6 +206,9 @@ struct optee_rpmi {
 	struct rhashtable shm_rht;
 	u32 sec_caps;
 	u32 notification_count;
+	u32 signal;
+	struct workqueue_struct *notif_wq;
+	struct work_struct notif_work;
 };
 #endif
 
diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
index db541f3de425..ffe44d81e857 100644
--- a/drivers/tee/optee/rpmi_abi.c
+++ b/drivers/tee/optee/rpmi_abi.c
@@ -685,6 +685,88 @@ static int optee_rpmi_do_call_with_arg(struct tee_context *ctx,
 	return optee_rpmi_yielding_call(ctx, &req, rpc_arg, system_thread);
 }
 
+/* Yielding bottom halves run here, not in the transport retrieval worker. */
+static void optee_rpmi_notif_work(struct work_struct *work)
+{
+	struct optee_rpmi *rpmi = container_of(work, struct optee_rpmi, notif_work);
+	struct optee *optee = container_of(rpmi, struct optee, rpmi);
+
+	optee_do_bottom_half(optee->ctx);
+}
+
+static void optee_rpmi_notif_callback(struct rpmi_tee_device *rdev, u32 signal,
+				      void *cb_data)
+{
+	struct optee *optee = cb_data;
+
+	queue_work(optee->rpmi.notif_wq, &optee->rpmi.notif_work);
+}
+
+/* Relinquish is not a barrier for an already-selected transport callback. */
+static void optee_rpmi_async_notif_uninit(struct optee *optee)
+{
+	struct optee_rpmi *rpmi = &optee->rpmi;
+	struct rpmi_tee_device *rdev = rpmi->rdev;
+	int ret;
+
+	if (!rpmi->notif_wq)
+		return;
+
+	ret = optee_stop_async_notif(optee->ctx);
+	if (ret)
+		dev_warn(&rdev->dev, "stop notifications failed: %d\n", ret);
+
+	ret = rdev->ops->notifier_ops->notify_relinquish(rdev, rpmi->signal);
+	if (ret && ret != -EOPNOTSUPP)
+		dev_warn(&rdev->dev,
+			 "relinquish notification failed: %d\n", ret);
+
+	destroy_workqueue(rpmi->notif_wq);
+
+	rpmi->notif_wq = NULL;
+}
+
+/* Enable OP-TEE's bottom-half doorbell using the reserved RPMI signal. */
+static int optee_rpmi_enable_async_notif(struct optee *optee)
+{
+	struct optee_rpmi_enable_notif_req req = {
+		.op = cpu_to_le32(OPTEE_RPMI_ENABLE_ASYNC_NOTIF),
+		.signal_id = cpu_to_le32(optee->rpmi.signal),
+	};
+	struct optee_rpmi_status_resp resp;
+
+	return optee_rpmi_call(optee, &req, sizeof(req), &resp, sizeof(resp));
+}
+
+static int optee_rpmi_async_notif_init(struct optee *optee)
+{
+	struct optee_rpmi *rpmi = &optee->rpmi;
+	struct rpmi_tee_device *rdev = rpmi->rdev;
+	int ret;
+
+	INIT_WORK(&rpmi->notif_work, optee_rpmi_notif_work);
+	rpmi->notif_wq = alloc_workqueue("optee_rpmi_notif", WQ_UNBOUND, 1);
+	if (!rpmi->notif_wq)
+		return -ENOMEM;
+
+	ret = rdev->ops->notifier_ops->notify_request(rdev,
+						      optee_rpmi_notif_callback,
+						      optee, &rpmi->signal);
+	if (ret) {
+		destroy_workqueue(rpmi->notif_wq);
+		/* Checked in optee_rpmi_async_notif_uninit(). */
+		rpmi->notif_wq = NULL;
+
+		return ret;
+	}
+
+	ret = optee_rpmi_enable_async_notif(optee);
+	if (ret)
+		optee_rpmi_async_notif_uninit(optee);
+
+	return ret;
+}
+
 /* Query and store the trusted OS revision. */
 static int optee_rpmi_get_os_version(struct optee *optee)
 {
@@ -823,6 +905,7 @@ static void optee_rpmi_remove(struct rpmi_tee_device *rdev)
 {
 	struct optee *optee = dev_get_drvdata(&rdev->dev);
 
+	optee_rpmi_async_notif_uninit(optee);
 	optee_remove_common(optee);
 	optee_rpmi_shm_rht_uninit(optee);
 	kfree(optee);
@@ -901,6 +984,10 @@ static int optee_rpmi_probe(struct rpmi_tee_device *rdev)
 
 	optee->ctx = ctx;
 	dev_set_drvdata(&rdev->dev, optee);
+	ret = optee_rpmi_async_notif_init(optee);
+	if (ret)
+		dev_warn(&rdev->dev, "async notifications unavailable: %d\n", ret);
+
 	if (optee->in_kernel_rpmb_routing)
 		blocking_notifier_chain_register(&optee_rpmb_intf_added,
 						 &optee->rpmb_intf);

-- 
2.34.1

Re: [PATCH RFC v2 8/8] tee: optee: support RPMI asynchronous notification doorbells

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tee: optee: UAF in optee_rpmi_notif_callback() due to unsynchronized teardown
--

commit 78a8a655d3ae3b299cafbd7aaef4c7ef599402d8
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

    tee: optee: support RPMI asynchronous notification doorbells
    
This commit adds support for RPMI asynchronous notifications in OP-TEE. It
allocates an RPMI signal, sets up a callback to queue a bottom-half on a
private workqueue, and provides initialization and teardown routines.

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index db541f3de4250..ffe44d81e8571 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c

[ ... ]

> +static void optee_rpmi_notif_callback(struct rpmi_tee_device *rdev, u32 signal,
> +				      void *cb_data)
> +{
> +	struct optee *optee = cb_data;
> +
> +	queue_work(optee->rpmi.notif_wq, &optee->rpmi.notif_work);

[Severity: High]
Can this callback execute after the optee structure has been freed and the
workqueue destroyed?

If an RPMI notification is dispatched just prior to notify_relinquish(), it
appears this callback could run concurrently with or after the teardown
sequence in optee_rpmi_async_notif_uninit(). This would lead to a
use-after-free on the optee structure and a potential NULL pointer
dereference or crash when passing the destroyed workqueue to queue_work().

> +}
> +
> +/* Relinquish is not a barrier for an already-selected transport callback. */
> +static void optee_rpmi_async_notif_uninit(struct optee *optee)
> +{
> +	struct optee_rpmi *rpmi = &optee->rpmi;
> +	struct rpmi_tee_device *rdev = rpmi->rdev;
> +	int ret;
> +
> +	if (!rpmi->notif_wq)
> +		return;
> +
> +	ret = optee_stop_async_notif(optee->ctx);
> +	if (ret)
> +		dev_warn(&rdev->dev, "stop notifications failed: %d\n", ret);
> +
> +	ret = rdev->ops->notifier_ops->notify_relinquish(rdev, rpmi->signal);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_warn(&rdev->dev,
> +			 "relinquish notification failed: %d\n", ret);
> +
> +	destroy_workqueue(rpmi->notif_wq);
> +
> +	rpmi->notif_wq = NULL;

[Severity: High]
Does this teardown sequence need to synchronize with in-flight callbacks
before destroying the workqueue and returning?

Because notify_relinquish() does not wait for already-selected callbacks to
finish, destroying the workqueue here and subsequently freeing the optee
structure in optee_rpmi_remove() could race with an executing callback.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=8

Re: [PATCH RFC v2 0/8] tee: optee: add RPMI backend support on RISC-V

[Jens Wiklander]

Hi Amir,

On Tue, Oct 6, 2026 at 2:39 AM Amirreza Zarrabi
<amirreza.zarrabi@oss.qualcomm.com> wrote:
>
> This series adds an RPMI backend to the OP-TEE driver for RISC-V.
> It builds on the separately posted RPMI TEE transport series [1],
> which provides service discovery, synchronous calls, memory parcels
> and asynchronous signals.
>
> This revision is effectively a full rewrite of the original RFC.
> The generic RPMI transport has been separated from the OP-TEE backend,
> and the backend has been reworked around the service bus and a new
> OP-TEE control ABI.
>
> Unlike v1, the OP-TEE backend no longer manages SBI MPXY mailbox
> channels or implements RPMI TEE service-group operations directly.
> It binds to the OP-TEE service UUID on the RPMI TEE bus and uses the
> transport's public operations. No additional OP-TEE device-tree node
> is required.
>
> The backend reuses the common OP-TEE session, shared-memory, argument
> cache, call queue and RPC infrastructure. Shared memory is represented
> by RPMI parcel IDs and nonces. Yielding calls pass command and RPC
> buffer ranges within a parcel and resume suspended execution using
> an opaque token returned by OP-TEE.
>
> The OP-TEE control ABI uses fixed-width little-endian messages carried
> in TEE_CALL payloads rather than reproducing FF-A's register-based
> message layout. Asynchronous notifications use an allocated TEE-to-REE
> signal as a bottom-half doorbell, while ordinary logical notifications
> continue through the common RPC path.
>
> Matching OP-TEE OS support for this ABI has not yet been implemented.
> This remains an RFC for review of the backend integration and the
> Linux-to-OP-TEE protocol.
>
> [1] RPMI TEE transport dependency:
> https://lore.kernel.org/op-tee/20260928-riscv-rpmi-tee-abi-v1-0-04908b81d885@oss.qualcomm.com/
>
> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
> ---
> Changes in v2:
> - Effectively rewrite the backend around the separately posted RPMI
>   TEE transport and a new OP-TEE control ABI.
> - Remove backend-owned per-hart mailbox channels and the OP-TEE-specific
>   device-tree binding.
> - Replace the register-like control payload with fixed-width messages,
>   explicit command/RPC buffer ranges and opaque resume tokens.
> - Add a parcel-reference parameter layout carrying the parcel ID,
>   nonce, and 64-bit offset and size without changing parameter size.
> - Make argument-offset support part of the baseline ABI and retain
>   the common shared argument cache.
> - Use a transport-allocated signal for asynchronous bottom-half
>   notifications.
> - Link to v1: https://lore.kernel.org/r/20260912-rpmi-tee-service-grp-dev-v1-0-1d1d35c2a859@oss.qualcomm.com
>
> ---
> Amirreza Zarrabi (8):
>       tee: optee: allow RPMI transport builds on RISC-V
>       tee: optee: define the RPMI control and parcel-reference ABI
>       tee: optee: add RPMI shared-memory and parameter support
>       tee: optee: add RPMI dynamic shared-memory pool
>       tee: optee: add RPMI RPC handling
>       tee: optee: execute yielding RPMI calls
>       tee: optee: bind RPMI services and negotiate backend capabilities
>       tee: optee: support RPMI asynchronous notification doorbells
>
>  drivers/tee/Kconfig               |    3 +-
>  drivers/tee/optee/Kconfig         |    3 +-
>  drivers/tee/optee/Makefile        |    1 +
>  drivers/tee/optee/call.c          |    2 +
>  drivers/tee/optee/core.c          |   10 +-
>  drivers/tee/optee/optee_msg.h     |   38 +-
>  drivers/tee/optee/optee_private.h |   54 +-
>  drivers/tee/optee/optee_rpmi.h    |  234 ++++++++
>  drivers/tee/optee/rpmi_abi.c      | 1059 +++++++++++++++++++++++++++++++++++++
>  9 files changed, 1389 insertions(+), 15 deletions(-)

The rpmi changes in the optee driver harmonize quite well with the
rest of the driver, but there are two things I'd like fixed:
- avoid <linux/cleanup.h> macros. They are distracting.
- use rc for the trivial int ERRNO values.

I would be great with a QEMU-based end-to-end prototype to demonstrate
that the ABI works.

More comments to come in the individual patches.

Cheers,
Jens

Re: [PATCH RFC v2 0/8] tee: optee: add RPMI backend support on RISC-V

[Amirreza Zarrabi]

Hi Jens,

On 10/8/2026 5:15 PM, Jens Wiklander wrote:
> Hi Amir,
> 
> On Tue, Oct 6, 2026 at 2:39 AM Amirreza Zarrabi
> <amirreza.zarrabi@oss.qualcomm.com> wrote:
>>
>> This series adds an RPMI backend to the OP-TEE driver for RISC-V.
>> It builds on the separately posted RPMI TEE transport series [1],
>> which provides service discovery, synchronous calls, memory parcels
>> and asynchronous signals.
>>
>> This revision is effectively a full rewrite of the original RFC.
>> The generic RPMI transport has been separated from the OP-TEE backend,
>> and the backend has been reworked around the service bus and a new
>> OP-TEE control ABI.
>>
>> Unlike v1, the OP-TEE backend no longer manages SBI MPXY mailbox
>> channels or implements RPMI TEE service-group operations directly.
>> It binds to the OP-TEE service UUID on the RPMI TEE bus and uses the
>> transport's public operations. No additional OP-TEE device-tree node
>> is required.
>>
>> The backend reuses the common OP-TEE session, shared-memory, argument
>> cache, call queue and RPC infrastructure. Shared memory is represented
>> by RPMI parcel IDs and nonces. Yielding calls pass command and RPC
>> buffer ranges within a parcel and resume suspended execution using
>> an opaque token returned by OP-TEE.
>>
>> The OP-TEE control ABI uses fixed-width little-endian messages carried
>> in TEE_CALL payloads rather than reproducing FF-A's register-based
>> message layout. Asynchronous notifications use an allocated TEE-to-REE
>> signal as a bottom-half doorbell, while ordinary logical notifications
>> continue through the common RPC path.
>>
>> Matching OP-TEE OS support for this ABI has not yet been implemented.
>> This remains an RFC for review of the backend integration and the
>> Linux-to-OP-TEE protocol.
>>
>> [1] RPMI TEE transport dependency:
>> https://lore.kernel.org/op-tee/20260928-riscv-rpmi-tee-abi-v1-0-04908b81d885@oss.qualcomm.com/
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>> Changes in v2:
>> - Effectively rewrite the backend around the separately posted RPMI
>>   TEE transport and a new OP-TEE control ABI.
>> - Remove backend-owned per-hart mailbox channels and the OP-TEE-specific
>>   device-tree binding.
>> - Replace the register-like control payload with fixed-width messages,
>>   explicit command/RPC buffer ranges and opaque resume tokens.
>> - Add a parcel-reference parameter layout carrying the parcel ID,
>>   nonce, and 64-bit offset and size without changing parameter size.
>> - Make argument-offset support part of the baseline ABI and retain
>>   the common shared argument cache.
>> - Use a transport-allocated signal for asynchronous bottom-half
>>   notifications.
>> - Link to v1: https://lore.kernel.org/r/20260912-rpmi-tee-service-grp-dev-v1-0-1d1d35c2a859@oss.qualcomm.com
>>
>> ---
>> Amirreza Zarrabi (8):
>>       tee: optee: allow RPMI transport builds on RISC-V
>>       tee: optee: define the RPMI control and parcel-reference ABI
>>       tee: optee: add RPMI shared-memory and parameter support
>>       tee: optee: add RPMI dynamic shared-memory pool
>>       tee: optee: add RPMI RPC handling
>>       tee: optee: execute yielding RPMI calls
>>       tee: optee: bind RPMI services and negotiate backend capabilities
>>       tee: optee: support RPMI asynchronous notification doorbells
>>
>>  drivers/tee/Kconfig               |    3 +-
>>  drivers/tee/optee/Kconfig         |    3 +-
>>  drivers/tee/optee/Makefile        |    1 +
>>  drivers/tee/optee/call.c          |    2 +
>>  drivers/tee/optee/core.c          |   10 +-
>>  drivers/tee/optee/optee_msg.h     |   38 +-
>>  drivers/tee/optee/optee_private.h |   54 +-
>>  drivers/tee/optee/optee_rpmi.h    |  234 ++++++++
>>  drivers/tee/optee/rpmi_abi.c      | 1059 +++++++++++++++++++++++++++++++++++++
>>  9 files changed, 1389 insertions(+), 15 deletions(-)
> 
> The rpmi changes in the optee driver harmonize quite well with the
> rest of the driver, but there are two things I'd like fixed:
> - avoid <linux/cleanup.h> macros. They are distracting.
> - use rc for the trivial int ERRNO values.
> 

I'll make these changes.

> I would be great with a QEMU-based end-to-end prototype to demonstrate
> that the ABI works.
> 

The OP-TEE side implementation is nearly complete. I'll incorporate your
comments here and update the OP-TEE code accordingly. I haven't started
the OpenSBI changes yet. Getting a QEMU-based end-to-end prototype working
is my first priority before moving the series out of RFC.

Apologies for the obvious oversights and bugs in this version. So far,
I've only compile-tested the code; it still needs end-to-end validation.

> More comments to come in the individual patches.

Thanks, Jens, for the early comments and feedback. They are very helpful
in shaping the final code and guiding the direction of the implementation.

Best regards,
Amir

> 
> Cheers,
> Jens