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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220825192201.GA9699@quicinc.com>
Date:   Thu, 25 Aug 2022 12:22:01 -0700
From:   Guru Das Srinagesh <quic_gurus@...cinc.com>
To:     Rajendra Nayak <quic_rjendra@...cinc.com>
CC:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        "David Heidelberg" <david@...t.cz>,
        Robert Marko <robimarko@...il.com>,
        Elliot Berman <quic_eberman@...cinc.com>
Subject: Re: [PATCH v2 4/5] firmware: qcom: scm: Add wait-queue helper
 functions

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.