Re: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-14 Thread Mathieu Poirier
On Thu, Dec 10, 2020 at 10:54:17AM -0600, Bjorn Andersson wrote:
> On Sun 06 Dec 20:07 CST 2020, Peng Fan wrote:
> 
> > Hi Bjorn,
> > 
> > > Subject: Re: [PATCH V3 1/7] remoteproc: elf: support platform specific
> > > memory hook
> > > 
> > > On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:
> > > 
> > > > From: Peng Fan 
> > > >
> > > > To arm64, "dc  zva, dst" is used in memset.
> > > > Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> > > >
> > > > "If the memory region being zeroed is any type of Device memory, this
> > > > instruction can give an alignment fault which is prioritized in the
> > > > same way as other alignment faults that are determined by the memory
> > > > type."
> > > >
> > > > On i.MX platforms, when elf is loaded to onchip TCM area, the region
> > > > is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> > > > on i.MX not able to write correct data to TCM area.
> > > >
> > > > So we need to use io helpers, and extend the elf loader to support
> > > > platform specific memory functions.
> > > >
> > > > Acked-by: Richard Zhu 
> > > > Signed-off-by: Peng Fan 
> > > > Reviewed-by: Mathieu Poirier 
> > > > ---
> > > >  drivers/remoteproc/remoteproc_elf_loader.c | 20
> > > ++--
> > > >  include/linux/remoteproc.h |  4 
> > > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> > > > b/drivers/remoteproc/remoteproc_elf_loader.c
> > > > index df68d87752e4..6cb71fe47261 100644
> > > > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > > > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > > > @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc,
> > > > const struct firmware *fw)  }
> > > EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> > > >
> > > > +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const
> > > > +void *src, size_t count) {
> > > > +   if (!rproc->ops->elf_memcpy)
> > > > +   memcpy(dest, src, count);
> > > > +
> > > > +   rproc->ops->elf_memcpy(rproc, dest, src, count);
> > > 
> > > Looking at the current set of remoteproc drivers I get a feeling that 
> > > we'll end
> > > up with a while bunch of functions that all just wraps memcpy_toio(). And 
> > > the
> > > reason for this is that we are we're "abusing" the carveout to carry the
> > > __iomem pointer without keeping track of it.
> > > 
> > > And this is not the only time we're supposed to use an io-accessor, 
> > > another
> > > example is rproc_copy_segment() in rproc_coredump.c
> > > 
> > > It also means that if a platform driver for some reason where to support 
> > > both
> > > ioremap and normal carveouts the elf_memcpy op would be quite quirky.
> > > 
> > > 
> > > So I would prefer if we track the knowledge about void *va being a __iomem
> > > or not in the struct rproc_mem_entry and make rproc_da_to_va() return this
> > > information as well.
> > > 
> > > Then instead of extending the ops we can make this simply call memcpy or
> > > memcpy_toio() depending on this.
> > 
> > A draft proposal as below, are you ok with the approach?
> > 
> 
> Yes, this looks very reasonable and should be directly useful for the
> other users of ioremap as well.

Bjorn is correct in his assessment that using rproc_ops::elf_memcpy and
rproc_ops::elf_memset will lead to platform driver just wrapping xyz_toio().
On the flip side the advantage is that the core code stays the same when the
next wave of memory accessor is needed.

But there is no telling when that will happen and what the requirements will be
so I'm also in favour of moving forward with what you suggested below.  Please
read on, there's a couple of comments to a address.

> 
> May I ask that you rename the boolean "iomem" with "is_iomem", to make
> it obvious that it's a boolean in the cases where we're already juggling
> physical, virtual and device addresses?
> 
> Thanks,
> Bjorn
> 
> > diff --git a/drivers/remoteproc/remoteproc_core.c 
> > b/drivers/remoteproc/remoteproc_core.c
> > index 46c

Re: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-11 Thread Mathieu Poirier
On Wed, 9 Dec 2020 at 08:00, Peng Fan  wrote:
>
> > Subject: RE: [PATCH V3 1/7] remoteproc: elf: support platform specific
> > memory hook
> >
> > Hi Bjorn,
> >
> > > Subject: Re: [PATCH V3 1/7] remoteproc: elf: support platform specific
> > > memory hook
> > >
> > > On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:
> > >
> > > > From: Peng Fan 
> > > >
> > > > To arm64, "dc  zva, dst" is used in memset.
> > > > Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> > > >
> > > > "If the memory region being zeroed is any type of Device memory,
> > > > this instruction can give an alignment fault which is prioritized in
> > > > the same way as other alignment faults that are determined by the
> > > > memory type."
> > > >
> > > > On i.MX platforms, when elf is loaded to onchip TCM area, the region
> > > > is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> > > > on i.MX not able to write correct data to TCM area.
> > > >
> > > > So we need to use io helpers, and extend the elf loader to support
> > > > platform specific memory functions.
> > > >
> > > > Acked-by: Richard Zhu 
> > > > Signed-off-by: Peng Fan 
> > > > Reviewed-by: Mathieu Poirier 
> > > > ---
> > > >  drivers/remoteproc/remoteproc_elf_loader.c | 20
> > > ++--
> > > >  include/linux/remoteproc.h |  4 
> > > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> > > > b/drivers/remoteproc/remoteproc_elf_loader.c
> > > > index df68d87752e4..6cb71fe47261 100644
> > > > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > > > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > > > @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc
> > > > *rproc, const struct firmware *fw)  }
> > > EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> > > >
> > > > +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const
> > > > +void *src, size_t count) {
> > > > + if (!rproc->ops->elf_memcpy)
> > > > + memcpy(dest, src, count);
> > > > +
> > > > + rproc->ops->elf_memcpy(rproc, dest, src, count);
> > >
> > > Looking at the current set of remoteproc drivers I get a feeling that
> > > we'll end up with a while bunch of functions that all just wraps
> > > memcpy_toio(). And the reason for this is that we are we're "abusing"
> > > the carveout to carry the __iomem pointer without keeping track of it.
> > >
> > > And this is not the only time we're supposed to use an io-accessor,
> > > another example is rproc_copy_segment() in rproc_coredump.c
> > >
> > > It also means that if a platform driver for some reason where to
> > > support both ioremap and normal carveouts the elf_memcpy op would be
> > quite quirky.
> > >
> > >
> > > So I would prefer if we track the knowledge about void *va being a
> > > __iomem or not in the struct rproc_mem_entry and make rproc_da_to_va()
> > > return this information as well.
> > >
> > > Then instead of extending the ops we can make this simply call memcpy
> > > or
> > > memcpy_toio() depending on this.
> >
> > A draft proposal as below, are you ok with the approach?
>
> Mathieu, do you have any comments?
>

I will look into this on Monday.


Re: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-10 Thread Bjorn Andersson
On Sun 06 Dec 20:07 CST 2020, Peng Fan wrote:

> Hi Bjorn,
> 
> > Subject: Re: [PATCH V3 1/7] remoteproc: elf: support platform specific
> > memory hook
> > 
> > On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:
> > 
> > > From: Peng Fan 
> > >
> > > To arm64, "dc  zva, dst" is used in memset.
> > > Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> > >
> > > "If the memory region being zeroed is any type of Device memory, this
> > > instruction can give an alignment fault which is prioritized in the
> > > same way as other alignment faults that are determined by the memory
> > > type."
> > >
> > > On i.MX platforms, when elf is loaded to onchip TCM area, the region
> > > is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> > > on i.MX not able to write correct data to TCM area.
> > >
> > > So we need to use io helpers, and extend the elf loader to support
> > > platform specific memory functions.
> > >
> > > Acked-by: Richard Zhu 
> > > Signed-off-by: Peng Fan 
> > > Reviewed-by: Mathieu Poirier 
> > > ---
> > >  drivers/remoteproc/remoteproc_elf_loader.c | 20
> > ++--
> > >  include/linux/remoteproc.h |  4 
> > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> > > b/drivers/remoteproc/remoteproc_elf_loader.c
> > > index df68d87752e4..6cb71fe47261 100644
> > > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > > @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc,
> > > const struct firmware *fw)  }
> > EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> > >
> > > +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const
> > > +void *src, size_t count) {
> > > + if (!rproc->ops->elf_memcpy)
> > > + memcpy(dest, src, count);
> > > +
> > > + rproc->ops->elf_memcpy(rproc, dest, src, count);
> > 
> > Looking at the current set of remoteproc drivers I get a feeling that we'll 
> > end
> > up with a while bunch of functions that all just wraps memcpy_toio(). And 
> > the
> > reason for this is that we are we're "abusing" the carveout to carry the
> > __iomem pointer without keeping track of it.
> > 
> > And this is not the only time we're supposed to use an io-accessor, another
> > example is rproc_copy_segment() in rproc_coredump.c
> > 
> > It also means that if a platform driver for some reason where to support 
> > both
> > ioremap and normal carveouts the elf_memcpy op would be quite quirky.
> > 
> > 
> > So I would prefer if we track the knowledge about void *va being a __iomem
> > or not in the struct rproc_mem_entry and make rproc_da_to_va() return this
> > information as well.
> > 
> > Then instead of extending the ops we can make this simply call memcpy or
> > memcpy_toio() depending on this.
> 
> A draft proposal as below, are you ok with the approach?
> 

Yes, this looks very reasonable and should be directly useful for the
other users of ioremap as well.

May I ask that you rename the boolean "iomem" with "is_iomem", to make
it obvious that it's a boolean in the cases where we're already juggling
physical, virtual and device addresses?

Thanks,
Bjorn

> diff --git a/drivers/remoteproc/remoteproc_core.c 
> b/drivers/remoteproc/remoteproc_core.c
> index 46c2937ebea9..bbb6e0613c1b 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -189,13 +189,13 @@ EXPORT_SYMBOL(rproc_va_to_pa);
>   * here the output of the DMA API for the carveouts, which should be more
>   * correct.
>   */
> -void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
> +void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *iomem)
>  {
> struct rproc_mem_entry *carveout;
> void *ptr = NULL;
> 
> if (rproc->ops->da_to_va) {
> -   ptr = rproc->ops->da_to_va(rproc, da, len);
> +   ptr = rproc->ops->da_to_va(rproc, da, len, iomem);
> if (ptr)
> goto out;
> }
> @@ -217,6 +217,9 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t 
> len)
> 
> ptr = carveout->va + offset;
> 
> +   if (iomem)
> +   

