linux-kernel - Re: [PATCH 1/3] firmware: qcom_scm: Add support for tsens reinit workaround

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: <YtYdxlbLvgimN2MQ@builder.lan>
Date:   Mon, 18 Jul 2022 21:58:14 -0500
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Bhupesh Sharma <bhupesh.sharma@...aro.org>
Cc:     linux-pm@...r.kernel.org, bhupesh.linux@...il.com,
        linux-kernel@...r.kernel.org, Amit Kucheria <amitk@...nel.org>,
        Thara Gopinath <thara.gopinath@...il.com>,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 1/3] firmware: qcom_scm: Add support for tsens reinit
 workaround

On Fri 01 Jul 09:58 CDT 2022, Bhupesh Sharma wrote:

Please update $subject to match the most uses prefix for the qcom_scm
driver.

> Some versions of QCoM tsens controller might enter a

s/QCoM/Qualcomm/ please.

> 'bad state' while running stability tests causing sensor
> temperatures/interrupts status to be in an 'invalid' state.
> 
> It is recommended to re-initialize the tsens controller
> via trustzone (secure registers) using scm call(s) when that
> happens.
> 
> Add support for the same in the qcom_scm driver.
> 
> Cc: Amit Kucheria <amitk@...nel.org>
> Cc: Thara Gopinath <thara.gopinath@...il.com>
> Cc: linux-pm@...r.kernel.org
> Cc: linux-arm-msm@...r.kernel.org
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@...aro.org>
> ---
>  drivers/firmware/qcom_scm.c | 17 +++++++++++++++++
>  drivers/firmware/qcom_scm.h |  4 ++++
>  include/linux/qcom_scm.h    |  2 ++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 3163660fa8e2..0bc7cc466218 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -796,6 +796,23 @@ int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
>  }
>  EXPORT_SYMBOL(qcom_scm_mem_protect_video_var);
>  
> +int qcom_scm_tsens_reinit(int *tsens_ret)
> +{
> +	unsigned int ret;

qcom_scm_call() returns negative numbers on error, so this should be
signed.

> +	struct qcom_scm_desc desc = {

const?

> +		.svc = QCOM_SCM_SVC_TSENS,
> +		.cmd = QCOM_SCM_TSENS_INIT_ID,
> +	};
> +	struct qcom_scm_res res;
> +
> +	ret = qcom_scm_call(__scm->dev, &desc, &res);
> +	if (tsens_ret)
> +		*tsens_ret = res.result[0];

Most similar qcom_scm functions use negative return value for errors and
positive (including 0) values for the returned data.

Looking at patch 3, the only thing you seem to care about is tsens_ret
being 0 or not, so I do think you would be fine returning both using the
return value.

Regards,
Bjorn

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_tsens_reinit);
> +
>  static int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,
>  				 size_t mem_sz, phys_addr_t src, size_t src_sz,
>  				 phys_addr_t dest, size_t dest_sz)
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index 0d51eef2472f..495fa00230c7 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -94,6 +94,10 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
>  #define QCOM_SCM_PIL_PAS_IS_SUPPORTED	0x07
>  #define QCOM_SCM_PIL_PAS_MSS_RESET	0x0a
>  
> +/* TSENS Services and Function IDs */
> +#define QCOM_SCM_SVC_TSENS		0x1E
> +#define QCOM_SCM_TSENS_INIT_ID		0x5
> +
>  #define QCOM_SCM_SVC_IO			0x05
>  #define QCOM_SCM_IO_READ		0x01
>  #define QCOM_SCM_IO_WRITE		0x02
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index f8335644a01a..f8c9eb739df1 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -124,4 +124,6 @@ extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
>  extern int qcom_scm_lmh_profile_change(u32 profile_id);
>  extern bool qcom_scm_lmh_dcvsh_available(void);
>  
> +extern int qcom_scm_tsens_reinit(int *tsens_ret);
> +
>  #endif
> -- 
> 2.35.3
>