| 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: <ea769342-4d0e-497b-bee9-ed761401b63c@linaro.org>
Date: Wed, 20 Aug 2025 12:48:55 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Mukesh Ojha <mukesh.ojha@....qualcomm.com>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>,
Vikash Garodia <quic_vgarodia@...cinc.com>,
Dikshita Agarwal <quic_dikshita@...cinc.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: Abhinav Kumar <abhinav.kumar@...ux.dev>, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH v2 02/11] soc: qcom: mdtloader: Add context aware
qcom_mdt_pas_load() helper
On 19/08/2025 17:54, Mukesh Ojha wrote:
> Currently, remoteproc and non-remoteproc subsystems use different
> variants of the MDT loader helper API, primarily due to the handling of
> the metadata context. Remoteproc subsystems retain this context until
> authentication and reset, while non-remoteproc subsystems (e.g., video,
> graphics) do not require it.
>
> Add context aware qcom_mdt_pas_load() function which uses context
> returned from qcom_scm_pas_ctx_init() and use it till subsystems
> is out of set. This will also help in unifying the API used by
> remoteproc and non-remoteproc subsystems drivers.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
> ---
> If this approach is preferred, will convert all subsystem drivers to use the
> same set of API's using context and completely get away with qcom_mdt_load()
>
> -Mukesh
>
> drivers/remoteproc/qcom_q6v5_pas.c | 53 ++++++++++++++---------------
> drivers/soc/qcom/mdt_loader.c | 26 ++++++++++----
> include/linux/soc/qcom/mdt_loader.h | 22 ++++++------
> 3 files changed, 56 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 55a7da801183..e376c0338576 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -115,8 +115,8 @@ struct qcom_pas {
> struct qcom_rproc_ssr ssr_subdev;
> struct qcom_sysmon *sysmon;
>
> - struct qcom_scm_pas_metadata pas_metadata;
> - struct qcom_scm_pas_metadata dtb_pas_metadata;
> + struct qcom_scm_pas_ctx *pas_ctx;
> + struct qcom_scm_pas_ctx *dtb_pas_ctx;
> };
>
> static void qcom_pas_segment_dump(struct rproc *rproc,
> @@ -209,9 +209,9 @@ static int qcom_pas_unprepare(struct rproc *rproc)
> * auth_and_reset() was successful, but in other cases clean it up
> * here.
> */
> - qcom_scm_pas_metadata_release(&pas->pas_metadata);
> + qcom_scm_pas_metadata_release(pas->pas_ctx->metadata);
> if (pas->dtb_pas_id)
> - qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
> + qcom_scm_pas_metadata_release(pas->dtb_pas_ctx->metadata);
>
> return 0;
> }
> @@ -235,15 +235,8 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
> return ret;
> }
>
> - ret = qcom_mdt_pas_init(pas->dev, pas->dtb_firmware, pas->dtb_firmware_name,
> - pas->dtb_pas_id, pas->dtb_mem_phys,
> - &pas->dtb_pas_metadata);
> - if (ret)
> - goto release_dtb_firmware;
> -
> - ret = qcom_mdt_load_no_init(pas->dev, pas->dtb_firmware, pas->dtb_firmware_name,
> - pas->dtb_mem_region, pas->dtb_mem_phys,
> - pas->dtb_mem_size, &pas->dtb_mem_reloc);
> + ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware, pas->dtb_firmware_name,
> + pas->dtb_mem_region, &pas->dtb_mem_reloc);
> if (ret)
> goto release_dtb_metadata;
> }
> @@ -251,9 +244,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
> return 0;
>
> release_dtb_metadata:
> - qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
> -
> -release_dtb_firmware:
> + qcom_scm_pas_metadata_release(pas->dtb_pas_ctx->metadata);
> release_firmware(pas->dtb_firmware);
>
> return ret;
> @@ -301,14 +292,8 @@ static int qcom_pas_start(struct rproc *rproc)
> }
> }
>
> - ret = qcom_mdt_pas_init(pas->dev, pas->firmware, rproc->firmware, pas->pas_id,
> - pas->mem_phys, &pas->pas_metadata);
> - if (ret)
> - goto disable_px_supply;
> -
> - ret = qcom_mdt_load_no_init(pas->dev, pas->firmware, rproc->firmware,
> - pas->mem_region, pas->mem_phys, pas->mem_size,
> - &pas->mem_reloc);
> + ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
> + pas->mem_region, &pas->dtb_mem_reloc);
> if (ret)
> goto release_pas_metadata;
>
> @@ -328,9 +313,9 @@ static int qcom_pas_start(struct rproc *rproc)
> goto release_pas_metadata;
> }
>
> - qcom_scm_pas_metadata_release(&pas->pas_metadata);
> + qcom_scm_pas_metadata_release(pas->pas_ctx->metadata);
> if (pas->dtb_pas_id)
> - qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
> + qcom_scm_pas_metadata_release(pas->dtb_pas_ctx->metadata);
>
> /* firmware is used to pass reference from qcom_pas_start(), drop it now */
> pas->firmware = NULL;
> @@ -338,9 +323,9 @@ static int qcom_pas_start(struct rproc *rproc)
> return 0;
>
> release_pas_metadata:
> - qcom_scm_pas_metadata_release(&pas->pas_metadata);
> + qcom_scm_pas_metadata_release(pas->pas_ctx->metadata);
> if (pas->dtb_pas_id)
> - qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
> + qcom_scm_pas_metadata_release(pas->dtb_pas_ctx->metadata);
> disable_px_supply:
> if (pas->px_supply)
> regulator_disable(pas->px_supply);
> @@ -774,6 +759,18 @@ static int qcom_pas_probe(struct platform_device *pdev)
> }
>
> qcom_add_ssr_subdev(rproc, &pas->ssr_subdev, desc->ssr_name);
> +
> + pas->pas_ctx = qcom_scm_pas_ctx_init(pas->dev, pas->pas_id, pas->mem_phys,
> + pas->mem_size, true);
> + if (!pas->pas_ctx)
> + goto remove_ssr_sysmon;
this function already returns -ENOMEM you don't set ret to any
particular value so if qcom_scm_pas_ctx_init() returned NULL, you would
exit your probe function here "error out" with returning ret = 0
Please ERR_PTR() in qcom_scm_pas_ctx_init() and return the error up the
call stack via your remove_ssr_sysmon jump label.
---
bod
Powered by blists - more mailing lists