Re: [PATCH v2 2/2] firmware: arm_scmi: Augment SMC/HVC to allow optional interrupt

[Sudeep Holla]

On Fri, Nov 20, 2020 at 08:27:38AM -0500, Jim Quinlan wrote:
> On Fri, Nov 20, 2020 at 6:14 AM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> >
> > On Thu, Nov 19, 2020 at 01:34:18PM -0500, Jim Quinlan wrote:
> > > On Fri, Nov 13, 2020 at 10:12 AM Jim Quinlan <james.quinlan@xxxxxxxxxxxx> wrote:
> > > >
> > > > On Fri, Nov 13, 2020 at 9:36 AM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> > > > >
> > > > > On Fri, Nov 13, 2020 at 09:26:43AM -0500, Jim Quinlan wrote:
> > > > > > Hi, these are fast calls.  Regards, Jim
> > > > > >
> > > > > >
> > > > > > On Fri, Nov 13, 2020 at 4:47 AM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> > > > > > >
> > > > > > > On Thu, Nov 12, 2020 at 12:56:27PM -0500, Jim Quinlan wrote:
> > > > > > > > The SMC/HVC SCMI transport is modified to allow the completion of an SCMI
> > > > > > > > message to be indicated by an interrupt rather than the return of the smc
> > > > > > > > call.  This accommodates the existing behavior of the BrcmSTB SCMI
> > > > > > > > "platform" whose SW is already out in the field and cannot be changed.
> > > > > > > >
> > > > > > >
> > > > > > > Sorry for missing to check with you earlier. Are these not fast smc calls ?
> > > > > > > Can we check the SMC Function IDs for the same and expect IRQ to be present
> > > > > > > if they are not fast calls ?
> > > > > > Hi, if I understand you correctly you want to do something like this:
> > > > > >
> > > > > >  if (! ARM_SMCCC_IS_FAST_CALL(func_id)) {
> > > > > >         /* look for irq and request it */
> > > > > > }
> > > > > >
> > > > >
> > > > > Yes.
> > > > >
> > > > > > But we  do use fast calls.
> > > > >
> > > > > What was the rationale for retaining fast SMC calls but use IRQ for Tx
> > > > > completion ?
> > > > >
> > > > > Is it because you offload it to some other microprocessor and don't
> > > > > continue execution on secure side in whcih case you can afford fast call ?
> > > Hi Sudeep,
> > >
> >
> > Thanks for the details. Unfortunately more questions:
> >
> > > Here is my understanding:  Some SMC calls may take a few longer to
> > > complete than others. The longer ones tie up the CPU core that is
> > > handling the SMC call, and so nothing can be scheduled on that
> > > specific core.
> >
> > So far good.
> >
> > > Unfortunately, we have a real-time OS that runs
> > > sporadically on one specific core and if that happens to be the same
> > > core that is handling the SMC, the RTOS will miss its deadline.  So we
> > > need to have the SMC return immediately and use an SGI for task
> > > completion.
> > >
> >
> > So it sounds more like it can't be fast call then.
> Hi Sudeep,
>
> To be honest,  I'm not sure what the big difference between fast and
> slow SMC calls are other than the latter is "yielding" and
> interruptible.  We cannot tolerate them being interruptible.
>

OK

> >
> > Does that me, it will always return early and send SGI when the request
> > is complete ?
> Most calls send the SGI and return immediately.  The ones that take
> longer return from the SMC and send the SGI when the operation is
> completed.

That's relief.

> >
> > 1. If yes, what happens if there are multiple requests in parallel and
> >    second one completes before the first. Can we handle that with this
> >    patch set. Of will the second request fails until the first one is
> >    complete ? It extends to number of cpus in the system worst case.
>
> With SCMI we only have one message pending  at  a time;  perhaps I do
> not understand your question.  Having the SMC return is mostly a no-op
> as far as the SCMI  driver is concerned.
>

Yes we have lock, I forgot. There are requirements to make the smc atomic
by some vendors, was thinking about that and forgot about the lock and
how what I explained can never happen. Thanks for the patience.

If you ping and get Rob's ack on DT, I can take this patch along with
DT bindings for now as is. We can always enhance if required.

> Our SCMI messages cannot fail.  When we do have timeouts it indicates
> that something is wrong and needs to be fixed.
>

Good to know.

--
Regards,
Sudeep