[PATCH 1/2 V2] remoteproc: core: Export the rproc coredump APIs

[PATCH 1/2 V2] remoteproc: core: Export the rproc coredump APIs

[Yogesh Lal]

From: Siddharth Gupta <sidgup@codeaurora.org>

The remoteproc coredump APIs are currently only part of the internal
remoteproc header. This prevents the remoteproc platform drivers from
using these APIs when needed. This change moves the rproc_coredump()
and rproc_coredump_cleanup() APIs to the linux header and marks them
as exported symbols.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
---
 drivers/remoteproc/remoteproc_coredump.c | 2 ++
 drivers/remoteproc/remoteproc_internal.h | 4 ----
 include/linux/remoteproc.h               | 4 ++++
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_coredump.c b/drivers/remoteproc/remoteproc_coredump.c
index aee657c..aa45b68 100644
--- a/drivers/remoteproc/remoteproc_coredump.c
+++ b/drivers/remoteproc/remoteproc_coredump.c
@@ -32,6 +32,7 @@ void rproc_coredump_cleanup(struct rproc *rproc)
 		kfree(entry);
 	}
 }
+EXPORT_SYMBOL(rproc_coredump_cleanup);
 
 /**
  * rproc_coredump_add_segment() - add segment of device memory to coredump
@@ -327,6 +328,7 @@ void rproc_coredump(struct rproc *rproc)
 	 */
 	wait_for_completion(&dump_state.dump_done);
 }
+EXPORT_SYMBOL(rproc_coredump);
 
 /**
  * rproc_coredump_using_sections() - perform coredump using section headers
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index a328e63..a492a41 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -49,10 +49,6 @@ extern struct class rproc_class;
 int rproc_init_sysfs(void);
 void rproc_exit_sysfs(void);
 
-/* from remoteproc_coredump.c */
-void rproc_coredump_cleanup(struct rproc *rproc);
-void rproc_coredump(struct rproc *rproc);
-
 #ifdef CONFIG_REMOTEPROC_CDEV
 void rproc_init_cdev(void);
 void rproc_exit_cdev(void);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 83c09ac..b8c8c3a 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -673,6 +673,10 @@ void rproc_shutdown(struct rproc *rproc);
 int rproc_detach(struct rproc *rproc);
 int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
 void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
