diff mbox series

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

Message ID 1652181930-22212-2-git-send-email-quic_ylal@quicinc.com (mailing list archive)
State New
Headers show
Series [1/2,V2] remoteproc: core: Export the rproc coredump APIs | expand

Commit Message

Yogesh Lal May 10, 2022, 11:25 a.m. UTC
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(-)

Comments

Sibi Sankar May 21, 2022, 6:14 a.m. UTC | #1
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,
>
Sibi Sankar May 24, 2022, 10:03 a.m. UTC | #2
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,
>>
diff mbox series

Patch

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,