Re: [PATCH v2 4/5] firmware: qcom: scm: Add wait-queue helper functions

[Guru Das Srinagesh]

On Aug 11 2022 11:13, Rajendra Nayak wrote:
> 
> On 8/11/2022 8:30 AM, Guru Das Srinagesh wrote:
> >On Aug 02 2022 17:07, Rajendra Nayak wrote:
> >>
> >>On 7/23/2022 4:07 AM, Guru Das Srinagesh wrote:
> >>>When the firmware (FW) supports multiple requests per VM, and the VM
> >>>also supports it via the `allow-multi-call` device tree flag, the
> >>>floodgates are thrown open for them to all reach the firmware at the
> >>>same time.
> >
> >[...]
> >
> >>>   2) SCM_WAITQ_WAKE:
> >>>
> >>>   	When an SCM call receives this return value instead of success
> >>>   	or error, FW wishes to signal HLOS to wake up a (different)
> >>>   	previously sleeping call.
> >>>
> >>>   	FW tells HLOS which call to wake up via the additional return
> >>>   	values `wq_ctx`, `smc_call_ctx` and `flags`. The first two have
> >>>   	already been explained above.
> >>>
> >>>   	`flags` can be either WAKE_ONE or WAKE_ALL. Meaning, wake either
> >>>   	one, or all, of the SCM calls that HLOS is associating with the
> >>>   	given `wq_ctx`.
> >>>
> >>>A sleeping SCM call can be woken up by either an interrupt that FW
> >>>raises, or via a SCM_WAITQ_WAKE return value for a new SCM call.
> >>
> >>Do you know why the FW was not designed to always use an interrupt?
> >>That would have made the handling of this in kernel a lot less complicated.
> >
> >Because:
> >
> >1. Our firmware in TrustZone cannot raise interrupts on its own - it needs the
> >hypervisor to do that.
> >
> >2. Thus, in platforms where there is no hypervisor, there is no interrupt
> >possible - only SMC_WAITQ_WAKE.
> >
> >Therefore, relying only on an interrupt would render the driver unable to
> >support platforms without a hypervisor, which we didn't want to do.
> 
> Thanks Guru for the clarification, however what problem are we really solving
> with this on platforms _without_ a hypervisor?
> 
> Your cover letter said
> 'The problem this feature is fixing is as follows. In a scenario where there is
> a VM in addition to HLOS (and an underlying hypervisor):'
> 
> So I assumed this was primarily for platforms _with_ a VM/Hypervisor?
> 
> I understand that even with just the HLOS and no VM, if we can get these requests
> processed concurrently it still adds value, but eventually Trustzone will
> still process these requests sequentially right?

The Trustzone (TZ) firmware doesn't process all requests sequentially - there are a
few that require a "callback" back to HLOS. In such cases, the original SCM
call releases the TZ serialization lock, thereby allowing a new call to enter
TZ. It is better to have requests sleep and wake via WAITQ_SLEEP and WAITQ_WAKE
rather than the alternative - retrying an arbitrary amount of times via -EBUSY.