Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 Aug 2022 12:22:01 -0700
From:   Guru Das Srinagesh <quic_gurus@quicinc.com>
To:     Rajendra Nayak <quic_rjendra@quicinc.com>
CC:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        "David Heidelberg" <david@ixit.cz>,
        Robert Marko <robimarko@gmail.com>,
        Elliot Berman <quic_eberman@quicinc.com>
Subject: Re: [PATCH v2 4/5] firmware: qcom: scm: Add wait-queue helper
 functions
Message-ID: <20220825192201.GA9699@quicinc.com>
References: <1658529438-9234-1-git-send-email-quic_gurus@quicinc.com>
 <1658529438-9234-5-git-send-email-quic_gurus@quicinc.com>
 <1f284b9c-257b-a127-55c0-e6cc8c07a9eb@quicinc.com>
 <20220811030022.GA18104@quicinc.com>
 <456915bf-b7ff-efaa-72aa-62fd05344270@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <456915bf-b7ff-efaa-72aa-62fd05344270@quicinc.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

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.