linux-kernel - Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Shutdown lite ADSP DTB on X1E

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: <ygygwhw4whioqdxp6otiarmcmzhm7mblaq2lae2752323lqysy@yit6cg5fe6sz>
Date: Tue, 19 Aug 2025 14:43:51 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Sibi Sankar <quic_sibis@...cinc.com>, Abel Vesa <abel.vesa@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
        linux-kernel@...r.kernel.org, Konrad Dybcio <konradybcio@...nel.org>
Subject: Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Shutdown lite ADSP DTB on
 X1E

On Tue, Aug 19, 2025 at 01:08:04PM +0200, Stephan Gerhold wrote:
> The ADSP firmware on X1E has separate firmware binaries for the main
> firmware and the DTB. The same applies for the "lite" firmware loaded by
> the boot firmware.
> 
> When preparing to load the new ADSP firmware we shutdown the lite_pas_id
> for the main firmware, but we don't shutdown the corresponding lite pas_id
> for the DTB. The fact that we're leaving it "running" forever becomes
> obvious if you try to reuse (or just access) the memory region used by the
> "lite" firmware: The &adsp_boot_mem is accessible, but accessing the
> &adsp_boot_dtb_mem results in a crash.
> 
> We don't support reusing the memory regions currently, but nevertheless we
> should not keep part of the lite firmware running. Fix this by adding the
> lite_dtb_pas_id and shutting it down as well.
> 
> We don't have a way to detect if the lite firmware is actually running yet,
> so ignore the return status of qcom_scm_pas_shutdown() for now. This was
> already the case before, the assignment to "ret" was not used anywhere.
> 
> Fixes: 62210f7509e1 ("remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP")
> Signed-off-by: Stephan Gerhold <stephan.gerhold@...aro.org>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 02e29171cbbee2d305827365ef7d2241b6eb786b..6faedae8d32ef6c3c2071975f2f1e37a9ffd8abe 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -42,6 +42,7 @@ struct qcom_pas_data {
>  	int pas_id;
>  	int dtb_pas_id;
>  	int lite_pas_id;
> +	int lite_dtb_pas_id;
>  	unsigned int minidump_id;
>  	bool auto_boot;
>  	bool decrypt_shutdown;
> @@ -80,6 +81,7 @@ struct qcom_pas {
>  	int pas_id;
>  	int dtb_pas_id;
>  	int lite_pas_id;
> +	int lite_dtb_pas_id;
>  	unsigned int minidump_id;
>  	int crash_reason_smem;
>  	unsigned int smem_host_id;
> @@ -225,7 +227,9 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
>  	pas->firmware = fw;
>  
>  	if (pas->lite_pas_id)
> -		ret = qcom_scm_pas_shutdown(pas->lite_pas_id);
> +		qcom_scm_pas_shutdown(pas->lite_pas_id);

Unrelated change. With it being split to a separate commit:


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>



> +	if (pas->lite_dtb_pas_id)
> +		qcom_scm_pas_shutdown(pas->lite_dtb_pas_id);
>  
>  	if (pas->dtb_pas_id) {
>  		ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
> @@ -722,6 +726,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
>  	pas->minidump_id = desc->minidump_id;
>  	pas->pas_id = desc->pas_id;
>  	pas->lite_pas_id = desc->lite_pas_id;
> +	pas->lite_dtb_pas_id = desc->lite_dtb_pas_id;
>  	pas->info_name = desc->sysmon_name;
>  	pas->smem_host_id = desc->smem_host_id;
>  	pas->decrypt_shutdown = desc->decrypt_shutdown;
> @@ -1085,6 +1090,7 @@ static const struct qcom_pas_data x1e80100_adsp_resource = {
>  	.pas_id = 1,
>  	.dtb_pas_id = 0x24,
>  	.lite_pas_id = 0x1f,
> +	.lite_dtb_pas_id = 0x29,
>  	.minidump_id = 5,
>  	.auto_boot = true,
>  	.proxy_pd_names = (char*[]){
> 
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry