[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[Christoph Muellner]

From: Christoph Müllner <christoph.muellner@theobroma-systems.com>

Some machines have limited DMA engines, which cannot deal
with arbitrary addresses. This patch introduces a function
to model these restrictions on a machine level.

Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

Changes in v2: None

 common/board_f.c | 5 +++++
 include/init.h   | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/common/board_f.c b/common/board_f.c
index 7ef20f2042..92dc34d2d8 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -1053,3 +1053,8 @@ void board_init_f_r(void)
 	hang();
 }
 #endif /* CONFIG_X86 */
+
+__weak int mach_addr_is_dmaable(void __iomem *ptr)
+{
+	return 1;
+}
diff --git a/include/init.h b/include/init.h
index afc953d51e..1653d5086f 100644
--- a/include/init.h
+++ b/include/init.h
@@ -125,6 +125,8 @@ int misc_init_f(void);
 int embedded_dtb_select(void);
 #endif
 
+int mach_addr_is_dmaable(void __iomem *ptr);
+
 /* common/init/board_init.c */
 extern ulong monitor_flash_len;
 
-- 
2.11.0

[U-Boot] [PATCH v2 2/3] bouncebuf: Add DMA validation check to addr_aligned().

[Christoph Muellner]

From: Christoph Müllner <christoph.muellner@theobroma-systems.com>

Currently addr_aligned() performs an alignment and a length check
to validate the DMA address. However, some machines have stricter
restrictions of DMA-able addresses.

This patch adds a call to mach_addr_is_dmaable() to honor this
machine specific restrictions.

Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

Changes in v2: None

 common/bouncebuf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/bouncebuf.c b/common/bouncebuf.c
index a7098e2caf..1b82243b06 100644
--- a/common/bouncebuf.c
+++ b/common/bouncebuf.c
@@ -26,6 +26,12 @@ static int addr_aligned(struct bounce_buffer *state)
 		return 0;
 	}
 
+	/* Check if valid DMA address. */
+	if (!mach_addr_is_dmaable(state->user_buffer)) {
+		debug("Buffer address is not DMA-able\n");
+		return 0;
+	}
+
 	/* Aligned */
 	return 1;
 }
-- 
2.11.0

[U-Boot] [PATCH v2 3/3] rk3399: Add restriction for DMA-able addresses.

[Christoph Muellner]

From: Christoph Müllner <christoph.muellner@theobroma-systems.com>

Patches on the U-Boot mailing list from Rockchip engineers
indicate, that the RK3399's DMA engines are not able to use
addresses in high-memory (above 0xf8000000).

This patch models this restriction in an RK3399 specific
mach_addr_is_dmaable() function.

Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

Changes in v2:
    - Convert argument type from unsigned long to void pointer.

 arch/arm/mach-rockchip/rk3399/rk3399.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
index a7ccd4f3ed..8895417bda 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -109,3 +109,16 @@ void board_debug_uart_init(void)
 #endif
 }
 #endif
+
+int mach_addr_is_dmaable(void __iomem *ptr)
+{
+	uintptr_t addr = (uintptr_t)ptr;
+
+	/*
+	 * The RK3399 cannot cope with high-memory DMA targets/sources.
+	 */
+	if (addr < 0xf8000000UL)
+		return 1;
+
+	return 0;
+}
-- 
2.11.0

