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

[Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>]

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