diff mbox series

[v9,2/8] remoteproc: qcom: Add secure PIL support

Message ID 20240621114659.2958170-3-quic_gokulsri@quicinc.com (mailing list archive)
State Not Applicable, archived
Headers show
Series remoteproc: qcom: q6v5-wcss: Add support for secure pil | expand

Commit Message

Gokul Sriram Palanisamy June 21, 2024, 11:46 a.m. UTC 
IPQ8074 uses secure PIL. Hence, adding the support for the same.

Signed-off-by: Nikhil Prakash V <quic_nprakash@quicinc.com>
Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 43 +++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

Comments

Dmitry Baryshkov June 21, 2024, 9:17 p.m. UTC | #1 
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

> 
> Signed-off-by: Nikhil Prakash V <quic_nprakash@quicinc.com>
> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.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

>  };
>  
>  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.

> +
>  	/* 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

>  	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.

> +
>  	/* 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
>
Gokul Sriram Palanisamy June 25, 2024, 5:36 a.m. UTC | #2 
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@quicinc.com>
>> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
>> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.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
>>
diff mbox series

Patch

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;
 
 	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;
 };
 
 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;
+	}
+
 	/* 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:
 	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;
+	}
+
 	/* 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 = {