[U-Boot,v2,03/11] riscv: generic: Ensure that U-Boot runs within 4GB for 64bit systems

On 64bit systems, the DRAM top can be easily beyond 4GB and U-Boot
DMA mapping APIs will generate DMA addresses beyond 4GB. This
breaks DMA programming in 32bit DMA capable devices (such as
Cadence MACB ethernet). For example, If DRAM is more then 2GB
on QEMU sifive_u machine then Cadence MACB ethernet stops working
for U-Boot because it is a 32bit DMA capable device.

To handle 32bit DMA capable devices on 64bit systems, we provide
custom implementation of board_get_usable_ram_top() which ensures
that usable ram top is not more then 4GB. This in-turn ensures
that U-Boot always runs within 4GB hence DMA addresses generated
by DMA mapping APIs will be within 4GB too.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Alexander Graf <agraf@suse.de>
---
 arch/riscv/cpu/generic/dram.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

On Fri, 2019-01-18 at 11:18 +0000, Anup Patel wrote:
> On 64bit systems, the DRAM top can be easily beyond 4GB and U-Boot
> DMA mapping APIs will generate DMA addresses beyond 4GB. This
> breaks DMA programming in 32bit DMA capable devices (such as
> Cadence MACB ethernet). For example, If DRAM is more then 2GB
> on QEMU sifive_u machine then Cadence MACB ethernet stops working
> for U-Boot because it is a 32bit DMA capable device.
> 
> To handle 32bit DMA capable devices on 64bit systems, we provide
> custom implementation of board_get_usable_ram_top() which ensures
> that usable ram top is not more then 4GB. This in-turn ensures
> that U-Boot always runs within 4GB hence DMA addresses generated
> by DMA mapping APIs will be within 4GB too.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> Reviewed-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/riscv/cpu/generic/dram.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>

With one nit below.

