linux-kernel - Re: [PATCH v2 02/11] soc: qcom: mdtloader: Add context aware qcom_mdt_pas

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0b4472cb-0c73-4eb8-a360-22b40aae44f5@linaro.org>
Date: Wed, 3 Sep 2025 16:03:05 +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>
> ---

I'm struggling with the logic here a little bit.

You take this function which calls qcom_mtd_load_no_init();

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

and then turn it into

> +int qcom_mdt_pas_load(struct qcom_scm_pas_ctx *ctx, const struct firmware *fw,
> +		      const char *firmware, void *mem_region, phys_addr_t *reloc_base)
> +{
> +	int ret;
> +
> +	ret = __qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->peripheral,
> +				  ctx->mem_phys, ctx->metadata);
> +	if (ret)
> +		return ret;
> +
> +	return __qcom_mdt_load(ctx->dev, fw, firmware, mem_region, ctx->mem_phys,
> +			       ctx->mem_size, reloc_base);

Surely you want to qcom_mdt_load_no_init ?

On current kernel
return __qcom_mdt_load(ctx->dev, fw, firmware, mem_region, ctx,
		       mem_phys, ctx->mem_size,
		       reloc_base, true);

but that's a functional change

> +}
> +EXPORT_SYMBOL_GPL(qcom_mdt_pas_load);
the no_init version of this looks like this:

int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
                           const char *firmware, int pas_id,
                           void *mem_region, phys_addr_t mem_phys,
                           size_t mem_size, phys_addr_t *reloc_base)
{
         return __qcom_mdt_load(dev, fw, firmware, pas_id, mem_region,
			       mem_phys, mem_size, reloc_base, false);
}
EXPORT_SYMBOL_GPL(qcom_mdt_load_no_init);

So is it really the intention of this patch to change the callsites 
where qcom_mdt_pas_load() away from the init version to the no_init 
version ?

Maybe its a symptom of patch collision but hmm, having a hard time 
cherry-picking this to test.

---
bod