| Message ID | 20250425-multi_waitq_scm-v6-2-cba8ca5a6d03@oss.qualcomm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SCM: Support latest version of waitq-aware firmware | expand |
On Fri, Apr 25, 2025 at 04:48:02PM -0700, Unnathi Chalicheemala wrote: > > +static int qcom_scm_query_waitq_count(struct qcom_scm *scm) > +{ > + int ret; > + struct qcom_scm_desc desc = { > + .svc = QCOM_SCM_SVC_WAITQ, > + .cmd = QCOM_SCM_WAITQ_GET_INFO, > + .owner = ARM_SMCCC_OWNER_SIP > + }; > + struct qcom_scm_res res; > + > + if (!__qcom_scm_is_call_available(scm->dev, QCOM_SCM_SVC_WAITQ, QCOM_SCM_WAITQ_GET_INFO)) { > + dev_info(scm->dev, "Multi-waitqueue support unavailable\n"); > + return 1; > + } I am testing this patch on SM8750 and found that we are returning from here, do you know why it is happening? The first patch in this series does not check if scm call is available or not and I see scm returns the hwirq properly. I have commented out this block and able to see waitq count as 2, which is inline with what would be overlayed in the device tree. > + > + ret = qcom_scm_call_atomic(scm->dev, &desc, &res); > + if (ret) > + return ret; > + > + return res.result[0] & GENMASK(7, 0); > +} > + Thanks, Pavan
On 5/9/2025 2:48 AM, Pavan Kondeti wrote: > On Fri, Apr 25, 2025 at 04:48:02PM -0700, Unnathi Chalicheemala wrote: >> >> +static int qcom_scm_query_waitq_count(struct qcom_scm *scm) >> +{ >> + int ret; >> + struct qcom_scm_desc desc = { >> + .svc = QCOM_SCM_SVC_WAITQ, >> + .cmd = QCOM_SCM_WAITQ_GET_INFO, >> + .owner = ARM_SMCCC_OWNER_SIP >> + }; >> + struct qcom_scm_res res; >> + >> + if (!__qcom_scm_is_call_available(scm->dev, QCOM_SCM_SVC_WAITQ, QCOM_SCM_WAITQ_GET_INFO)) { >> + dev_info(scm->dev, "Multi-waitqueue support unavailable\n"); >> + return 1; >> + } > > I am testing this patch on SM8750 and found that we are returning from > here, do you know why it is happening? The first patch in this series > does not check if scm call is available or not and I see scm returns the > hwirq properly. I have commented out this block and able to see waitq > count as 2, which is inline with what would be overlayed in the device > tree. > Thanks for testing it on SM8750, Pavan. Initially, I wanted to check whether the SCM call was available before invoking it, so we could distinguish between a failed SCM call and a failure due to the call being unavailable. However, after some internal discussions, we found that qcom_scm_is_call_available() is handled in TZ, which doesn’t implement WAITQ_GET_INFO, and therefore always returns 0. Given this, I’ll be removing the qcom_scm_is_call_available() check and will treat both scenarios - SCM call failure and call not implemented on the target - as a single case. >> + >> + ret = qcom_scm_call_atomic(scm->dev, &desc, &res); >> + if (ret) >> + return ret; >> + >> + return res.result[0] & GENMASK(7, 0); >> +} >> + > > Thanks, > Pavan
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 529e1d067b1901c4417a1f1fd9c3255ee31de532..9f8db13ef1ce14cc324fa9f0abf5c6a97ceb7b8b 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -47,7 +47,7 @@ struct qcom_scm { struct clk *iface_clk; struct clk *bus_clk; struct icc_path *path; - struct completion waitq_comp; + struct completion *waitq; struct reset_controller_dev reset; /* control access to the interconnect path */ @@ -57,6 +57,7 @@ struct qcom_scm { u64 dload_mode_addr; struct qcom_tzmem_pool *mempool; + unsigned int wq_cnt; }; struct qcom_scm_current_perm_info { @@ -2118,6 +2119,28 @@ static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 virq) return 0; } +static int qcom_scm_query_waitq_count(struct qcom_scm *scm) +{ + int ret; + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_WAITQ, + .cmd = QCOM_SCM_WAITQ_GET_INFO, + .owner = ARM_SMCCC_OWNER_SIP + }; + struct qcom_scm_res res; + + if (!__qcom_scm_is_call_available(scm->dev, QCOM_SCM_SVC_WAITQ, QCOM_SCM_WAITQ_GET_INFO)) { + dev_info(scm->dev, "Multi-waitqueue support unavailable\n"); + return 1; + } + + ret = qcom_scm_call_atomic(scm->dev, &desc, &res); + if (ret) + return ret; + + return res.result[0] & GENMASK(7, 0); +} + static int qcom_scm_get_waitq_irq(void) { int ret; @@ -2149,42 +2172,40 @@ static int qcom_scm_get_waitq_irq(void) return ret; } -static int qcom_scm_assert_valid_wq_ctx(u32 wq_ctx) +static struct completion *qcom_scm_get_completion(u32 wq_ctx) { - /* FW currently only supports a single wq_ctx (zero). - * TODO: Update this logic to include dynamic allocation and lookup of - * completion structs when FW supports more wq_ctx values. - */ - if (wq_ctx != 0) { - dev_err(__scm->dev, "Firmware unexpectedly passed non-zero wq_ctx\n"); - return -EINVAL; - } + struct completion *wq; - return 0; + if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt)) + return ERR_PTR(-EINVAL); + + wq = &__scm->waitq[wq_ctx]; + + return wq; } int qcom_scm_wait_for_wq_completion(u32 wq_ctx) { - int ret; + struct completion *wq; - ret = qcom_scm_assert_valid_wq_ctx(wq_ctx); - if (ret) - return ret; + wq = qcom_scm_get_completion(wq_ctx); + if (IS_ERR(wq)) + return PTR_ERR(wq); - wait_for_completion(&__scm->waitq_comp); + wait_for_completion(wq); return 0; } static int qcom_scm_waitq_wakeup(unsigned int wq_ctx) { - int ret; + struct completion *wq; - ret = qcom_scm_assert_valid_wq_ctx(wq_ctx); - if (ret) - return ret; + wq = qcom_scm_get_completion(wq_ctx); + if (IS_ERR(wq)) + return PTR_ERR(wq); - complete(&__scm->waitq_comp); + complete(wq); return 0; } @@ -2260,6 +2281,7 @@ static int qcom_scm_probe(struct platform_device *pdev) struct qcom_tzmem_pool_config pool_config; struct qcom_scm *scm; int irq, ret; + int i; scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL); if (!scm) @@ -2270,7 +2292,19 @@ static int qcom_scm_probe(struct platform_device *pdev) if (ret < 0) return ret; - init_completion(&scm->waitq_comp); + ret = qcom_scm_query_waitq_count(scm); + if (ret < 0) + return ret; + + scm->wq_cnt = ret; + + scm->waitq = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq), GFP_KERNEL); + if (!scm->waitq) + return -ENOMEM; + + for (i = 0; i < scm->wq_cnt; i++) + init_completion(&scm->waitq[i]); + mutex_init(&scm->scm_bw_lock); scm->path = devm_of_icc_get(&pdev->dev, NULL);
Currently, only a single waitqueue context exists, with waitqueue id zero. Multi-waitqueue mechanism is added in firmware to support the case when multiple VMs make SMC calls or single VM making multiple calls on same CPU. When VMs make SMC call, firmware will allocate waitqueue context assuming the SMC call to be a blocking call. SMC calls that cannot acquire resources are returned to sleep in the calling VM. When resource is available, VM will be notified to wake sleeping thread and resume SMC call. SM8650 firmware can allocate two such waitq contexts so create these two waitqueue contexts. Unique waitqueue contexts are supported by a dynamically sized array where each unique wq_ctx is associated with a struct completion variable for easy lookup. To get the number of waitqueue contexts directly from firmware, qcom_scm_query_waitq_count() is introduced. On older targets which support only a single waitqueue, wq_cnt is set to 1 as SCM call for query_waitq_count() is not implemented for single waitqueue case. Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com> --- drivers/firmware/qcom/qcom_scm.c | 78 ++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 22 deletions(-)