RE: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-09 Thread Peng Fan
> Subject: RE: [PATCH V3 1/7] remoteproc: elf: support platform specific
> memory hook
> 
> Hi Bjorn,
> 
> > Subject: Re: [PATCH V3 1/7] remoteproc: elf: support platform specific
> > memory hook
> >
> > On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:
> >
> > > From: Peng Fan 
> > >
> > > To arm64, "dc  zva, dst" is used in memset.
> > > Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> > >
> > > "If the memory region being zeroed is any type of Device memory,
> > > this instruction can give an alignment fault which is prioritized in
> > > the same way as other alignment faults that are determined by the
> > > memory type."
> > >
> > > On i.MX platforms, when elf is loaded to onchip TCM area, the region
> > > is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> > > on i.MX not able to write correct data to TCM area.
> > >
> > > So we need to use io helpers, and extend the elf loader to support
> > > platform specific memory functions.
> > >
> > > Acked-by: Richard Zhu 
> > > Signed-off-by: Peng Fan 
> > > Reviewed-by: Mathieu Poirier 
> > > ---
> > >  drivers/remoteproc/remoteproc_elf_loader.c | 20
> > ++--
> > >  include/linux/remoteproc.h |  4 
> > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> > > b/drivers/remoteproc/remoteproc_elf_loader.c
> > > index df68d87752e4..6cb71fe47261 100644
> > > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > > @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc
> > > *rproc, const struct firmware *fw)  }
> > EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> > >
> > > +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const
> > > +void *src, size_t count) {
> > > + if (!rproc->ops->elf_memcpy)
> > > + memcpy(dest, src, count);
> > > +
> > > + rproc->ops->elf_memcpy(rproc, dest, src, count);
> >
> > Looking at the current set of remoteproc drivers I get a feeling that
> > we'll end up with a while bunch of functions that all just wraps
> > memcpy_toio(). And the reason for this is that we are we're "abusing"
> > the carveout to carry the __iomem pointer without keeping track of it.
> >
> > And this is not the only time we're supposed to use an io-accessor,
> > another example is rproc_copy_segment() in rproc_coredump.c
> >
> > It also means that if a platform driver for some reason where to
> > support both ioremap and normal carveouts the elf_memcpy op would be
> quite quirky.
> >
> >
> > So I would prefer if we track the knowledge about void *va being a
> > __iomem or not in the struct rproc_mem_entry and make rproc_da_to_va()
> > return this information as well.
> >
> > Then instead of extending the ops we can make this simply call memcpy
> > or
> > memcpy_toio() depending on this.
> 
> A draft proposal as below, are you ok with the approach?

Mathieu, do you have any comments?

Thanks,
Peng.

> 
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 46c2937ebea9..bbb6e0613c1b 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -189,13 +189,13 @@ EXPORT_SYMBOL(rproc_va_to_pa);
>   * here the output of the DMA API for the carveouts, which should be more
>   * correct.
>   */
> -void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
> +void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool
> +*iomem)
>  {
> struct rproc_mem_entry *carveout;
> void *ptr = NULL;
> 
> if (rproc->ops->da_to_va) {
> -   ptr = rproc->ops->da_to_va(rproc, da, len);
> +   ptr = rproc->ops->da_to_va(rproc, da, len, iomem);
> if (ptr)
> goto out;
> }
> @@ -217,6 +217,9 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da,
> size_t len)
> 
> ptr = carveout->va + offset;
> 
> +   if (iomem)
> +   iomem = carveout->iomem;
> +
> break;
> }
> 
> diff --git a/drivers/remoteproc/remoteproc_coredump.c
> b/drivers/remoteproc/remoteproc_coredump.c
> index 34530dc20cb4..5ff9389e6319 

RE: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-06 Thread Peng Fan
Hi Bjorn,

> Subject: Re: [PATCH V3 1/7] remoteproc: elf: support platform specific
> memory hook
> 
> On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:
> 
> > From: Peng Fan 
> >
> > To arm64, "dc  zva, dst" is used in memset.
> > Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> >
> > "If the memory region being zeroed is any type of Device memory, this
> > instruction can give an alignment fault which is prioritized in the
> > same way as other alignment faults that are determined by the memory
> > type."
> >
> > On i.MX platforms, when elf is loaded to onchip TCM area, the region
> > is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> > on i.MX not able to write correct data to TCM area.
> >
> > So we need to use io helpers, and extend the elf loader to support
> > platform specific memory functions.
> >
> > Acked-by: Richard Zhu 
> > Signed-off-by: Peng Fan 
> > Reviewed-by: Mathieu Poirier 
> > ---
> >  drivers/remoteproc/remoteproc_elf_loader.c | 20
> ++--
> >  include/linux/remoteproc.h |  4 
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> > b/drivers/remoteproc/remoteproc_elf_loader.c
> > index df68d87752e4..6cb71fe47261 100644
> > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc,
> > const struct firmware *fw)  }
> EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> >
> > +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const
> > +void *src, size_t count) {
> > +   if (!rproc->ops->elf_memcpy)
> > +   memcpy(dest, src, count);
> > +
> > +   rproc->ops->elf_memcpy(rproc, dest, src, count);
> 
> Looking at the current set of remoteproc drivers I get a feeling that we'll 
> end
> up with a while bunch of functions that all just wraps memcpy_toio(). And the
> reason for this is that we are we're "abusing" the carveout to carry the
> __iomem pointer without keeping track of it.
> 
> And this is not the only time we're supposed to use an io-accessor, another
> example is rproc_copy_segment() in rproc_coredump.c
> 
> It also means that if a platform driver for some reason where to support both
> ioremap and normal carveouts the elf_memcpy op would be quite quirky.
> 
> 
> So I would prefer if we track the knowledge about void *va being a __iomem
> or not in the struct rproc_mem_entry and make rproc_da_to_va() return this
> information as well.
> 
> Then instead of extending the ops we can make this simply call memcpy or
> memcpy_toio() depending on this.

A draft proposal as below, are you ok with the approach?

diff --git a/drivers/remoteproc/remoteproc_core.c 
b/drivers/remoteproc/remoteproc_core.c
index 46c2937ebea9..bbb6e0613c1b 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -189,13 +189,13 @@ EXPORT_SYMBOL(rproc_va_to_pa);
  * here the output of the DMA API for the carveouts, which should be more
  * correct.
  */
-void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *iomem)
 {
struct rproc_mem_entry *carveout;
void *ptr = NULL;

if (rproc->ops->da_to_va) {
-   ptr = rproc->ops->da_to_va(rproc, da, len);
+   ptr = rproc->ops->da_to_va(rproc, da, len, iomem);
if (ptr)
goto out;
}
@@ -217,6 +217,9 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t 
len)

