| 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
| ||
|
Message-ID: <20251202125224.yh4o3p2i6kjfjxdz@hu-mojha-hyd.qualcomm.com>
Date: Tue, 2 Dec 2025 18:22:24 +0530
From: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
To: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org,
Unnathi Chalicheemala <unnathi.chalicheemala@....qualcomm.com>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v10 2/3] firmware: qcom_scm: Support multiple waitq
contexts
On Sun, Nov 30, 2025 at 08:11:03PM +0530, Shivendra Pratap wrote:
> From: Unnathi Chalicheemala <unnathi.chalicheemala@....qualcomm.com>
>
> Currently, only a single waitqueue context exists in the driver.
> 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. Enhance the driver to support multiple waitqueue when
> support is present in the firmware.
>
> When VMs make a SMC call, firmware allocates a waitqueue context,
> assuming the SMC call to be a blocking call. The SMC calls that cannot
> acquire resources, while execution in firmware, are returned to sleep
> in the calling VM. When the resource becomes available in the
> firmware, the VM gets notified to wake the sleeping thread and resume
> SMC call. The current qcom_scm driver supports single waitqueue as the
> old firmwares support only single waitqueue with waitqueue id zero.
> Multi-waitqueue mechanism is added in firmware starting SM8650 to
> support the case when multiple VMs make SMC calls or single VM making
> multiple calls on same CPU. To enable this support in qcom_scm driver,
> add support for handling multiple waitqueues. For instance, SM8650
> firmware can allocate two such waitq contexts, so the driver needs to
> implement two waitqueue contexts. For a generalized approach, the
> number of supported waitqueues can be queried from the firmware using
> a SMC call.
>
> Introduce qcom_scm_query_waitq_count to get the number of waitqueue
> contexts supported by the firmware and allocate “N” unique waitqueue
> contexts with a dynamic sized array where each unique wq_ctx is
> associated with a struct completion variable for easy lookup. Older
> targets which support only a single waitqueue, may return an error for
> qcom_scm_query_waitq_count, set the wq_cnt to one for such failures.
>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@....qualcomm.com>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
> ---
> drivers/firmware/qcom/qcom_scm.c | 74 ++++++++++++++++++++++++++++------------
> 1 file changed, 52 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 79ab1707f71b0157835deaea6309f33016e3de8c..ef3d81a5340512a79c252430db5f09cd8c253173 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_comps;
> 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 {
> @@ -2247,6 +2248,23 @@ static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 hwirq)
> 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;
nit: I know, it is not something we are following everywhere in this file.
Move the `ret` to here to follow xmas tree for newer functions
> +
> + 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(struct qcom_scm *scm)
> {
> struct device_node *parent_irq_node;
> @@ -2278,42 +2296,40 @@ static int qcom_scm_get_waitq_irq(struct qcom_scm *scm)
> return irq_create_fwspec_mapping(&fwspec);
> }
>
> -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_comps[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;
> }
> @@ -2389,6 +2405,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)
> @@ -2399,7 +2416,6 @@ static int qcom_scm_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - init_completion(&scm->waitq_comp);
> mutex_init(&scm->scm_bw_lock);
>
> scm->path = devm_of_icc_get(&pdev->dev, NULL);
> @@ -2451,6 +2467,20 @@ static int qcom_scm_probe(struct platform_device *pdev)
> return dev_err_probe(scm->dev, PTR_ERR(scm->mempool),
> "Failed to create the SCM memory pool\n");
>
> + ret = qcom_scm_query_waitq_count(scm);
> + if (ret < 0)
> + scm->wq_cnt = 1;
#define QCOM_SCM_DEFAULT_WAITQ_COUNT 1
> + else
> + scm->wq_cnt = ret;
Since, we are not erroring out, can we do this as
scm->wq_cnt = ret < 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret;
> +
> + scm->waitq_comps = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq_comps),
> + GFP_KERNEL);
> + if (!scm->waitq_comps)
> + return -ENOMEM;
> +
> + for (i = 0; i < scm->wq_cnt; i++)
> + init_completion(&scm->waitq_comps[i]);
> +
> irq = qcom_scm_get_waitq_irq(scm);
> if (irq < 0)
> irq = platform_get_irq_optional(pdev, 0);
Rest all looks good to me.
Reviewed-by: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
>
> --
> 2.34.1
>
--
-Mukesh Ojha
Powered by blists - more mailing lists