diff mbox series

remoteproc: qcom: pas: Adjust the phys addr wrt the mem region

Message ID 1654200007-5453-1-git-send-email-quic_ylal@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series remoteproc: qcom: pas: Adjust the phys addr wrt the mem region | expand

Commit Message

Yogesh Lal June 2, 2022, 8 p.m. UTC
The minidump table in the toc contains physical addresses that may lie
before the physical address of the first elf segment in relocatable
images. This change adds a custom dump function for minidumps which
calculates the offset into the carveout region using the start of
the physical address instead of the start of the first elf segment.

Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
---
 drivers/remoteproc/qcom_common.c   |  9 +++++----
 drivers/remoteproc/qcom_common.h   |  5 ++++-
 drivers/remoteproc/qcom_q6v5_pas.c | 21 ++++++++++++++++++++-
 3 files changed, 29 insertions(+), 6 deletions(-)

Comments

Sibi Sankar June 24, 2022, 11:19 a.m. UTC | #1
Hey Yogesh,
Thanks for the patch.

On 6/3/22 1:30 AM, Yogesh Lal wrote:
> The minidump table in the toc contains physical addresses that may lie
> before the physical address of the first elf segment in relocatable

doesn't this apply to full coredumps as well? Do you plan to address
that in a separate patch?

> images. This change adds a custom dump function for minidumps which
> calculates the offset into the carveout region using the start of
> the physical address instead of the start of the first elf segment.
> 
> Signed-off-by: Yogesh Lal <quic_ylal@quicinc.com>
> ---
>   drivers/remoteproc/qcom_common.c   |  9 +++++----
>   drivers/remoteproc/qcom_common.h   |  5 ++++-
>   drivers/remoteproc/qcom_q6v5_pas.c | 21 ++++++++++++++++++++-
>   3 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 246e716..503326b 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -101,7 +101,8 @@ static void qcom_minidump_cleanup(struct rproc *rproc)
>   	}
>   }
>   
> -static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem)
> +static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem,
> +									rproc_dumpfn_t dumpfn)
>   {
>   	struct minidump_region __iomem *ptr;
>   	struct minidump_region region;
> @@ -131,7 +132,7 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
>   			}
>   			da = le64_to_cpu(region.address);
>   			size = le32_to_cpu(region.size);
> -			rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
> +			rproc_coredump_add_custom_segment(rproc, da, size, dumpfn, name);
>   		}
>   	}
>   
> @@ -139,7 +140,7 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
>   	return 0;
>   }
>   
> -void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
> +void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, rproc_dumpfn_t dumpfn)
>   {
>   	int ret;
>   	struct minidump_subsystem *subsystem;
> @@ -179,7 +180,7 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
>   
>   	rproc_coredump_cleanup(rproc);
>   
> -	ret = qcom_add_minidump_segments(rproc, subsystem);
> +	ret = qcom_add_minidump_segments(rproc, subsystem, dumpfn);
>   	if (ret) {
>   		dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
>   		goto clean_minidump;
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index c35adf7..29e528b 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -33,7 +33,10 @@ struct qcom_rproc_ssr {
>   	struct qcom_ssr_subsystem *info;
>   };
>   
> -void qcom_minidump(struct rproc *rproc, unsigned int minidump_id);
> +typedef void (*rproc_dumpfn_t)(struct rproc *rproc, struct rproc_dump_segment *segment,
> +			void *dest, size_t offset, size_t size);

you can perhaps stick with not using typedef like how it is handled in
remoteproc_coredump.

> +
> +void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, rproc_dumpfn_t dumpfn);
>   
>   void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
>   			   const char *ssr_name);
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 6e5cbca..9c6cb0b 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -83,11 +83,30 @@ struct qcom_adsp {
>   	struct qcom_sysmon *sysmon;
>   };
>   
> +void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment,
> +						void *dest, size_t offset, size_t size)
> +{
> +	struct qcom_adsp *adsp = rproc->priv;
> +	int total_offset;
> +
> +	total_offset = segment->da + segment->offset + offset - adsp->mem_phys;
> +	if (total_offset < 0 || total_offset + size > adsp->mem_size) {
> +		dev_err(adsp->dev,
> +			"invalid copy request for segment %pad with offset %zu and size %zu)\n",
> +			&segment->da, offset, size);
> +		memset(dest, 0xff, size);
> +		return;
> +	}
> +
> +	memcpy_fromio(dest, adsp->mem_region + total_offset, size);
> +}
> +
> +
remove additional empty line.

