On Thu, 26 Oct 2023 at 19:57, Jens Wiklander jens.wiklander@linaro.org wrote:
On Thu, Oct 26, 2023 at 1:33 PM Sumit Garg sumit.garg@linaro.org wrote:
On Thu, 26 Oct 2023 at 16:06, Jens Wiklander jens.wiklander@linaro.org wrote:
On Thu, Oct 26, 2023 at 11:36 AM Sumit Garg sumit.garg@linaro.org wrote:
On Fri, 20 Oct 2023 at 19:29, Balint Dobszay balint.dobszay@arm.com wrote:
On 19 Oct 2023, at 16:16, Jens Wiklander wrote:
On Tue, Oct 17, 2023 at 1:14 PM Sumit Garg sumit.garg@linaro.org wrote: > On Mon, 16 Oct 2023 at 13:27, Jens Wiklander jens.wiklander@linaro.org wrote: >> On Fri, Oct 13, 2023 at 1:38 PM Sumit Garg sumit.garg@linaro.org wrote: >>> On Tue, 10 Oct 2023 at 21:11, Balint Dobszay balint.dobszay@arm.com wrote: >>>> On 3 Oct 2023, at 17:42, Sumit Garg wrote: >>>>> On Wed, 27 Sept 2023 at 20:56, Balint Dobszay balint.dobszay@arm.com wrote:
[snip]
>>>>>> +static int tstee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg, >>>>>> + struct tee_param *param) >>>>>> +{ >>>>>> + struct tstee *tstee = tee_get_drvdata(ctx->teedev); >>>>>> + struct ffa_device *ffa_dev = tstee->ffa_dev; >>>>>> + struct ts_context_data *ctxdata = ctx->data; >>>>>> + struct ffa_send_direct_data ffa_data; >>>>>> + struct tee_shm *shm = NULL; >>>>>> + struct ts_session *sess; >>>>>> + u32 req_len, ffa_args[5] = {}; >>>>>> + int shm_id, rc; >>>>>> + u8 iface_id; >>>>>> + u64 handle; >>>>>> + u16 opcode; >>>>>> + >>>>>> + mutex_lock(&ctxdata->mutex); >>>>>> + sess = find_session(ctxdata, arg->session); >>>>>> + >>>>>> + /* Do this while holding the mutex to make sure that the session wasn't closed meanwhile */ >>>>>> + if (sess) >>>>>> + iface_id = sess->iface_id; >>>>>> + >>>>>> + mutex_unlock(&ctxdata->mutex); >>>>>> + if (!sess) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + opcode = lower_16_bits(arg->func); >>>>>> + shm_id = lower_32_bits(param[0].u.value.a); >>>>>> + req_len = lower_32_bits(param[0].u.value.b); >>>>>> + >>>>>> + if (shm_id != 0) { >>>>>> + shm = tee_shm_get_from_id(ctx, shm_id); >>>>>> + if (IS_ERR(shm)) >>>>>> + return PTR_ERR(shm); >>>>>> + >>>>>> + if (shm->size < req_len) { >>>>>> + pr_err("request doesn't fit into shared memory buffer\n"); >>>>>> + rc = -EINVAL; >>>>>> + goto out; >>>>>> + } >>>>>> + >>>>>> + handle = shm->sec_world_id; >>>>>> + } else { >>>>>> + handle = FFA_INVALID_MEM_HANDLE; >>>>>> + } >>>>>> + >>>>>> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id, opcode); >>>>>> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle); >>>>>> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle); >>>>>> + ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len; >>>>>> + ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0; >>>>>> + >>>>>> + arg_list_to_ffa_data(ffa_args, &ffa_data); >>>>>> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data); >>>>> >>>>> I haven't dug deeper into the ABI yet, which is something I will look >>>>> into. But these RPC commands caught my attention. Are these RPC calls >>>>> blocking in nature? Is there a possibility that these could cause CPU >>>>> stalls? Do the Linux interrupts remain unhandled until the RPC calls >>>>> return? >>>> >>>> Yes, that is correct. We did encounter CPU stalls indeed, our solution >>>> was to enable preemption of S-EL0 SPs in OP-TEE [3] which solved the >>>> issue. >>> >>> I would have preferred to unite FFA_INTERRUPT and >>> OPTEE_FFA_YIELDING_CALL_RETURN_INTERRUPT since underneath both are >>> using FFA ABI. >>> >>> Jens, >>> >>> Can we change OP-TEE to use FFA_INTERRUPT as well when using FFA ABI? >> >> No, OP-TEE uses managed exit. Among other advantages, it allows >> resuming execution on a different CPU. >> > > I suppose that should be the case with FFA_INTERRUPT too. OP-TEE > should be able to resume SPs on different CPUs as well, right?
Possibly, but I leave that to Balint and company to sort out if that's desired or not.
FF-A mandates that S-EL0 SPs have a single execution context, run only on a single PE in the system at any point of time and are capable of migrating. Also, FF-A allows resuming a S-EL0 SP on a different CPU after it gets preempted by a NS interrupt. I think OP-TEE as S-EL1 SPMC does support this, but I don't have a setup yet that would explicitly test this scenario.
You can try to add a few minutes loop within a secure partition and see if the Linux scheduler reschedules on a different CPUs. I suppose you need to keep the system loaded with other normal world apps too.
Managed exit is only available for S-EL1 SPs.
So does that mean OP-TEE can use FF-A constructs like (FFA_INTERRUPT) for managed exit instead of custom OPTEE_FFA_YIELDING_CALL_RETURN_INTERRUPT?
No, and to be clear OPTEE_FFA_YIELDING_CALL_RETURN_INTERRUPT isn't a custom solution, it's fully within the specification.
I am not talking about it being out of specification. It's rather that if the base FF-A layer provides you with a feature then we shouldn't need to reimplement in every SP (OP-TEE or other trusted OS) communication stack.
Let's take a look at the code we have in the OP-TEE driver for this: case OPTEE_FFA_YIELDING_CALL_RETURN_INTERRUPT: /* Interrupt delivered by now */ break;
The implementation is pretty small. We're of course piggy-backing on the rest of the RPC code, but that would still be needed even if the FF-A framework could handle the managed exits. From OP-TEE's point of view, there's nothing to gain.
Returning via FFA_INTERRUPT is a simplified model where the SP has less control over the CPU.
I would like to understand which exact bits you are referring to. From Linux point of view, I don't see any difference.
Given how the ABI is defined we can't use FFA_INTERRUPT for managed exit.
Okay I see that ABI issue. We have to be backwards compatible and that will cause more pain rather than reusing FFA_INTERRUPT here. I think I can live with that in mind.
-Sumit
There's a vCPU field that is used by the SPMC, there's nothing for OP-TEE to suspend/resume a thread to let another thread use the CPU.
I imagine that it wouldn't play very nicely with spinlocks.
Aren't interrupts disabled while spinlock is being held?
Yes, but without managed exit that doesn't matter much since suspend/resume are transparent to the SP.
Cheers, Jens
-Sumit
Cheers, Jens
-Sumit