> diff --git a/arch/riscv/cpu/generic/dram.c
> b/arch/riscv/cpu/generic/dram.c
> index 84d87d2a7f..5725d3c7ae 100644
> --- a/arch/riscv/cpu/generic/dram.c
> +++ b/arch/riscv/cpu/generic/dram.c
> @@ -5,6 +5,9 @@
>  
>  #include <common.h>
>  #include <fdtdec.h>
> +#include <linux/sizes.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>  
>  int dram_init(void)
>  {
> @@ -15,3 +18,20 @@ int dram_init_banksize(void)
>  {
>  	return fdtdec_setup_memory_banksize();
>  }
> +
> +ulong board_get_usable_ram_top(ulong total_size)
> +{
> +#ifdef CONFIG_64BIT
> +	/*
> +	 * Ensure that we run from first 4GB so that all
> +	 * addresses used by U-Boot are 32bit addresses.
> +	 *
> +	 * This in-turn ensures that 32bit DMA capabale

nit: capable

> +	 * devices work fine because DMA mapping APIs will
> +	 * provide 32bit DMA addresses only.
> +	 */
> +	if (gd->ram_top > SZ_4G)
> +		return SZ_4G;
> +#endif
> +	return gd->ram_top;
> +}

> -----Original Message-----
> From: Auer, Lukas [mailto:lukas.auer@aisec.fraunhofer.de]
> Sent: Monday, January 21, 2019 1:35 AM
> To: sjg@chromium.org; bmeng.cn@gmail.com; rick@andestech.com; Anup
> Patel <Anup.Patel@wdc.com>; joe.hershberger@ni.com;
> yamada.masahiro@socionext.com
> Cc: paul.walmsley@sifive.com; palmer@sifive.com; hch@infradead.org; u-
> boot@lists.denx.de; agraf@suse.de; Atish Patra <Atish.Patra@wdc.com>
> Subject: Re: [PATCH v2 03/11] riscv: generic: Ensure that U-Boot runs within
> 4GB for 64bit systems
> 
> On Fri, 2019-01-18 at 11:18 +0000, Anup Patel wrote:
> > On 64bit systems, the DRAM top can be easily beyond 4GB and U-Boot
> DMA
> > mapping APIs will generate DMA addresses beyond 4GB. This breaks DMA
> > programming in 32bit DMA capable devices (such as Cadence MACB
> > ethernet). For example, If DRAM is more then 2GB on QEMU sifive_u
> > machine then Cadence MACB ethernet stops working for U-Boot because
> it
> > is a 32bit DMA capable device.
> >
> > To handle 32bit DMA capable devices on 64bit systems, we provide
> > custom implementation of board_get_usable_ram_top() which ensures
> that
> > usable ram top is not more then 4GB. This in-turn ensures that U-Boot
> > always runs within 4GB hence DMA addresses generated by DMA mapping
> > APIs will be within 4GB too.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > Reviewed-by: Alexander Graf <agraf@suse.de>
> > ---
> >  arch/riscv/cpu/generic/dram.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> 
> Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> 
> With one nit below.
> 
> > diff --git a/arch/riscv/cpu/generic/dram.c
> > b/arch/riscv/cpu/generic/dram.c index 84d87d2a7f..5725d3c7ae 100644
> > --- a/arch/riscv/cpu/generic/dram.c
> > +++ b/arch/riscv/cpu/generic/dram.c
> > @@ -5,6 +5,9 @@
> >
> >  #include <common.h>
> >  #include <fdtdec.h>
> > +#include <linux/sizes.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> >
> >  int dram_init(void)
> >  {
> > @@ -15,3 +18,20 @@ int dram_init_banksize(void)  {
> >  	return fdtdec_setup_memory_banksize();  }
> > +
> > +ulong board_get_usable_ram_top(ulong total_size) { #ifdef
> > +CONFIG_64BIT
> > +	/*
> > +	 * Ensure that we run from first 4GB so that all
> > +	 * addresses used by U-Boot are 32bit addresses.
> > +	 *
> > +	 * This in-turn ensures that 32bit DMA capabale
> 
> nit: capable

Sure, will update.

> 
> > +	 * devices work fine because DMA mapping APIs will
> > +	 * provide 32bit DMA addresses only.
> > +	 */
> > +	if (gd->ram_top > SZ_4G)
> > +		return SZ_4G;
> > +#endif
> > +	return gd->ram_top;
> > +}

Regards,
Anup

On Fri, Jan 18, 2019 at 7:19 PM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> On 64bit systems, the DRAM top can be easily beyond 4GB and U-Boot
> DMA mapping APIs will generate DMA addresses beyond 4GB. This
> breaks DMA programming in 32bit DMA capable devices (such as
> Cadence MACB ethernet). For example, If DRAM is more then 2GB
> on QEMU sifive_u machine then Cadence MACB ethernet stops working
> for U-Boot because it is a 32bit DMA capable device.
>
> To handle 32bit DMA capable devices on 64bit systems, we provide
> custom implementation of board_get_usable_ram_top() which ensures
> that usable ram top is not more then 4GB. This in-turn ensures
> that U-Boot always runs within 4GB hence DMA addresses generated
> by DMA mapping APIs will be within 4GB too.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> Reviewed-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/riscv/cpu/generic/dram.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

But see one comment below:

> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> index 84d87d2a7f..5725d3c7ae 100644
> --- a/arch/riscv/cpu/generic/dram.c
> +++ b/arch/riscv/cpu/generic/dram.c
> @@ -5,6 +5,9 @@
>
>  #include <common.h>
>  #include <fdtdec.h>
> +#include <linux/sizes.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>  int dram_init(void)
>  {
> @@ -15,3 +18,20 @@ int dram_init_banksize(void)
>  {
>         return fdtdec_setup_memory_banksize();
>  }
> +
> +ulong board_get_usable_ram_top(ulong total_size)
> +{
> +#ifdef CONFIG_64BIT
> +       /*
> +        * Ensure that we run from first 4GB so that all
> +        * addresses used by U-Boot are 32bit addresses.
> +        *
> +        * This in-turn ensures that 32bit DMA capabale
> +        * devices work fine because DMA mapping APIs will
> +        * provide 32bit DMA addresses only.
> +        */
> +       if (gd->ram_top > SZ_4G)
> +               return SZ_4G;
> +#endif
> +       return gd->ram_top;
> +}

I was wondering whether we should change this for 32-bit too, given
it's a valid configuration to have more than 4GB memory in a 32-bit
system.

Regards,
Bin

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn@gmail.com]
> Sent: Tuesday, January 22, 2019 7:37 AM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Rick Chen <rick@andestech.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Lukas Auer <lukas.auer@aisec.fraunhofer.de>;
> Masahiro Yamada <yamada.masahiro@socionext.com>; Simon Glass
> <sjg@chromium.org>; Alexander Graf <agraf@suse.de>; Palmer Dabbelt
> <palmer@sifive.com>; Paul Walmsley <paul.walmsley@sifive.com>; Atish
> Patra <Atish.Patra@wdc.com>; Christoph Hellwig <hch@infradead.org>; U-
> Boot Mailing List <u-boot@lists.denx.de>
> Subject: Re: [PATCH v2 03/11] riscv: generic: Ensure that U-Boot runs within
> 4GB for 64bit systems
> 
> On Fri, Jan 18, 2019 at 7:19 PM Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > On 64bit systems, the DRAM top can be easily beyond 4GB and U-Boot
> DMA
> > mapping APIs will generate DMA addresses beyond 4GB. This breaks DMA
> > programming in 32bit DMA capable devices (such as Cadence MACB
> > ethernet). For example, If DRAM is more then 2GB on QEMU sifive_u
> > machine then Cadence MACB ethernet stops working for U-Boot because
> it
> > is a 32bit DMA capable device.
> >
> > To handle 32bit DMA capable devices on 64bit systems, we provide
> > custom implementation of board_get_usable_ram_top() which ensures
> that
> > usable ram top is not more then 4GB. This in-turn ensures that U-Boot
> > always runs within 4GB hence DMA addresses generated by DMA mapping
> > APIs will be within 4GB too.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > Reviewed-by: Alexander Graf <agraf@suse.de>
> > ---
> >  arch/riscv/cpu/generic/dram.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> But see one comment below:
> 
> > diff --git a/arch/riscv/cpu/generic/dram.c
> > b/arch/riscv/cpu/generic/dram.c index 84d87d2a7f..5725d3c7ae 100644
> > --- a/arch/riscv/cpu/generic/dram.c
> > +++ b/arch/riscv/cpu/generic/dram.c
> > @@ -5,6 +5,9 @@
> >
> >  #include <common.h>
> >  #include <fdtdec.h>
> > +#include <linux/sizes.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> >
> >  int dram_init(void)
> >  {
> > @@ -15,3 +18,20 @@ int dram_init_banksize(void)  {
> >         return fdtdec_setup_memory_banksize();  }
> > +
> > +ulong board_get_usable_ram_top(ulong total_size) { #ifdef
> > +CONFIG_64BIT
> > +       /*
> > +        * Ensure that we run from first 4GB so that all
> > +        * addresses used by U-Boot are 32bit addresses.
> > +        *
> > +        * This in-turn ensures that 32bit DMA capabale
> > +        * devices work fine because DMA mapping APIs will
> > +        * provide 32bit DMA addresses only.
> > +        */
> > +       if (gd->ram_top > SZ_4G)
> > +               return SZ_4G;
> > +#endif
> > +       return gd->ram_top;
> > +}
> 
> I was wondering whether we should change this for 32-bit too, given it's a
> valid configuration to have more than 4GB memory in a 32-bit system.

We had considered this but "gd->ram_top" is "unsigned long" so on
32bit system it will be 32bit only. In other words, value of "gd->ram_top"
on 32bit system can never exceed 4GB.

Regards,
Anup