| 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
| ||
|
Date: Tue, 25 Jun 2024 11:06:37 +0530
From: Gokul Sriram P <quic_gokulsri@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <sboyd@...nel.org>, <andersson@...nel.org>, <bjorn.andersson@...aro.org>,
<david.brown@...aro.org>, <devicetree@...r.kernel.org>,
<jassisinghbrar@...il.com>, <linux-arm-msm@...r.kernel.org>,
<linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-remoteproc@...r.kernel.org>, <mark.rutland@....com>,
<mturquette@...libre.com>, <ohad@...ery.com>, <robh@...nel.org>,
<sricharan@...eaurora.org>, <gokulsri@...eaurora.org>
Subject: Re: [PATCH v9 2/8] remoteproc: qcom: Add secure PIL support
On 6/22/2024 2:47 AM, Dmitry Baryshkov wrote:
> On Fri, Jun 21, 2024 at 05:16:53PM GMT, Gokul Sriram Palanisamy wrote:
>> IPQ8074 uses secure PIL. Hence, adding the support for the same.
> See Documentation/process/submitting-patches.rst
Thanks. Will add detailed description.
>> Signed-off-by: Nikhil Prakash V <quic_nprakash@...cinc.com>
>> Signed-off-by: Sricharan R <quic_srichara@...cinc.com>
>> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@...cinc.com>
>> ---
>> drivers/remoteproc/qcom_q6v5_wcss.c | 43 +++++++++++++++++++++++++++--
>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
>> index 366b19cbd994..e45e79d80238 100644
>> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
>> @@ -18,6 +18,7 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/reset.h>
>> #include <linux/soc/qcom/mdt_loader.h>
>> +#include <linux/firmware/qcom/qcom_scm.h>
>> #include "qcom_common.h"
>> #include "qcom_pil_info.h"
>> #include "qcom_q6v5.h"
>> @@ -86,6 +87,9 @@
>> #define TCSR_WCSS_CLK_ENABLE 0x14
>>
>> #define MAX_HALT_REG 3
>> +
>> +#define WCNSS_PAS_ID 6
>> +
>> enum {
>> WCSS_IPQ8074,
>> WCSS_QCS404,
>> @@ -134,6 +138,7 @@ struct q6v5_wcss {
>> unsigned int crash_reason_smem;
>> u32 version;
>> bool requires_force_stop;
>> + bool need_mem_protection;
> needs
>
>>
>> struct qcom_rproc_glink glink_subdev;
>> struct qcom_rproc_ssr ssr_subdev;
>> @@ -152,6 +157,7 @@ struct wcss_data {
>> int ssctl_id;
>> const struct rproc_ops *ops;
>> bool requires_force_stop;
>> + bool need_mem_protection;
> needs
Thanks. Will update.
>> };
>>
>> static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
>> @@ -251,6 +257,15 @@ static int q6v5_wcss_start(struct rproc *rproc)
>>
>> qcom_q6v5_prepare(&wcss->q6v5);
>>
>> + if (wcss->need_mem_protection) {
>> + ret = qcom_scm_pas_auth_and_reset(WCNSS_PAS_ID);
>> + if (ret) {
>> + dev_err(wcss->dev, "wcss_reset failed\n");
>> + return ret;
>> + }
>> + goto wait_for_reset;
>> + }
> Use if/else instead of a goto.
sure. Will update.
>
>> +
>> /* Release Q6 and WCSS reset */
>> ret = reset_control_deassert(wcss->wcss_reset);
>> if (ret) {
>> @@ -285,6 +300,7 @@ static int q6v5_wcss_start(struct rproc *rproc)
>> if (ret)
>> goto wcss_q6_reset;
>>
>> +wait_for_reset:
> This is more like wait_for_start
yes, this looks more appropriate. Will update. Thanks.
>> ret = qcom_q6v5_wait_for_start(&wcss->q6v5, 5 * HZ);
>> if (ret == -ETIMEDOUT)
>> dev_err(wcss->dev, "start timed out\n");
>> @@ -718,6 +734,15 @@ static int q6v5_wcss_stop(struct rproc *rproc)
>> struct q6v5_wcss *wcss = rproc->priv;
>> int ret;
>>
>> + if (wcss->need_mem_protection) {
>> + ret = qcom_scm_pas_shutdown(WCNSS_PAS_ID);
>> + if (ret) {
>> + dev_err(wcss->dev, "not able to shutdown\n");
>> + return ret;
>> + }
>> + goto pas_done;
>> + }
> if/else. Or abstract this to functions.
sure. Will update appropriately.
>> +
>> /* WCSS powerdown */
>> if (wcss->requires_force_stop) {
>> ret = qcom_q6v5_request_stop(&wcss->q6v5, NULL);
>> @@ -742,6 +767,7 @@ static int q6v5_wcss_stop(struct rproc *rproc)
>> return ret;
>> }
>>
>> +pas_done:
>> clk_disable_unprepare(wcss->prng_clk);
>> qcom_q6v5_unprepare(&wcss->q6v5);
>>
>> @@ -765,9 +791,15 @@ static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
>> struct q6v5_wcss *wcss = rproc->priv;
>> int ret;
>>
>> - ret = qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
>> - 0, wcss->mem_region, wcss->mem_phys,
>> - wcss->mem_size, &wcss->mem_reloc);
>> + if (wcss->need_mem_protection)
>> + ret = qcom_mdt_load(wcss->dev, fw, rproc->firmware,
>> + WCNSS_PAS_ID, wcss->mem_region,
>> + wcss->mem_phys, wcss->mem_size,
>> + &wcss->mem_reloc);
>> + else
>> + ret = qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
>> + 0, wcss->mem_region, wcss->mem_phys,
>> + wcss->mem_size, &wcss->mem_reloc);
>> if (ret)
>> return ret;
>>
>> @@ -1035,6 +1067,9 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
>> if (!desc)
>> return -EINVAL;
>>
>> + if (desc->need_mem_protection && !qcom_scm_is_available())
>> + return -EPROBE_DEFER;
>> +
>> rproc = devm_rproc_alloc(&pdev->dev, pdev->name, desc->ops,
>> desc->firmware_name, sizeof(*wcss));
>> if (!rproc) {
>> @@ -1048,6 +1083,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
>>
>> wcss->version = desc->version;
>> wcss->requires_force_stop = desc->requires_force_stop;
>> + wcss->need_mem_protection = desc->need_mem_protection;
>>
>> ret = q6v5_wcss_init_mmio(wcss, pdev);
>> if (ret)
>> @@ -1111,6 +1147,7 @@ static const struct wcss_data wcss_ipq8074_res_init = {
>> .wcss_q6_reset_required = true,
>> .ops = &q6v5_wcss_ipq8074_ops,
>> .requires_force_stop = true,
>> + .need_mem_protection = true,
>> };
>>
>> static const struct wcss_data wcss_qcs404_res_init = {
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists