diff mbox series

[4/7] remoteproc: add warning on resource table cast

Message ID 1543526968-56091-5-git-send-email-loic.pallardy@st.com
State New
Headers show
Series remoteproc: Fixes for memoy carveout management | expand

Commit Message

Loic Pallardy Nov. 29, 2018, 9:29 p.m. UTC
Today resource table supports only 32bit address fields.
This is not compliant with 64bit platform for which addresses
are cast in 32bit.
This patch adds warn messages when address cast is done.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

---
 drivers/remoteproc/remoteproc_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
2.7.4

Comments

Arnd Bergmann Nov. 30, 2018, 2:33 p.m. UTC | #1
On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:
>

> Today resource table supports only 32bit address fields.

> This is not compliant with 64bit platform for which addresses

> are cast in 32bit.

> This patch adds warn messages when address cast is done.

>

> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

> ---

>  drivers/remoteproc/remoteproc_core.c | 8 ++++++++

>  1 file changed, 8 insertions(+)

>

> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c

> index 18a1bbf820c9..61c954bd695e 100644

> --- a/drivers/remoteproc/remoteproc_core.c

> +++ b/drivers/remoteproc/remoteproc_core.c

> @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,

>                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",

>                         mem->da, &dma);

>         } else {

> +               /* Update device address as undefined by requester */

> +               if (sizeof(dma_addr_t) > sizeof(u32))

> +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");

> +

>                 mem->da = (u32)dma;

>         }


I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough
to support any machine that the kernel can run on. If you build a kernel that
happens to work on an ARM32 platform with LPAE enabled, then this will
warn every time, even if you are running on a machine that only has a 32-bit
address space.

     Arnd
Loic Pallardy Nov. 30, 2018, 8:48 p.m. UTC | #2
> -----Original Message-----

> From: Arnd Bergmann <arnd@arndb.de>

> Sent: vendredi 30 novembre 2018 15:33

> To: Loic PALLARDY <loic.pallardy@st.com>

> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>; Ohad Ben-Cohen

> <ohad@wizery.com>; linux-remoteproc@vger.kernel.org; Linux Kernel

> Mailing List <linux-kernel@vger.kernel.org>; Arnaud POULIQUEN

> <arnaud.pouliquen@st.com>; Benjamin Gaignard

> <benjamin.gaignard@linaro.org>; Suman Anna <s-anna@ti.com>

> Subject: Re: [PATCH 4/7] remoteproc: add warning on resource table cast

> 

> On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:

> >

> > Today resource table supports only 32bit address fields.

> > This is not compliant with 64bit platform for which addresses

> > are cast in 32bit.

> > This patch adds warn messages when address cast is done.

> >

> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

> > ---

> >  drivers/remoteproc/remoteproc_core.c | 8 ++++++++

> >  1 file changed, 8 insertions(+)

> >

> > diff --git a/drivers/remoteproc/remoteproc_core.c

> b/drivers/remoteproc/remoteproc_core.c

> > index 18a1bbf820c9..61c954bd695e 100644

> > --- a/drivers/remoteproc/remoteproc_core.c

> > +++ b/drivers/remoteproc/remoteproc_core.c

> > @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,

> >                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",

> >                         mem->da, &dma);

> >         } else {

> > +               /* Update device address as undefined by requester */

> > +               if (sizeof(dma_addr_t) > sizeof(u32))

> > +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table

> format\n");

> > +

> >                 mem->da = (u32)dma;

> >         }

> 

> I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough

> to support any machine that the kernel can run on. If you build a kernel that

> happens to work on an ARM32 platform with LPAE enabled, then this will

> warn every time, even if you are running on a machine that only has a 32-bit

> address space.


I suppose if LPAE is enabled, address space is larger than 32bit.
But I agree it is possible to display a warn only when some information will be lost
due to cast.
I'll propose a new version testing dma address itself and displaying warn only if above 4GB.

Regards,
Loic
> 

>      Arnd
diff mbox series

Patch

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 18a1bbf820c9..61c954bd695e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -772,6 +772,10 @@  static int rproc_alloc_carveout(struct rproc *rproc,
 		dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
 			mem->da, &dma);
 	} else {
+		/* Update device address as undefined by requester */
+		if (sizeof(dma_addr_t) > sizeof(u32))
+			dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
+
 		mem->da = (u32)dma;
 	}
 
@@ -1150,6 +1154,10 @@  static int rproc_alloc_registered_carveouts(struct rproc *rproc)
 			 */
 
 			/* Use va if defined else dma to generate pa */
+			if (sizeof(dma_addr_t) > sizeof(u32) ||
+			    sizeof(phys_addr_t) > sizeof(u32))
+				dev_warn(dev, "Physical address cast in 32bit to fit resource table format\n");
+
 			if (entry->va)
 				rsc->pa = (u32)rproc_va_to_pa(entry->va);
 			else