>   static void adsp_minidump(struct rproc *rproc)
>   {
>   	struct qcom_adsp *adsp = rproc->priv;
>   
> -	qcom_minidump(rproc, adsp->minidump_id);
> +	qcom_minidump(rproc, adsp->minidump_id, adsp_segment_dump);
>   }
>   
>   static int adsp_pds_enable(struct qcom_adsp *adsp, struct device **pds,
>
diff mbox series

Patch

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 246e716..503326b 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -101,7 +101,8 @@  static void qcom_minidump_cleanup(struct rproc *rproc)
 	}
 }
 
-static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem)
+static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem,
+									rproc_dumpfn_t dumpfn)
 {
 	struct minidump_region __iomem *ptr;
 	struct minidump_region region;
@@ -131,7 +132,7 @@  static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
 			}
 			da = le64_to_cpu(region.address);
 			size = le32_to_cpu(region.size);
-			rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
+			rproc_coredump_add_custom_segment(rproc, da, size, dumpfn, name);
 		}
 	}
 
@@ -139,7 +140,7 @@  static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
 	return 0;
 }
 
-void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
+void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, rproc_dumpfn_t dumpfn)
 {
 	int ret;
 	struct minidump_subsystem *subsystem;
@@ -179,7 +180,7 @@  void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
 
 	rproc_coredump_cleanup(rproc);
 
-	ret = qcom_add_minidump_segments(rproc, subsystem);
+	ret = qcom_add_minidump_segments(rproc, subsystem, dumpfn);
 	if (ret) {
 		dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
 		goto clean_minidump;
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index c35adf7..29e528b 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -33,7 +33,10 @@  struct qcom_rproc_ssr {
 	struct qcom_ssr_subsystem *info;
 };
 
-void qcom_minidump(struct rproc *rproc, unsigned int minidump_id);
+typedef void (*rproc_dumpfn_t)(struct rproc *rproc, struct rproc_dump_segment *segment,
+			void *dest, size_t offset, size_t size);
+
+void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, rproc_dumpfn_t dumpfn);
 
 void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
 			   const char *ssr_name);
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 6e5cbca..9c6cb0b 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -83,11 +83,30 @@  struct qcom_adsp {
 	struct qcom_sysmon *sysmon;
 };
 
+void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment,
+						void *dest, size_t offset, size_t size)
+{
+	struct qcom_adsp *adsp = rproc->priv;
+	int total_offset;
+
+	total_offset = segment->da + segment->offset + offset - adsp->mem_phys;
+	if (total_offset < 0 || total_offset + size > adsp->mem_size) {
+		dev_err(adsp->dev,
+			"invalid copy request for segment %pad with offset %zu and size %zu)\n",
+			&segment->da, offset, size);
+		memset(dest, 0xff, size);
+		return;
+	}
+
+	memcpy_fromio(dest, adsp->mem_region + total_offset, size);
+}
+
+
 static void adsp_minidump(struct rproc *rproc)
 {
 	struct qcom_adsp *adsp = rproc->priv;
 
-	qcom_minidump(rproc, adsp->minidump_id);
+	qcom_minidump(rproc, adsp->minidump_id, adsp_segment_dump);
 }
 
 static int adsp_pds_enable(struct qcom_adsp *adsp, struct device **pds,