ptr = carveout->va + offset;

+   if (iomem)
+   iomem = carveout->iomem;
+
break;
}

diff --git a/drivers/remoteproc/remoteproc_coredump.c 
b/drivers/remoteproc/remoteproc_coredump.c
index 34530dc20cb4..5ff9389e6319 100644
--- a/drivers/remoteproc/remoteproc_coredump.c
+++ b/drivers/remoteproc/remoteproc_coredump.c
@@ -153,18 +153,22 @@ static void rproc_copy_segment(struct rproc *rproc, void 
*dest,
   size_t offset, size_t size)
 {
void *ptr;
+   bool iomem;

if (segment->dump) {
segment->dump(rproc, segment, dest, offset, size);
} else {
-   ptr = rproc_da_to_va(rproc, segment->da + offset, size);
+   ptr = rproc_da_to_va(rproc, segment->da + offset, size, );
if (!ptr) {
dev_err(>dev,
"invalid c

Re: [PATCH V3 1/7] remoteproc: elf: support platform specific memory hook

2020-12-04 Thread Bjorn Andersson
On Fri 04 Dec 01:40 CST 2020, Peng Fan (OSS) wrote:

> From: Peng Fan 
> 
> To arm64, "dc  zva, dst" is used in memset.
> Per ARM DDI 0487A.j, chapter C5.3.8 DC ZVA, Data Cache Zero by VA,
> 
> "If the memory region being zeroed is any type of Device memory,
> this instruction can give an alignment fault which is prioritized
> in the same way as other alignment faults that are determined
> by the memory type."
> 
> On i.MX platforms, when elf is loaded to onchip TCM area, the region
> is ioremapped, so "dc zva, dst" will trigger abort. And ioremap_wc()
> on i.MX not able to write correct data to TCM area.
> 
> So we need to use io helpers, and extend the elf loader to support
> platform specific memory functions.
> 
> Acked-by: Richard Zhu 
> Signed-off-by: Peng Fan 
> Reviewed-by: Mathieu Poirier 
> ---
>  drivers/remoteproc/remoteproc_elf_loader.c | 20 ++--
>  include/linux/remoteproc.h |  4 
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c 
> b/drivers/remoteproc/remoteproc_elf_loader.c
> index df68d87752e4..6cb71fe47261 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -129,6 +129,22 @@ u64 rproc_elf_get_boot_addr(struct rproc *rproc, const 
> struct firmware *fw)
>  }
>  EXPORT_SYMBOL(rproc_elf_get_boot_addr);
>  
> +static void rproc_elf_memcpy(struct rproc *rproc, void *dest, const void 
> *src, size_t count)
> +{
> + if (!rproc->ops->elf_memcpy)
> + memcpy(dest, src, count);
> +
> + rproc->ops->elf_memcpy(rproc, dest, src, count);

Looking at the current set of remoteproc drivers I get a feeling that
we'll end up with a while bunch of functions that all just wraps
memcpy_toio(). And the reason for this is that we are we're "abusing" the
carveout to carry the __iomem pointer without keeping track of it.

And this is not the only time we're supposed to use an io-accessor,
another example is rproc_copy_segment() in rproc_coredump.c

It also means that if a platform driver for some reason where to support
both ioremap and normal carveouts the elf_memcpy op would be quite
quirky.


So I would prefer if we track the knowledge about void *va being a
__iomem or not in the struct rproc_mem_entry and make rproc_da_to_va()
return this information as well.

Then instead of extending the ops we can make this simply call memcpy or
memcpy_toio() depending on this.

Regards,
Bjorn

> +}
> +
> +static void rproc_elf_memset(struct rproc *rproc, void *s, int c, size_t 
> count)
> +{
> + if (!rproc->ops->elf_memset)
> + memset(s, c, count);
> +
> + rproc->ops->elf_memset(rproc, s, c, count);
> +}
> +
>  /**
>   * rproc_elf_load_segments() - load firmware segments to memory
>   * @rproc: remote processor which will be booted using these fw segments
> @@ -214,7 +230,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const 
> struct firmware *fw)
>  
>   /* put the segment where the remote processor expects it */
>   if (filesz)
> - memcpy(ptr, elf_data + offset, filesz);
> + rproc_elf_memcpy(rproc, ptr, elf_data + offset, filesz);
>  
>   /*
>* Zero out remaining memory for this segment.
> @@ -224,7 +240,7 @@ int rproc_elf_load_segments(struct rproc *rproc, const 
> struct firmware *fw)
>* this.
>*/
>   if (memsz > filesz)
> - memset(ptr + filesz, 0, memsz - filesz);
> + rproc_elf_memset(rproc, ptr + filesz, 0, memsz - 
> filesz);
>   }
>  
>   return ret;
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index e8ac041c64d9..06c52f88a3fd 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -373,6 +373,8 @@ enum rsc_handling_status {
>   *   expects to find it
>   * @sanity_check:sanity check the fw image
>   * @get_boot_addr:   get boot address to entry point specified in firmware
> + * @elf_memcpy:  platform specific elf loader memcpy
> + * @elf_memset:  platform specific elf loader memset
>   * @panic:   optional callback to react to system panic, core will delay
>   *   panic at least the returned number of milliseconds
>   */
> @@ -392,6 +394,8 @@ struct rproc_ops {
>   int (*load)(struct rproc *rproc, const struct firmware *fw);
>   int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
>   u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
> + void (*elf_memcpy)(struct rproc *rproc, void *dest, const void *src, 
> size_t count);
> + void (*elf_memset)(struct rproc *rproc, void *s, int c, size_t count);
>   unsigned long (*panic)(struct rproc *rproc);
>  };
>  
> -- 
> 2.28.0
>