+
+/* from remoteproc_coredump.c */
+void rproc_coredump_cleanup(struct rproc *rproc);
+void rproc_coredump(struct rproc *rproc);
 void rproc_coredump_using_sections(struct rproc *rproc);
 int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
 int rproc_coredump_add_custom_segment(struct rproc *rproc,
-- 
2.7.4

[PATCH 2/2 V2] remoteproc: qcom: Add full coredump fallback mechanism

[Yogesh Lal]

From: Siddharth Gupta <sidgup@codeaurora.org>

If a remoteproc's firmware does not support minidump but the driver
adds an ID, the minidump driver does not collect any coredumps when
the remoteproc crashes. This hinders the purpose of coredump
collection. This change adds a fallback mechanism in the event of a
crash.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
---
 drivers/remoteproc/qcom_common.c   | 7 +++++--
 drivers/remoteproc/qcom_q6v5_pas.c | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4b91e3c..b3fdc66 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -163,8 +163,11 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
 	 */
 	if (subsystem->regions_baseptr == 0 ||
 	    le32_to_cpu(subsystem->status) != 1 ||
-	    le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED ||
-	    le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
+	    le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED) {
+		return rproc_coredump(rproc);
+	}
+
+	if (le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
 		dev_err(&rproc->dev, "Minidump not ready, skipping\n");
 		return;
 	}
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 401b1ec..6e5cbca 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -274,6 +274,7 @@ static const struct rproc_ops adsp_minidump_ops = {
 	.start = adsp_start,
 	.stop = adsp_stop,
 	.da_to_va = adsp_da_to_va,
+	.parse_fw = qcom_register_dump_segments,
 	.load = adsp_load,
 	.panic = adsp_panic,
 	.coredump = adsp_minidump,
-- 
2.7.4

Re: [PATCH 1/2 V2] remoteproc: core: Export the rproc coredump APIs

[Sibi Sankar]

On 5/10/22 4:55 PM, Yogesh Lal wrote:
> From: Siddharth Gupta <sidgup@codeaurora.org>
> 
> The remoteproc coredump APIs are currently only part of the internal
> remoteproc header. This prevents the remoteproc platform drivers from
> using these APIs when needed. This change moves the rproc_coredump()
> and rproc_coredump_cleanup() APIs to the linux header and marks them
> as exported symbols.
> 
> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>

> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
> ---
>   drivers/remoteproc/remoteproc_coredump.c | 2 ++
>   drivers/remoteproc/remoteproc_internal.h | 4 ----
>   include/linux/remoteproc.h               | 4 ++++
>   3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_coredump.c b/drivers/remoteproc/remoteproc_coredump.c
> index aee657c..aa45b68 100644
> --- a/drivers/remoteproc/remoteproc_coredump.c
> +++ b/drivers/remoteproc/remoteproc_coredump.c
> @@ -32,6 +32,7 @@ void rproc_coredump_cleanup(struct rproc *rproc)
>   		kfree(entry);
>   	}
>   }
> +EXPORT_SYMBOL(rproc_coredump_cleanup);
>   
>   /**
>    * rproc_coredump_add_segment() - add segment of device memory to coredump
> @@ -327,6 +328,7 @@ void rproc_coredump(struct rproc *rproc)
>   	 */
>   	wait_for_completion(&dump_state.dump_done);
>   }
> +EXPORT_SYMBOL(rproc_coredump);
>   
>   /**
>    * rproc_coredump_using_sections() - perform coredump using section headers
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index a328e63..a492a41 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -49,10 +49,6 @@ extern struct class rproc_class;
>   int rproc_init_sysfs(void);
>   void rproc_exit_sysfs(void);
>   
> -/* from remoteproc_coredump.c */
> -void rproc_coredump_cleanup(struct rproc *rproc);
> -void rproc_coredump(struct rproc *rproc);
> -
>   #ifdef CONFIG_REMOTEPROC_CDEV
>   void rproc_init_cdev(void);
>   void rproc_exit_cdev(void);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 83c09ac..b8c8c3a 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -673,6 +673,10 @@ void rproc_shutdown(struct rproc *rproc);
>   int rproc_detach(struct rproc *rproc);
>   int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
>   void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
> +
> +/* from remoteproc_coredump.c */
> +void rproc_coredump_cleanup(struct rproc *rproc);
> +void rproc_coredump(struct rproc *rproc);
>   void rproc_coredump_using_sections(struct rproc *rproc);
>   int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
>   int rproc_coredump_add_custom_segment(struct rproc *rproc,
> 

Re: [PATCH 2/2 V2] remoteproc: qcom: Add full coredump fallback mechanism

[Sibi Sankar]

Hey Yogesh,
Looks like you missed adding the patch that uses the exported
rproc_cleanup api.


On 5/10/22 4:55 PM, Yogesh Lal wrote:
> From: Siddharth Gupta <sidgup@codeaurora.org>
> 
> If a remoteproc's firmware does not support minidump but the driver
> adds an ID, the minidump driver does not collect any coredumps when
> the remoteproc crashes. This hinders the purpose of coredump
> collection. This change adds a fallback mechanism in the event of a
> crash.
> 

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>

> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
> ---
>   drivers/remoteproc/qcom_common.c   | 7 +++++--
>   drivers/remoteproc/qcom_q6v5_pas.c | 1 +
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 4b91e3c..b3fdc66 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -163,8 +163,11 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
>   	 */
>   	if (subsystem->regions_baseptr == 0 ||
>   	    le32_to_cpu(subsystem->status) != 1 ||
> -	    le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED ||
> -	    le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
> +	    le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED) {
> +		return rproc_coredump(rproc);
> +	}
> +
> +	if (le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
>   		dev_err(&rproc->dev, "Minidump not ready, skipping\n");
>   		return;
>   	}
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 401b1ec..6e5cbca 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -274,6 +274,7 @@ static const struct rproc_ops adsp_minidump_ops = {
>   	.start = adsp_start,
>   	.stop = adsp_stop,
>   	.da_to_va = adsp_da_to_va,
> +	.parse_fw = qcom_register_dump_segments,
>   	.load = adsp_load,
>   	.panic = adsp_panic,
>   	.coredump = adsp_minidump,
> 

Re: [PATCH 2/2 V2] remoteproc: qcom: Add full coredump fallback mechanism

[Sibi Sankar]

On 5/21/22 11:44 AM, Sibi Sankar wrote:
> Hey Yogesh,
> Looks like you missed adding the patch that uses the exported
> rproc_cleanup api.
> 
> 
> On 5/10/22 4:55 PM, Yogesh Lal wrote:
>> From: Siddharth Gupta <sidgup@codeaurora.org>
>>
>> If a remoteproc's firmware does not support minidump but the driver
>> adds an ID, the minidump driver does not collect any coredumps when
>> the remoteproc crashes. This hinders the purpose of coredump
>> collection. This change adds a fallback mechanism in the event of a
>> crash.
>>
> 
> Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>
> 
>> Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
>> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
>> ---
>>   drivers/remoteproc/qcom_common.c   | 7 +++++--
>>   drivers/remoteproc/qcom_q6v5_pas.c | 1 +
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_common.c 
>> b/drivers/remoteproc/qcom_common.c
>> index 4b91e3c..b3fdc66 100644
>> --- a/drivers/remoteproc/qcom_common.c
>> +++ b/drivers/remoteproc/qcom_common.c
>> @@ -163,8 +163,11 @@ void qcom_minidump(struct rproc *rproc, unsigned 
>> int minidump_id)
>>        */
>>       if (subsystem->regions_baseptr == 0 ||
>>           le32_to_cpu(subsystem->status) != 1 ||
>> -        le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED ||
>> -        le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
>> +        le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED) {
>> +        return rproc_coredump(rproc);
>> +    }
>> +
>> +    if (le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
>>           dev_err(&rproc->dev, "Minidump not ready, skipping\n");
>>           return;
>>       }

Yogesh,

  /**

   * Clear out the dump segments populated by parse_fw before

   * re-populating them with minidump segments.

   */
   rproc_coredump_cleanup(rproc);

You'll still need to cleanup segments populated by parse_fw
before you move onto to minidumps.

>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c 
>> b/drivers/remoteproc/qcom_q6v5_pas.c
>> index 401b1ec..6e5cbca 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pas.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
>> @@ -274,6 +274,7 @@ static const struct rproc_ops adsp_minidump_ops = {
>>       .start = adsp_start,
>>       .stop = adsp_stop,
>>       .da_to_va = adsp_da_to_va,
>> +    .parse_fw = qcom_register_dump_segments,
>>       .load = adsp_load,
>>       .panic = adsp_panic,
>>       .coredump = adsp_minidump,
>>