All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] remoteproc: qcom: Add fallback mechanism for full coredump collection
@ 2022-04-06 18:27 Yogesh Lal
  2022-04-22 15:22 ` Bjorn Andersson
  0 siblings, 1 reply; 3+ messages in thread
From: Yogesh Lal @ 2022-04-06 18:27 UTC (permalink / raw)
  To: bjorn.andersson, quic_sibis
  Cc: linux-arm-msm, linux-remoteproc, linux-kernel, Yogesh Lal

In case remoteproc's firmware missing minidump support, during crash
scenario coredump does not collected. This change adds a fallback
mechanism for full coredump collection in the event of a crash.

Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
---
 drivers/remoteproc/qcom_common.c   | 11 ++++++++---
 drivers/remoteproc/qcom_q6v5_pas.c |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4b91e3c..68bd0bc 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -162,13 +162,18 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
 	 * is initialized in memory and encryption status is set.
 	 */
 	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->status) != 1 ||
+		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;
 	}
 
+	rproc_coredump_cleanup(rproc);
+
 	ret = qcom_add_minidump_segments(rproc, subsystem);
 	if (ret) {
 		dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 1ae47cc..40bf747 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -293,6 +293,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] remoteproc: qcom: Add fallback mechanism for full coredump collection
  2022-04-06 18:27 [PATCH] remoteproc: qcom: Add fallback mechanism for full coredump collection Yogesh Lal
@ 2022-04-22 15:22 ` Bjorn Andersson
  2022-04-26 10:15   ` Yogesh Lal
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Andersson @ 2022-04-22 15:22 UTC (permalink / raw)
  To: Yogesh Lal; +Cc: quic_sibis, linux-arm-msm, linux-remoteproc, linux-kernel

On Wed 06 Apr 13:27 CDT 2022, Yogesh Lal wrote:

> In case remoteproc's firmware missing minidump support, during crash
> scenario coredump does not collected. This change adds a fallback
> mechanism for full coredump collection in the event of a crash.
> 
> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
> ---
>  drivers/remoteproc/qcom_common.c   | 11 ++++++++---
>  drivers/remoteproc/qcom_q6v5_pas.c |  1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 4b91e3c..68bd0bc 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -162,13 +162,18 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
>  	 * is initialized in memory and encryption status is set.
>  	 */
>  	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->status) != 1 ||
> +		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;
>  	}
>  
> +	rproc_coredump_cleanup(rproc);

The patch looks good, but could you please explain in the commit message
why this needs to be added? If the thing described in the message
happens this code path wouldn't be taken.

Should it be a separate patch, or is it needed because of the fallback
etc?

Thanks,
Bjorn

> +
>  	ret = qcom_add_minidump_segments(rproc, subsystem);
>  	if (ret) {
>  		dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 1ae47cc..40bf747 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -293,6 +293,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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] remoteproc: qcom: Add fallback mechanism for full coredump collection
  2022-04-22 15:22 ` Bjorn Andersson
@ 2022-04-26 10:15   ` Yogesh Lal
  0 siblings, 0 replies; 3+ messages in thread
From: Yogesh Lal @ 2022-04-26 10:15 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Sibi Sankar (QUIC), linux-arm-msm, linux-remoteproc, linux-kernel


On 4/22/2022 8:52 PM, Bjorn Andersson wrote:
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On Wed 06 Apr 13:27 CDT 2022, Yogesh Lal wrote:
>
>> In case remoteproc's firmware missing minidump support, during crash
>> scenario coredump does not collected. This change adds a fallback
>> mechanism for full coredump collection in the event of a crash.
>>
>> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
>> ---
>>   drivers/remoteproc/qcom_common.c   | 11 ++++++++---
>>   drivers/remoteproc/qcom_q6v5_pas.c |  1 +
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
>> index 4b91e3c..68bd0bc 100644
>> --- a/drivers/remoteproc/qcom_common.c
>> +++ b/drivers/remoteproc/qcom_common.c
>> @@ -162,13 +162,18 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
>>         * is initialized in memory and encryption status is set.
>>         */
>>        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->status) != 1 ||
>> +             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;
>>        }
>>
>> +     rproc_coredump_cleanup(rproc);
> The patch looks good, but could you please explain in the commit message
> why this needs to be added? If the thing described in the message
> happens this code path wouldn't be taken.
>
> Should it be a separate patch, or is it needed because of the fallback
> etc?
>
> Thanks,
> Bjorn

Will push separate patch for it.

Thanks

Yogesh Lal


>> +
>>        ret = qcom_add_minidump_segments(rproc, subsystem);
>>        if (ret) {
>>                dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
>> index 1ae47cc..40bf747 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pas.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
>> @@ -293,6 +293,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
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-26 10:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-06 18:27 [PATCH] remoteproc: qcom: Add fallback mechanism for full coredump collection Yogesh Lal
2022-04-22 15:22 ` Bjorn Andersson
2022-04-26 10:15   ` Yogesh Lal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.