[V5,0/2] SCM: Add support for wait-queue aware firmware

This patch series enables the QCOM SCM driver to support firmware (FW) versions
that expect the high-level OS (HLOS) to be tolerant of SCM call requests not
being processed right away and, instead, being placed on a wait-queue in FW and
processed accordingly.

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):

1. HLOS makes an SMC call on core 5
2. The hypervisor scheduling interrupt interrupts this SMC call.
3. The hypervisor schedules the VM on core 5.
4. The VM makes an SMC call on core 5.
5. The SMC call is non-interruptibly stuck on FW spinlock on core 5.
6. HLOS cannot reschedule since core 5 is not responding to Reschedule IPIs.
7. Watchdog timer expires waiting for core 5.

This problem is solved by FW returning a new return code SCM_WAITQ_SLEEP to
HLOS right away when it is overwhelmed by the VM's SMC call. HLOS then places
the call on a wait-queue and wakes it up when it receives an interrupt that
signifies "all-clear".

There are two new SMC calls also being defined in this design that, together
with one new return code, form the handshake protocol between Linux and FW.

This design is also backwards-compatible with existing firmware versions that
do not support this feature.

v5:
- Pick up R-b
- Handle the wake_one/wake_all flags [Guru]
- Rename flag handler to qcom_scm_waitq_wakeup [Bjorn]
- Resume scm call can return ebusy as well handle that scenario by retrying
  the original smc call and not the resume call

v4:
- platform_set_drvdata will be used by __scm_smc_do_quirk_handle_waitq to
  get access to scm struct from device so retain it
- Use a single completion as it satisfies all of the current usecases [Bjorn]
- Inline scm_get_wq_ctx [Bjorn]
- Convert all pr_err to dev_err [Bjorn]
- Handle idr_destroy in a thread safe manner [Bjorn]
- Misc. Style fixes [Bjorn]
- Qualify bindings [Krzysztoff]

v3:
- Drop allow-multi-call property since HLOS doesn't completely support it
  yet.
- Fixup irq handling so as not to affect SoCs without the interrupt.
- Fix warnings reported by kernel test-bot.

v2:
- Changes made to patches 4 and 5 are listed therein.
- Rebased dt-bindings on top of the YAML conversion patch [1].

Older version of the series:
[v4] https://patchwork.kernel.org/project/linux-arm-msm/cover/20221114082644.28739-1-quic_sibis@quicinc.com/
[v3] https://patchwork.kernel.org/project/linux-arm-msm/cover/1666086406-5452-1-git-send-email-quic_sibis@quicinc.com/ 
[v2] https://patchwork.kernel.org/project/linux-arm-msm/cover/1661898311-30126-1-git-send-email-quic_gurus@quicinc.com/
[v1] https://patchwork.kernel.org/project/linux-arm-msm/cover/1656359076-13018-1-git-send-email-quic_gurus@quicinc.com/


Guru Das Srinagesh (2):
  dt-bindings: firmware: qcom-scm: Add optional interrupt
  firmware: qcom: scm: Add wait-queue handling logic

 .../bindings/firmware/qcom,scm.yaml           |   6 +
 drivers/firmware/qcom_scm-smc.c               |  97 +++++++++++++--
 drivers/firmware/qcom_scm.c                   | 115 +++++++++++++++++-
 drivers/firmware/qcom_scm.h                   |   9 ++
 4 files changed, 219 insertions(+), 8 deletions(-)

On 23/11/2022 21:46, 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.

Just two nitpicks while browsing the code...

>  static int qcom_scm_probe(struct platform_device *pdev)
>  {
>  	struct qcom_scm *scm;
>  	unsigned long clks;
> -	int ret;
> +	int irq, ret;
>  
>  	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
>  	if (!scm)
> @@ -1399,9 +1486,29 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	platform_set_drvdata(pdev, scm);
> +
>  	__scm = scm;
>  	__scm->dev = &pdev->dev;
>  
> +	spin_lock_init(&__scm->waitq.idr_lock);
> +	idr_init(&__scm->waitq.idr);
> +	init_completion(&__scm->waitq.waitq_comp);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		if (irq != -ENXIO)
> +			return irq;
> +	} else {
> +		ret = devm_request_threaded_irq(__scm->dev, irq, NULL, qcom_scm_irq_handler,
> +						IRQF_ONESHOT, "qcom-scm", __scm);
> +		if (ret < 0) {
> +			dev_err(scm->dev, "Failed to request qcom-scm irq: %d\n", ret);
> +			idr_destroy(&__scm->waitq.idr);
> +			return ret;

return dev_err_probe().

> +		}
> +	}
> +
>  	__get_convention();
>  
>  	/*
> @@ -1417,6 +1524,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  
>  static void qcom_scm_shutdown(struct platform_device *pdev)
>  {
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&__scm->waitq.idr_lock, flags);
> +	idr_destroy(&__scm->waitq.idr);
> +	spin_unlock_irqrestore(&__scm->waitq.idr_lock, flags);
> +
>  	/* Clean shutdown, disable download mode to allow normal restart */
>  	if (download_mode)
>  		qcom_scm_set_download_mode(false);
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index db3d08a01209..323cb49d4976 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -60,6 +60,10 @@ struct qcom_scm_res {
>  	u64 result[MAX_QCOM_SCM_RETS];
>  };
>  
> +struct qcom_scm;
> +extern struct completion *qcom_scm_lookup_wq(struct qcom_scm *scm, u32 wq_ctx);
> +extern int scm_get_wq_ctx(u32 *wq_ctx, u32 *flags, u32 *more_pending);

No need for externs for new entries.

Best regards,
Krzysztof