[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[Simon Glass]

Hi Christoph,

On Tue, 7 May 2019 at 09:59, Christoph Muellner
<christoph.muellner@theobroma-systems.com> wrote:
>
> From: Christoph Müllner <christoph.muellner@theobroma-systems.com>
>
> Some machines have limited DMA engines, which cannot deal
> with arbitrary addresses. This patch introduces a function
> to model these restrictions on a machine level.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>
> Changes in v2: None
>
>  common/board_f.c | 5 +++++
>  include/init.h   | 2 ++
>  2 files changed, 7 insertions(+)
>

Can we handle this with driver model somehow? How does the kernel
handle it? Is there a device-tree binding for the DMA node that could
provide this information.

Also, where is this function called from?

Regards,
Simon

[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[Heiko Stuebner]

Hi Simon,

Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass:
> On Tue, 7 May 2019 at 09:59, Christoph Muellner
> <christoph.muellner@theobroma-systems.com> wrote:
> >
> > From: Christoph Müllner <christoph.muellner@theobroma-systems.com>
> >
> > Some machines have limited DMA engines, which cannot deal
> > with arbitrary addresses. This patch introduces a function
> > to model these restrictions on a machine level.
> >
> > Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > ---
> >
> > Changes in v2: None
> >
> >  common/board_f.c | 5 +++++
> >  include/init.h   | 2 ++
> >  2 files changed, 7 insertions(+)
> >
> 
> Can we handle this with driver model somehow? How does the kernel
> handle it?

The problem Christoph is trying to solve here is doing a mmc transfer
from mmc to the sram (not main memory), which cannot use dma.
So this exact problem doesn't happen in the kernel itself.

But back in veyron times we had a similar dma issue with anything accessing
that area as dma hung the system, but the solution was just reserving the
memblock:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9

As the commit describes, other options would've been soc-settings
also going around the kernel's driver model or limiting the dma-able
memory even more (to 2GB), so we opted to just reserve the upper 16MB
completely.


> Is there a device-tree binding for the DMA node that could
> provide this information.

I don't think so. At least in the kernel affecting the dma-mask is a
setting for the using component (mmc, gmac, whatever), so would
mean adapting every device doing dma ... and even then there wasn't
a dt-binding for something like that, which in turn would've required
to adapt every driver to set a specific per-soc dma-mask for Rockchip
compatibles - hence the "simple" reserve above was the least intrusive
option.


> Also, where is this function called from?

In the theobroma u-boot it gets called from the bouncebuffer right now
	https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96
to check if the mmc drivers can dma directly or needs to use the
bouncebuffer for reaching something like the sram.


Heiko

[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[Simon Glass]

Hi Heiko,

On Wed, 22 May 2019 at 05:29, Heiko Stuebner <heiko@sntech.de> wrote:
>
> Hi Simon,
>
> Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass:
> > On Tue, 7 May 2019 at 09:59, Christoph Muellner
> > <christoph.muellner@theobroma-systems.com> wrote:
> > >
> > > From: Christoph Müllner <christoph.muellner@theobroma-systems.com>
> > >
> > > Some machines have limited DMA engines, which cannot deal
> > > with arbitrary addresses. This patch introduces a function
> > > to model these restrictions on a machine level.
> > >
> > > Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
> > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> > > ---
> > >
> > > Changes in v2: None
> > >
> > >  common/board_f.c | 5 +++++
> > >  include/init.h   | 2 ++
> > >  2 files changed, 7 insertions(+)
> > >
> >
> > Can we handle this with driver model somehow? How does the kernel
> > handle it?
>
> The problem Christoph is trying to solve here is doing a mmc transfer
> from mmc to the sram (not main memory), which cannot use dma.
> So this exact problem doesn't happen in the kernel itself.
>
> But back in veyron times we had a similar dma issue with anything accessing
> that area as dma hung the system, but the solution was just reserving the
> memblock:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9
>
> As the commit describes, other options would've been soc-settings
> also going around the kernel's driver model or limiting the dma-able
> memory even more (to 2GB), so we opted to just reserve the upper 16MB
> completely.

OK I see.

>
>
> > Is there a device-tree binding for the DMA node that could
> > provide this information.
>
> I don't think so. At least in the kernel affecting the dma-mask is a
> setting for the using component (mmc, gmac, whatever), so would
> mean adapting every device doing dma ... and even then there wasn't
> a dt-binding for something like that, which in turn would've required
> to adapt every driver to set a specific per-soc dma-mask for Rockchip
> compatibles - hence the "simple" reserve above was the least intrusive
> option.

That's odd. Are you saying that some devices can DMA from SRAM and some cannot?

If the DMA is not allowed, could the DMA driver return -EPERM or
similar when the attempt is made, and then the bounce buffer can be
used if needed?

>
>
> > Also, where is this function called from?
>
> In the theobroma u-boot it gets called from the bouncebuffer right now
>         https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96
> to check if the mmc drivers can dma directly or needs to use the
> bouncebuffer for reaching something like the sram.

OK ta.

>
>
> Heiko
>
>

Regards,
Simon

[U-Boot] [PATCH v2 1/3] board_f: Add mach specific DMA address check function.

[Christoph Müllner]

On 23.05.19 00:57, Simon Glass wrote:
> Hi Heiko,
> 
> On Wed, 22 May 2019 at 05:29, Heiko Stuebner <heiko@sntech.de> wrote:
>>
>> Hi Simon,
>>
>> Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass:
>>> On Tue, 7 May 2019 at 09:59, Christoph Muellner
>>> <christoph.muellner@theobroma-systems.com> wrote:
>>>>
>>>> From: Christoph Müllner <christoph.muellner@theobroma-systems.com>
>>>>
>>>> Some machines have limited DMA engines, which cannot deal
>>>> with arbitrary addresses. This patch introduces a function
>>>> to model these restrictions on a machine level.
>>>>
>>>> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>> ---
>>>>
>>>> Changes in v2: None
>>>>
>>>>  common/board_f.c | 5 +++++
>>>>  include/init.h   | 2 ++
>>>>  2 files changed, 7 insertions(+)
>>>>
>>>
>>> Can we handle this with driver model somehow? How does the kernel
>>> handle it?
>>
>> The problem Christoph is trying to solve here is doing a mmc transfer
>> from mmc to the sram (not main memory), which cannot use dma.
>> So this exact problem doesn't happen in the kernel itself.
>>
>> But back in veyron times we had a similar dma issue with anything accessing
>> that area as dma hung the system, but the solution was just reserving the
>> memblock:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9
>>
>> As the commit describes, other options would've been soc-settings
>> also going around the kernel's driver model or limiting the dma-able
>> memory even more (to 2GB), so we opted to just reserve the upper 16MB
>> completely.
> 
> OK I see.
> 
>>
>>
>>> Is there a device-tree binding for the DMA node that could
>>> provide this information.
>>
>> I don't think so. At least in the kernel affecting the dma-mask is a
>> setting for the using component (mmc, gmac, whatever), so would
>> mean adapting every device doing dma ... and even then there wasn't
>> a dt-binding for something like that, which in turn would've required
>> to adapt every driver to set a specific per-soc dma-mask for Rockchip
>> compatibles - hence the "simple" reserve above was the least intrusive
>> option.
> 
> That's odd. Are you saying that some devices can DMA from SRAM and some cannot?

Yes, that is also confirmed by Kever (see commit message of [1]).
My patch series assumes, that only the CPU can see the non-DDR memory areas
(including SRAM). I would really like to get feedback here from Kever.

Kever has posted a first patch to address this end of March [2].
After waiting a few weeks, I've prepared a reworked version
(because I needed it for mainline ATF testing on our RK3399-Q7).
A few hours after I sent them to the list Kever also pushed
another patch for this [1].

From my perspective mainline ATF cannot be used with mainline U-Boot
on RK3399 when booting from SD card or eMMC at the moment.

[1] https://patchwork.ozlabs.org/patch/1096366/
[2] https://patchwork.ozlabs.org/patch/1069730/

> 
> If the DMA is not allowed, could the DMA driver return -EPERM or
> similar when the attempt is made, and then the bounce buffer can be
> used if needed?
> 
>>
>>
>>> Also, where is this function called from?
>>
>> In the theobroma u-boot it gets called from the bouncebuffer right now
>>         https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96
>> to check if the mmc drivers can dma directly or needs to use the
>> bouncebuffer for reaching something like the sram.
> 
> OK ta.
> 
>>
>>
>> Heiko
>>
>>
> 
> Regards,
> Simon
>