* [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use
@ 2020-03-17 19:19 Sibi Sankar
2020-04-08 2:10 ` Bjorn Andersson
0 siblings, 1 reply; 4+ messages in thread
From: Sibi Sankar @ 2020-03-17 19:19 UTC (permalink / raw)
To: bjorn.andersson
Cc: agross, ohad, linux-arm-msm, linux-remoteproc, linux-kernel,
evgreen, Sibi Sankar
The application processor accessing the mpss region when the Q6 modem
is running will lead to an XPU violation. Fix this by un-mapping the
mpss region post copy during processor out of reset sequence and
coredumps.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_mss.c | 53 ++++++++++++++++--------------
1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index ce49c3236ff7c..b1ad4de179019 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -196,7 +196,6 @@ struct q6v5 {
phys_addr_t mpss_phys;
phys_addr_t mpss_reloc;
- void *mpss_region;
size_t mpss_size;
struct qcom_rproc_glink glink_subdev;
@@ -1061,6 +1060,18 @@ static int q6v5_reload_mba(struct rproc *rproc)
return ret;
}
+static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
+{
+ struct q6v5 *qproc = rproc->priv;
+ int offset;
+
+ offset = da - qproc->mpss_reloc;
+ if (offset < 0 || offset + len > qproc->mpss_size)
+ return NULL;
+
+ return devm_ioremap_wc(qproc->dev, qproc->mpss_phys + offset, len);
+}
+
static int q6v5_mpss_load(struct q6v5 *qproc)
{
const struct elf32_phdr *phdrs;
@@ -1156,7 +1167,11 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
goto release_firmware;
}
- ptr = qproc->mpss_region + offset;
+ ptr = q6v5_da_to_va(qproc->rproc, phdr->p_paddr, phdr->p_memsz);
+ if (!ptr) {
+ dev_err(qproc->dev, "failed to map memory\n");
+ goto release_firmware;
+ }
if (phdr->p_filesz && phdr->p_offset < fw->size) {
/* Firmware is large enough to be non-split */
@@ -1165,6 +1180,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
"failed to load segment %d from truncated file %s\n",
i, fw_name);
ret = -EINVAL;
+ devm_iounmap(qproc->dev, ptr);
goto release_firmware;
}
@@ -1175,6 +1191,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
ret = request_firmware(&seg_fw, fw_name, qproc->dev);
if (ret) {
dev_err(qproc->dev, "failed to load %s\n", fw_name);
+ devm_iounmap(qproc->dev, ptr);
goto release_firmware;
}
@@ -1187,6 +1204,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
memset(ptr + phdr->p_filesz, 0,
phdr->p_memsz - phdr->p_filesz);
}
+ devm_iounmap(qproc->dev, ptr);
size += phdr->p_memsz;
code_length = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
@@ -1236,7 +1254,7 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
int ret = 0;
struct q6v5 *qproc = rproc->priv;
unsigned long mask = BIT((unsigned long)segment->priv);
- void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
+ void *ptr = NULL;
/* Unlock mba before copying segments */
if (!qproc->dump_mba_loaded) {
@@ -1250,10 +1268,15 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
}
}
- if (!ptr || ret)
- memset(dest, 0xff, segment->size);
- else
+ if (!ret)
+ ptr = rproc_da_to_va(rproc, segment->da, segment->size);
+
+ if (ptr) {
memcpy(dest, ptr, segment->size);
+ devm_iounmap(qproc->dev, ptr);
+ } else {
+ memset(dest, 0xff, segment->size);
+ }
qproc->dump_segment_mask |= mask;
@@ -1327,18 +1350,6 @@ static int q6v5_stop(struct rproc *rproc)
return 0;
}
-static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
-{
- struct q6v5 *qproc = rproc->priv;
- int offset;
-
- offset = da - qproc->mpss_reloc;
- if (offset < 0 || offset + len > qproc->mpss_size)
- return NULL;
-
- return qproc->mpss_region + offset;
-}
-
static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
const struct firmware *mba_fw)
{
@@ -1595,12 +1606,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
qproc->mpss_phys = qproc->mpss_reloc = r.start;
qproc->mpss_size = resource_size(&r);
- qproc->mpss_region = devm_ioremap_wc(qproc->dev, qproc->mpss_phys, qproc->mpss_size);
- if (!qproc->mpss_region) {
- dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
- &r.start, qproc->mpss_size);
- return -EBUSY;
- }
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use
2020-03-17 19:19 [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use Sibi Sankar
@ 2020-04-08 2:10 ` Bjorn Andersson
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2020-04-08 2:10 UTC (permalink / raw)
To: Sibi Sankar
Cc: agross, ohad, linux-arm-msm, linux-remoteproc, linux-kernel, evgreen
On Tue 17 Mar 12:19 PDT 2020, Sibi Sankar wrote:
> The application processor accessing the mpss region when the Q6 modem
> is running will lead to an XPU violation. Fix this by un-mapping the
> mpss region post copy during processor out of reset sequence and
> coredumps.
>
Does this problem not apply to the "mba" region?
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 53 ++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index ce49c3236ff7c..b1ad4de179019 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -196,7 +196,6 @@ struct q6v5 {
>
> phys_addr_t mpss_phys;
> phys_addr_t mpss_reloc;
> - void *mpss_region;
> size_t mpss_size;
>
> struct qcom_rproc_glink glink_subdev;
> @@ -1061,6 +1060,18 @@ static int q6v5_reload_mba(struct rproc *rproc)
> return ret;
> }
>
> +static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
> +{
> + struct q6v5 *qproc = rproc->priv;
> + int offset;
> +
> + offset = da - qproc->mpss_reloc;
> + if (offset < 0 || offset + len > qproc->mpss_size)
> + return NULL;
> +
> + return devm_ioremap_wc(qproc->dev, qproc->mpss_phys + offset, len);
This function isn't expected to have side effects.
So I think you should add the ioremap/iounmap to the beginning/end of
mpss_load and the dump_segment directly instead.
> +}
> +
> static int q6v5_mpss_load(struct q6v5 *qproc)
> {
> const struct elf32_phdr *phdrs;
> @@ -1156,7 +1167,11 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> goto release_firmware;
> }
>
> - ptr = qproc->mpss_region + offset;
> + ptr = q6v5_da_to_va(qproc->rproc, phdr->p_paddr, phdr->p_memsz);
rproc_da_to_va() here.
> + if (!ptr) {
> + dev_err(qproc->dev, "failed to map memory\n");
Now this will be able to fail, so you should add this error handling
snippet, just with a slightly different message.
> + goto release_firmware;
> + }
>
> if (phdr->p_filesz && phdr->p_offset < fw->size) {
> /* Firmware is large enough to be non-split */
> @@ -1165,6 +1180,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> "failed to load segment %d from truncated file %s\n",
> i, fw_name);
> ret = -EINVAL;
> + devm_iounmap(qproc->dev, ptr);
> goto release_firmware;
> }
>
> @@ -1175,6 +1191,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> ret = request_firmware(&seg_fw, fw_name, qproc->dev);
> if (ret) {
> dev_err(qproc->dev, "failed to load %s\n", fw_name);
> + devm_iounmap(qproc->dev, ptr);
> goto release_firmware;
> }
>
> @@ -1187,6 +1204,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> memset(ptr + phdr->p_filesz, 0,
> phdr->p_memsz - phdr->p_filesz);
> }
> + devm_iounmap(qproc->dev, ptr);
Move this to the end an unmap the entire thing.
And generally, please avoid devm for things where you manually unmap.
Regards,
Bjorn
> size += phdr->p_memsz;
>
> code_length = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
> @@ -1236,7 +1254,7 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
> int ret = 0;
> struct q6v5 *qproc = rproc->priv;
> unsigned long mask = BIT((unsigned long)segment->priv);
> - void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
> + void *ptr = NULL;
>
> /* Unlock mba before copying segments */
> if (!qproc->dump_mba_loaded) {
> @@ -1250,10 +1268,15 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
> }
> }
>
> - if (!ptr || ret)
> - memset(dest, 0xff, segment->size);
> - else
> + if (!ret)
> + ptr = rproc_da_to_va(rproc, segment->da, segment->size);
> +
> + if (ptr) {
> memcpy(dest, ptr, segment->size);
> + devm_iounmap(qproc->dev, ptr);
> + } else {
> + memset(dest, 0xff, segment->size);
> + }
>
> qproc->dump_segment_mask |= mask;
>
> @@ -1327,18 +1350,6 @@ static int q6v5_stop(struct rproc *rproc)
> return 0;
> }
>
> -static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
> -{
> - struct q6v5 *qproc = rproc->priv;
> - int offset;
> -
> - offset = da - qproc->mpss_reloc;
> - if (offset < 0 || offset + len > qproc->mpss_size)
> - return NULL;
> -
> - return qproc->mpss_region + offset;
> -}
> -
> static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
> const struct firmware *mba_fw)
> {
> @@ -1595,12 +1606,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
>
> qproc->mpss_phys = qproc->mpss_reloc = r.start;
> qproc->mpss_size = resource_size(&r);
> - qproc->mpss_region = devm_ioremap_wc(qproc->dev, qproc->mpss_phys, qproc->mpss_size);
> - if (!qproc->mpss_region) {
> - dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
> - &r.start, qproc->mpss_size);
> - return -EBUSY;
> - }
>
> return 0;
> }
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use
@ 2020-04-08 2:10 ` Bjorn Andersson
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2020-04-08 2:10 UTC (permalink / raw)
To: Sibi Sankar
Cc: agross, ohad, linux-arm-msm, linux-remoteproc, linux-kernel, evgreen
On Tue 17 Mar 12:19 PDT 2020, Sibi Sankar wrote:
> The application processor accessing the mpss region when the Q6 modem
> is running will lead to an XPU violation. Fix this by un-mapping the
> mpss region post copy during processor out of reset sequence and
> coredumps.
>
Does this problem not apply to the "mba" region?
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 53 ++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index ce49c3236ff7c..b1ad4de179019 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -196,7 +196,6 @@ struct q6v5 {
>
> phys_addr_t mpss_phys;
> phys_addr_t mpss_reloc;
> - void *mpss_region;
> size_t mpss_size;
>
> struct qcom_rproc_glink glink_subdev;
> @@ -1061,6 +1060,18 @@ static int q6v5_reload_mba(struct rproc *rproc)
> return ret;
> }
>
> +static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
> +{
> + struct q6v5 *qproc = rproc->priv;
> + int offset;
> +
> + offset = da - qproc->mpss_reloc;
> + if (offset < 0 || offset + len > qproc->mpss_size)
> + return NULL;
> +
> + return devm_ioremap_wc(qproc->dev, qproc->mpss_phys + offset, len);
This function isn't expected to have side effects.
So I think you should add the ioremap/iounmap to the beginning/end of
mpss_load and the dump_segment directly instead.
> +}
> +
> static int q6v5_mpss_load(struct q6v5 *qproc)
> {
> const struct elf32_phdr *phdrs;
> @@ -1156,7 +1167,11 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> goto release_firmware;
> }
>
> - ptr = qproc->mpss_region + offset;
> + ptr = q6v5_da_to_va(qproc->rproc, phdr->p_paddr, phdr->p_memsz);
rproc_da_to_va() here.
> + if (!ptr) {
> + dev_err(qproc->dev, "failed to map memory\n");
Now this will be able to fail, so you should add this error handling
snippet, just with a slightly different message.
> + goto release_firmware;
> + }
>
> if (phdr->p_filesz && phdr->p_offset < fw->size) {
> /* Firmware is large enough to be non-split */
> @@ -1165,6 +1180,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> "failed to load segment %d from truncated file %s\n",
> i, fw_name);
> ret = -EINVAL;
> + devm_iounmap(qproc->dev, ptr);
> goto release_firmware;
> }
>
> @@ -1175,6 +1191,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> ret = request_firmware(&seg_fw, fw_name, qproc->dev);
> if (ret) {
> dev_err(qproc->dev, "failed to load %s\n", fw_name);
> + devm_iounmap(qproc->dev, ptr);
> goto release_firmware;
> }
>
> @@ -1187,6 +1204,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> memset(ptr + phdr->p_filesz, 0,
> phdr->p_memsz - phdr->p_filesz);
> }
> + devm_iounmap(qproc->dev, ptr);
Move this to the end an unmap the entire thing.
And generally, please avoid devm for things where you manually unmap.
Regards,
Bjorn
> size += phdr->p_memsz;
>
> code_length = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
> @@ -1236,7 +1254,7 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
> int ret = 0;
> struct q6v5 *qproc = rproc->priv;
> unsigned long mask = BIT((unsigned long)segment->priv);
> - void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
> + void *ptr = NULL;
>
> /* Unlock mba before copying segments */
> if (!qproc->dump_mba_loaded) {
> @@ -1250,10 +1268,15 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
> }
> }
>
> - if (!ptr || ret)
> - memset(dest, 0xff, segment->size);
> - else
> + if (!ret)
> + ptr = rproc_da_to_va(rproc, segment->da, segment->size);
> +
> + if (ptr) {
> memcpy(dest, ptr, segment->size);
> + devm_iounmap(qproc->dev, ptr);
> + } else {
> + memset(dest, 0xff, segment->size);
> + }
>
> qproc->dump_segment_mask |= mask;
>
> @@ -1327,18 +1350,6 @@ static int q6v5_stop(struct rproc *rproc)
> return 0;
> }
>
> -static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
> -{
> - struct q6v5 *qproc = rproc->priv;
> - int offset;
> -
> - offset = da - qproc->mpss_reloc;
> - if (offset < 0 || offset + len > qproc->mpss_size)
> - return NULL;
> -
> - return qproc->mpss_region + offset;
> -}
> -
> static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
> const struct firmware *mba_fw)
> {
> @@ -1595,12 +1606,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
>
> qproc->mpss_phys = qproc->mpss_reloc = r.start;
> qproc->mpss_size = resource_size(&r);
> - qproc->mpss_region = devm_ioremap_wc(qproc->dev, qproc->mpss_phys, qproc->mpss_size);
> - if (!qproc->mpss_region) {
> - dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
> - &r.start, qproc->mpss_size);
> - return -EBUSY;
> - }
>
> return 0;
> }
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use
2020-04-08 2:10 ` Bjorn Andersson
(?)
@ 2020-04-08 12:20 ` Sibi Sankar
-1 siblings, 0 replies; 4+ messages in thread
From: Sibi Sankar @ 2020-04-08 12:20 UTC (permalink / raw)
To: Bjorn Andersson
Cc: agross, ohad, linux-arm-msm, linux-remoteproc, linux-kernel,
evgreen, linux-arm-msm-owner
Hey Bjorn,
Thanks for review!
On 2020-04-08 07:40, Bjorn Andersson wrote:
> On Tue 17 Mar 12:19 PDT 2020, Sibi Sankar wrote:
>
>> The application processor accessing the mpss region when the Q6 modem
>> is running will lead to an XPU violation. Fix this by un-mapping the
>> mpss region post copy during processor out of reset sequence and
>> coredumps.
>>
>
> Does this problem not apply to the "mba" region?
For mba region, memcpy is expected
to be completed before bringing
the Q6 out of reset. Downstream they
seem to use a wmb() to accomplish
this. Since we havn't observed any
issues until now, we can defer adding
any fixes related to mba region.
>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> drivers/remoteproc/qcom_q6v5_mss.c | 53
>> ++++++++++++++++--------------
>> 1 file changed, 29 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c
>> b/drivers/remoteproc/qcom_q6v5_mss.c
>> index ce49c3236ff7c..b1ad4de179019 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -196,7 +196,6 @@ struct q6v5 {
>>
>> phys_addr_t mpss_phys;
>> phys_addr_t mpss_reloc;
>> - void *mpss_region;
>> size_t mpss_size;
>>
>> struct qcom_rproc_glink glink_subdev;
>> @@ -1061,6 +1060,18 @@ static int q6v5_reload_mba(struct rproc *rproc)
>> return ret;
>> }
>>
>> +static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
>> +{
>> + struct q6v5 *qproc = rproc->priv;
>> + int offset;
>> +
>> + offset = da - qproc->mpss_reloc;
>> + if (offset < 0 || offset + len > qproc->mpss_size)
>> + return NULL;
>> +
>> + return devm_ioremap_wc(qproc->dev, qproc->mpss_phys + offset, len);
>
> This function isn't expected to have side effects.
unfortunately doing ioremap/iounmap
for the entire region isn't sufficient,
while testing I found per region remap
/unmap was required i.e after we moved to
away from the validating the entire blog
in a single pass.
Now with the mpss_region out of the picture
da_to_va really made no sense without having
ioremap function in it. Let me know what you
think.
>
> So I think you should add the ioremap/iounmap to the beginning/end of
> mpss_load and the dump_segment directly instead.
>
>> +}
>> +
>> static int q6v5_mpss_load(struct q6v5 *qproc)
>> {
>> const struct elf32_phdr *phdrs;
>> @@ -1156,7 +1167,11 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>> goto release_firmware;
>> }
>>
>> - ptr = qproc->mpss_region + offset;
>> + ptr = q6v5_da_to_va(qproc->rproc, phdr->p_paddr, phdr->p_memsz);
>
> rproc_da_to_va() here.
sure
>
>> + if (!ptr) {
>> + dev_err(qproc->dev, "failed to map memory\n");
>
> Now this will be able to fail, so you should add this error handling
> snippet, just with a slightly different message.
sure
>
>> + goto release_firmware;
>> + }
>>
>> if (phdr->p_filesz && phdr->p_offset < fw->size) {
>> /* Firmware is large enough to be non-split */
>> @@ -1165,6 +1180,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>> "failed to load segment %d from truncated file %s\n",
>> i, fw_name);
>> ret = -EINVAL;
>> + devm_iounmap(qproc->dev, ptr);
>> goto release_firmware;
>> }
>>
>> @@ -1175,6 +1191,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>> ret = request_firmware(&seg_fw, fw_name, qproc->dev);
>> if (ret) {
>> dev_err(qproc->dev, "failed to load %s\n", fw_name);
>> + devm_iounmap(qproc->dev, ptr);
>> goto release_firmware;
>> }
>>
>> @@ -1187,6 +1204,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>> memset(ptr + phdr->p_filesz, 0,
>> phdr->p_memsz - phdr->p_filesz);
>> }
>> + devm_iounmap(qproc->dev, ptr);
>
> Move this to the end an unmap the entire thing.
>
> And generally, please avoid devm for things where you manually unmap.
will take care of ^^ in the next re-spin.
>
> Regards,
> Bjorn
>
>> size += phdr->p_memsz;
>>
>> code_length = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
>> @@ -1236,7 +1254,7 @@ static void qcom_q6v5_dump_segment(struct rproc
>> *rproc,
>> int ret = 0;
>> struct q6v5 *qproc = rproc->priv;
>> unsigned long mask = BIT((unsigned long)segment->priv);
>> - void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
>> + void *ptr = NULL;
>>
>> /* Unlock mba before copying segments */
>> if (!qproc->dump_mba_loaded) {
>> @@ -1250,10 +1268,15 @@ static void qcom_q6v5_dump_segment(struct
>> rproc *rproc,
>> }
>> }
>>
>> - if (!ptr || ret)
>> - memset(dest, 0xff, segment->size);
>> - else
>> + if (!ret)
>> + ptr = rproc_da_to_va(rproc, segment->da, segment->size);
>> +
>> + if (ptr) {
>> memcpy(dest, ptr, segment->size);
>> + devm_iounmap(qproc->dev, ptr);
>> + } else {
>> + memset(dest, 0xff, segment->size);
>> + }
>>
>> qproc->dump_segment_mask |= mask;
>>
>> @@ -1327,18 +1350,6 @@ static int q6v5_stop(struct rproc *rproc)
>> return 0;
>> }
>>
>> -static void *q6v5_da_to_va(struct rproc *rproc, u64 da, size_t len)
>> -{
>> - struct q6v5 *qproc = rproc->priv;
>> - int offset;
>> -
>> - offset = da - qproc->mpss_reloc;
>> - if (offset < 0 || offset + len > qproc->mpss_size)
>> - return NULL;
>> -
>> - return qproc->mpss_region + offset;
>> -}
>> -
>> static int qcom_q6v5_register_dump_segments(struct rproc *rproc,
>> const struct firmware *mba_fw)
>> {
>> @@ -1595,12 +1606,6 @@ static int q6v5_alloc_memory_region(struct q6v5
>> *qproc)
>>
>> qproc->mpss_phys = qproc->mpss_reloc = r.start;
>> qproc->mpss_size = resource_size(&r);
>> - qproc->mpss_region = devm_ioremap_wc(qproc->dev, qproc->mpss_phys,
>> qproc->mpss_size);
>> - if (!qproc->mpss_region) {
>> - dev_err(qproc->dev, "unable to map memory region: %pa+%zx\n",
>> - &r.start, qproc->mpss_size);
>> - return -EBUSY;
>> - }
>>
>> return 0;
>> }
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-08 12:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 19:19 [PATCH] remoteproc: qcom_q6v5_mss: map/unmap mpss region before/after use Sibi Sankar
2020-04-08 2:10 ` Bjorn Andersson
2020-04-08 2:10 ` Bjorn Andersson
2020-04-08 2:10 ` Bjorn Andersson
2020-04-08 12:20 ` Sibi Sankar
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.