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:   Fri, 13 Jan 2023 14:16:20 -0800
From:   Guru Das Srinagesh <quic_gurus@quicinc.com>
To:     Sibi Sankar <quic_sibis@quicinc.com>
CC:     <andersson@kernel.org>, <srinivas.kandagatla@linaro.org>,
        <agross@kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <robh+dt@kernel.org>,
        <konrad.dybcio@somainline.org>, <robimarko@gmail.com>
Subject: Re: [PATCH V10 2/2] firmware: qcom: scm: Add wait-queue handling
 logic
Message-ID: <20230113221620.GA19768@quicinc.com>
References: <20230113161114.22607-1-quic_sibis@quicinc.com>
 <20230113161114.22607-3-quic_sibis@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230113161114.22607-3-quic_sibis@quicinc.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Jan 13 2023 21:41, Sibi Sankar wrote:
> From: Guru Das Srinagesh <quic_gurus@quicinc.com>
> 
> When the firmware (FW) supports multiple requests per VM, multiple requests
> from the same/different VM can reach the firmware at the same time. Since
> the firmware currently being used has limited resources, it guards them
> with a resource lock and puts requests on a wait-queue internally and
> signals to HLOS that it is doing so. It does this by returning a new return
> value in addition to success or error: SCM_WAITQ_SLEEP. A sleeping SCM call
> can be woken up by an interrupt that the FW raises.
> 
>   1) SCM_WAITQ_SLEEP:
> 
>   	When an SCM call receives this return value instead of success
>   	or error, FW has placed this call on a wait-queue and has signalled
> 	HLOS to put it to non-interruptible sleep.
> 
> 	Along with this return value, FW also passes to HLOS `wq_ctx` -
> 	a unique number (UID) identifying the wait-queue that it has put
> 	the call on, internally. This is to help HLOS with its own
> 	bookkeeping to wake this sleeping call later.
> 
> 	Additionally, FW also passes to HLOS `smc_call_ctx` - a UID
> 	identifying the SCM call thus being put to sleep. This is also
> 	for HLOS' bookkeeping to wake this call up later.
> 
> 	These two additional values are passed via the a1 and a2
> 	registers.
> 
> 	N.B.: The "ctx" in the above UID names = "context".
> 
> The handshake mechanism that HLOS uses to talk to FW about wait-queue
> operations involves two new SMC calls.
> 
>   1) get_wq_ctx():
> 
>     	Arguments: 	None
>     	Returns:	wq_ctx, flags, more_pending
> 
>     	Get the wait-queue context, and wake up either one or all of the
>     	sleeping SCM calls associated with that wait-queue.
> 
>     	Additionally, repeat this if there are more wait-queues that are
>     	ready to have their requests woken up (`more_pending`).
> 
>   2) wq_resume(smc_call_ctx):
> 
>   	Arguments:	smc_call_ctx
> 
>   	HLOS needs to issue this in response to receiving an
>   	IRQ, passing to FW the same smc_call_ctx that FW
>   	receives from HLOS via the get_wq_ctx() call.
> 
> (The mechanism to wake a SMC call back up is described in detail below)
> 
>  VM_1                     VM_2                            Firmware
>    │                        │                                 │
>    │                        │                                 │
>    │                        │                                 │
>    │                        │                                 │
>    │      REQUEST_1         │                                 │
>    ├────────────────────────┼─────────────────────────────────┤
>    │                        │                                 │
>    │                        │                              ┌──┼──┐
>    │                        │                              │  │  │
>    │                        │     REQUEST_2                │  │  │
>    │                        ├──────────────────────────────┼──┤  │
>    │                        │                              │  │  │Resource
>    │                        │                              │  │  │is busy
>    │                        │       {WQ_SLEEP}             │  │  │
>    │                        │◄─────────────────────────────┼──┤  │
>    │                        │  wq_ctx, smc_call_ctx        │  │  │
>    │                        │                              └──┼──┘
>    │   REQUEST_1 COMPLETE   │                                 │
>    │◄───────────────────────┼─────────────────────────────────┤
>    │                        │                                 │
>    │                        │         IRQ                     │
>    │                        │◄─-------------------------------│
>    │                        │                                 │
>    │                        │      get_wq_ctx()               │
>    │                        ├────────────────────────────────►│
>    │                        │                                 │
>    │                        │                                 │
>    │                        │◄────────────────────────────────┤
>    │                        │   wq_ctx, flags, and            │
>    │                        │        more_pending             │
>    │                        │                                 │
>    │                        │                                 │
>    │                        │ wq_resume(smc_call_ctx)         │
>    │                        ├────────────────────────────────►│
>    │                        │                                 │
>    │                        │                                 │
>    │                        │      REQUEST_2 COMPLETE         │
>    │                        │◄────────────────────────────────┤
>    │                        │                                 │
>    │                        │                                 │
> 
> With the exception of get_wq_ctx(), the other SMC call wq_resume() can
> return WQ_SLEEP (these nested rounds of WQ_SLEEP are not shown in the
> above diagram for the sake of simplicity). Therefore, introduce a new
> do-while loop to handle multiple WQ_SLEEP return values for the same
> parent SCM call.
> 
> Request Completion in the above diagram refers to either a success
> return value (zero) or error (and not SMC_WAITQ_SLEEP)
> 
> Also add the interrupt handler that wakes up a sleeping SCM call.
> 
> Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> Co-developed-by: Sibi Sankar <quic_sibis@quicinc.com>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>

Reviewed-by: Guru Das Srinagesh <quic_gurus@quicinc.com>

Thank you.

Guru Das.