[U-Boot] [PATCH v2 0/6] Add 3GiB DRAM support to 64-bit Allwinner SoCs

[U-Boot] [PATCH v2 0/6] Add 3GiB DRAM support to 64-bit Allwinner SoCs

[Icenowy Zheng]

Allwinner 64-bit SoCs have allocated 3GiB space in the memory map for
DRAM. If memory bigger than 3GiB is installed (as memory usually come as
pow of 2 and they are not known to support 3GiB LPDDR3 modules, it means
4GiB memory is installed), the whole 3GiB space can be all used.

However, in many situations we still only defined 2GiB for the DRAM.

Add support for 3GiB DRAM. Tested on a customized Pine A64-LTS with 4GiB
LPDDR3 memory installed.

Icenowy Zheng (6):
  sunxi: map DRAM part with 3G size on AArch64
  sunxi: add Kconfig option for the maximum accessible DRAM
  sunxi: let sunxi_dram_init return unsigned long long
  sunxi: restrict the ram_size to the accessible range in SPL
  sunxi: add code for recalculating the DRAM size in U-Boot
  sunxi: add size recalculation code for common DesignWare DRAM driver

 arch/arm/include/asm/arch-sunxi/dram.h |  5 ++++-
 arch/arm/mach-sunxi/Kconfig            | 15 +++++++++++++++
 arch/arm/mach-sunxi/Makefile           |  2 +-
 arch/arm/mach-sunxi/board.c            |  2 +-
 arch/arm/mach-sunxi/dram_sun6i.c       |  2 +-
 arch/arm/mach-sunxi/dram_sun8i_a23.c   |  2 +-
 arch/arm/mach-sunxi/dram_sun8i_a33.c   |  2 +-
 arch/arm/mach-sunxi/dram_sun8i_a83t.c  |  2 +-
 arch/arm/mach-sunxi/dram_sun9i.c       |  4 ++--
 arch/arm/mach-sunxi/dram_sunxi_dw.c    | 30 +++++++++++++++++++++++++++---
 board/sunxi/board.c                    | 21 +++++++++++++++++++--
 board/sunxi/dram_sun4i_auto.c          |  2 +-
 board/sunxi/dram_sun5i_auto.c          |  2 +-
 13 files changed, 75 insertions(+), 16 deletions(-)

-- 
2.15.1

[U-Boot] [PATCH v2 1/6] sunxi: map DRAM part with 3G size on AArch64

[Icenowy Zheng]

All Allwinner 64-bit SoCs now are known to be able to access 3GiB of
external DRAM, however the size of DRAM part in the MMU translation
table is still 2GiB.

Change the size of DRAM part in MMU table to 3GiB.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
No changes in v2.

 arch/arm/mach-sunxi/board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 1753faec1d..89696e0890 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -53,7 +53,7 @@ static struct mm_region sunxi_mem_map[] = {
 		/* RAM */
 		.virt = 0x40000000UL,
 		.phys = 0x40000000UL,
-		.size = 0x80000000UL,
+		.size = 0xC0000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {
-- 
2.15.1

[U-Boot] [PATCH v2 2/6] sunxi: add Kconfig option for the maximum accessible DRAM

[Icenowy Zheng]

Allwinner 64-bit SoCs can use 4GiB DRAM chip, however their memory map
has only allocated 3GiB for DRAM, so only 3GiB of the DRAM is
accessible.

Add a Kconfig option for the maximum accessible DRAM.

For A80 it should be a much higher value (8GiB), but as I have no A80
device to test and originally U-Boot only supports 2GiB DRAM on A80, it
currently still falls under the 2GiB situation.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
No changes in v2.

 arch/arm/mach-sunxi/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index b868f0e350..029821c82d 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -130,6 +130,13 @@ config MACH_SUNXI_H3_H5
 	select SUNXI_GEN_SUN6I
 	select SUPPORT_SPL
 
+# TODO: try out A80's 8GiB DRAM space
+config SUNXI_DRAM_MAX_SIZE
+	hex
+	default 0xC0000000 if MACH_SUN50I
+	default 0xC0000000 if MACH_SUN50I_H5
+	default 0x80000000
+
 choice
 	prompt "Sunxi SoC Variant"
 	optional
-- 
2.15.1

[U-Boot] [PATCH v2 3/6] sunxi: let sunxi_dram_init return unsigned long long

[Icenowy Zheng]

As 4GiB capacity is above the range of 32-bit unsigned integer, raise
the return type of sunxi_dram_init() to unsigned long long, thus it can
hold 4GiB capacity (or maybe more on A80).

Some controllers that are possible to use 4GiB+ memory module are
also changed to calculate its memory capacity in unsigned long long.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
No changes in v2.

 arch/arm/include/asm/arch-sunxi/dram.h | 2 +-
 arch/arm/mach-sunxi/dram_sun6i.c       | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a23.c   | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a33.c   | 2 +-
 arch/arm/mach-sunxi/dram_sun8i_a83t.c  | 2 +-
 arch/arm/mach-sunxi/dram_sun9i.c       | 4 ++--
 arch/arm/mach-sunxi/dram_sunxi_dw.c    | 4 ++--
 board/sunxi/board.c                    | 2 +-
 board/sunxi/dram_sun4i_auto.c          | 2 +-
 board/sunxi/dram_sun5i_auto.c          | 2 +-
 10 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h
index 80abac95b8..d08b82371d 100644
--- a/arch/arm/include/asm/arch-sunxi/dram.h
+++ b/arch/arm/include/asm/arch-sunxi/dram.h
@@ -32,7 +32,7 @@
 #include <asm/arch/dram_sun4i.h>
 #endif
 
-unsigned long sunxi_dram_init(void);
+unsigned long long sunxi_dram_init(void);
 void mctl_await_completion(u32 *reg, u32 mask, u32 val);
 bool mctl_mem_matches(u32 offset);
 
diff --git a/arch/arm/mach-sunxi/dram_sun6i.c b/arch/arm/mach-sunxi/dram_sun6i.c
index 5dbbf6186f..bdf52a2c38 100644
--- a/arch/arm/mach-sunxi/dram_sun6i.c
+++ b/arch/arm/mach-sunxi/dram_sun6i.c
@@ -326,7 +326,7 @@ static void mctl_port_cfg(void)
 	writel(0x00000307, &mctl_com->mbagcr[5]);
 }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 		(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a23.c b/arch/arm/mach-sunxi/dram_sun8i_a23.c
index c53671a0e9..169ccff41a 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a23.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a23.c
@@ -264,7 +264,7 @@ static void mctl_init(u32 *bus_width)
 	writel(0x00000000, &mctl_ctl->rfshctl3);
 }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 		(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c
index fa1620cb39..dfbbe6f39c 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a33.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c
@@ -325,7 +325,7 @@ static void mctl_sys_init(struct dram_para *para)
 	udelay(250);
 }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a83t.c b/arch/arm/mach-sunxi/dram_sun8i_a83t.c
index 55df1b9d54..ec4bccd635 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_a83t.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_a83t.c
@@ -423,7 +423,7 @@ static void mctl_sys_init(struct dram_para *para)
 	udelay(250);
 }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
diff --git a/arch/arm/mach-sunxi/dram_sun9i.c b/arch/arm/mach-sunxi/dram_sun9i.c
index 8c681f3541..dcb20f763e 100644
--- a/arch/arm/mach-sunxi/dram_sun9i.c
+++ b/arch/arm/mach-sunxi/dram_sun9i.c
@@ -854,7 +854,7 @@ signed int DRAMC_get_dram_size(void)
 	return 1 << dram_size;
 }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 		(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -957,5 +957,5 @@ unsigned long sunxi_dram_init(void)
 	mctl_com_init(&para);
 
 	/* return the proper RAM size */
-	return DRAMC_get_dram_size() << 20;
+	return ((unsigned long long)DRAMC_get_dram_size()) << 20;
 }
diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c
index 78b4ffb9c3..3bff1c46cd 100644
--- a/arch/arm/mach-sunxi/dram_sunxi_dw.c
+++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
@@ -682,7 +682,7 @@ static void mctl_auto_detect_dram_size(uint16_t socid, struct dram_para *para)
 	   3,  3,  3,  3,  3,  3,  3,  3,			\
 	   3,  3,  3,  3,  2,  0,  0      }
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
@@ -763,6 +763,6 @@ unsigned long sunxi_dram_init(void)
 	mctl_auto_detect_dram_size(socid, &para);
 	mctl_set_cr(socid, &para);
 
-	return (1UL << (para.row_bits + para.bank_bits)) * para.page_size *
+	return (1ULL << (para.row_bits + para.bank_bits)) * para.page_size *
 	       (para.dual_rank ? 2 : 1);
 }
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index e08e22f30c..1c5e1f380a 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -578,7 +578,7 @@ void sunxi_board_init(void)
 #endif
 #endif
 	printf("DRAM:");
-	gd->ram_size = sunxi_dram_init();
+	gd->ram_size = (phys_size_t)sunxi_dram_init();
 	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
 	if (!gd->ram_size)
 		hang();
diff --git a/board/sunxi/dram_sun4i_auto.c b/board/sunxi/dram_sun4i_auto.c
index 7d4409b51e..293c968f6b 100644
--- a/board/sunxi/dram_sun4i_auto.c
+++ b/board/sunxi/dram_sun4i_auto.c
@@ -29,7 +29,7 @@ static struct dram_para dram_para = {
 	.dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
 };
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	return dramc_init(&dram_para);
 }
diff --git a/board/sunxi/dram_sun5i_auto.c b/board/sunxi/dram_sun5i_auto.c
index e3fa243267..02e29b215f 100644
--- a/board/sunxi/dram_sun5i_auto.c
+++ b/board/sunxi/dram_sun5i_auto.c
@@ -32,7 +32,7 @@ static struct dram_para dram_para = {
 	.dqs_gating_delay = CONFIG_DRAM_DQS_GATING_DELAY,
 };
 
-unsigned long sunxi_dram_init(void)
+unsigned long long sunxi_dram_init(void)
 {
 	return dramc_init(&dram_para);
 }
-- 
2.15.1

[U-Boot] [PATCH v2 4/6] sunxi: restrict the ram_size to the accessible range in SPL

[Icenowy Zheng]

On newer Allwinner SoCs with the BROM start at 0x0 and the DRAM space at
<0x40000000 0xc0000000>, some parts of DRAM will be inaccessible when
4GiB module is used.

Restrict the ram_size written to global_data in SPL.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
No changes in v2.

 board/sunxi/board.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 1c5e1f380a..73dd41437d 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -518,6 +518,7 @@ int board_mmc_init(bd_t *bis)
 void sunxi_board_init(void)
 {
 	int power_failed = 0;
+	unsigned long long dram_real_size;
 
 #ifdef CONFIG_SY8106A_POWER
 	power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT);
@@ -578,8 +579,16 @@ void sunxi_board_init(void)
 #endif
 #endif
 	printf("DRAM:");
-	gd->ram_size = (phys_size_t)sunxi_dram_init();
-	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
+	dram_real_size = sunxi_dram_init();
+	printf(" %d MiB", (int)(dram_real_size >> 20));
+	if (dram_real_size > CONFIG_SUNXI_DRAM_MAX_SIZE) {
+		gd->ram_size = CONFIG_SUNXI_DRAM_MAX_SIZE;
+		printf(", %d MiB usable\n", (int)(gd->ram_size >> 20));
+	} else {
+		gd->ram_size = (phys_size_t)dram_real_size;
+		printf("\n");
+	}
+
 	if (!gd->ram_size)
 		hang();
 
-- 
2.15.1

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

The get_ram_size() function in U-Boot can only deal with memory size
smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit
SoCs, an alternative way to detect DRAM size is needed.

Add the possibility to use some DRAM size recalculating code in DRAM
driver.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
New patch in v2.

 arch/arm/include/asm/arch-sunxi/dram.h | 3 +++
 arch/arm/mach-sunxi/Kconfig            | 7 +++++++
 board/sunxi/board.c                    | 8 ++++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h
index d08b82371d..7443fc3507 100644
--- a/arch/arm/include/asm/arch-sunxi/dram.h
+++ b/arch/arm/include/asm/arch-sunxi/dram.h
@@ -35,5 +35,8 @@
 unsigned long long sunxi_dram_init(void);
 void mctl_await_completion(u32 *reg, u32 mask, u32 val);
 bool mctl_mem_matches(u32 offset);
+#if defined(CONFIG_DRAM_CAN_RECALCULATE_SIZE)
+unsigned long long sunxi_dram_recalculate_size(void);
+#endif
 
 #endif /* _SUNXI_DRAM_H */
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 029821c82d..b206472ead 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -6,6 +6,13 @@ config SPL_LDSCRIPT
 config IDENT_STRING
 	default " Allwinner Technology"
 
+config DRAM_CAN_RECALCULATE_SIZE
+	bool
+	help
+	  Select this if the DRAM controller driver is capable of
+	  re-calculating the size in main U-Boot. It's usable for
+	  size bigger than 2GiB.
+
 config DRAM_SUN4I
 	bool
 	help
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 73dd41437d..96192a7ec3 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -256,7 +256,16 @@ int board_init(void)
 
 int dram_init(void)
 {
+#ifndef CONFIG_DRAM_CAN_RECALCULATE_SIZE
 	gd->ram_size = get_ram_size((long *)PHYS_SDRAM_0, PHYS_SDRAM_0_SIZE);
+#else
+	unsigned long long real_dram_size = sunxi_dram_recalculate_size();
+
+	if (real_dram_size > CONFIG_SUNXI_DRAM_MAX_SIZE)
+		gd->ram_size = CONFIG_SUNXI_DRAM_MAX_SIZE;
+	else
+		gd->ram_size = (phys_size_t)real_dram_size;
+#endif
 
 	return 0;
 }
-- 
2.15.1

[U-Boot] [PATCH v2 6/6] sunxi: add size recalculation code for common DesignWare DRAM driver

[Icenowy Zheng]

The Allwinner A64 and H5 SoCs are both capable of use 3GiB capacity of
4GiB modules, and they both use the common DesignWare DRAM driver code.

Add size recalculation code for the driver.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
New patch in v2.

 arch/arm/mach-sunxi/Kconfig         |  1 +
 arch/arm/mach-sunxi/Makefile        |  2 +-
 arch/arm/mach-sunxi/dram_sunxi_dw.c | 28 ++++++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index b206472ead..0cdc34ea9a 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -108,6 +108,7 @@ config SUNXI_GEN_SUN6I
 
 config SUNXI_DRAM_DW
 	bool
+	select DRAM_CAN_RECALCULATE_SIZE
 	---help---
 	Select this for sunxi SoCs which uses a DRAM controller like the
 	DesignWare controller used in H3, mainly SoCs after H3, which do
diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 183175340a..b403045272 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_MACH_SUN8I)	+= clock_sun6i.o
 endif
 obj-$(CONFIG_MACH_SUN9I)	+= clock_sun9i.o gtbus_sun9i.o
 
+obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_DRAM_SUN4I)	+= dram_sun4i.o
 obj-$(CONFIG_DRAM_SUN6I)	+= dram_sun6i.o
@@ -40,6 +41,5 @@ obj-$(CONFIG_DRAM_SUN8I_A33)	+= dram_sun8i_a33.o
 obj-$(CONFIG_DRAM_SUN8I_A83T)	+= dram_sun8i_a83t.o
 obj-$(CONFIG_DRAM_SUN9I)	+= dram_sun9i.o
 obj-$(CONFIG_SPL_SPI_SUNXI)	+= spl_spi_sunxi.o
-obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_sunxi_dw.o
 obj-$(CONFIG_SUNXI_DRAM_DW)	+= dram_timings/
 endif
diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c
index 3bff1c46cd..f0232e63e7 100644
--- a/arch/arm/mach-sunxi/dram_sunxi_dw.c
+++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
@@ -16,6 +16,30 @@
 #include <asm/arch/cpu.h>
 #include <linux/kconfig.h>
 
+unsigned long long sunxi_dram_recalculate_size(void)
+{
+	struct sunxi_mctl_com_reg * const mctl_com =
+			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
+	u32 cr_reg = readl(&mctl_com->cr);
+	u32 temp;
+	unsigned int dram_size_shift;
+
+	temp = (cr_reg >> 8) & 0xf;	/* page size code */
+	dram_size_shift = (temp + 3);
+
+	temp = (cr_reg >> 4) & 0xf;	/* row width code */
+	dram_size_shift += (temp + 1);
+
+	temp = (cr_reg >> 2) & 0x3;	/* bank number code */
+	dram_size_shift += (temp + 2);
+
+	temp = cr_reg & 0x3;		/* rank number code */
+	dram_size_shift += temp;
+
+	return 1ULL << dram_size_shift;
+}
+
+#ifdef CONFIG_SPL_BUILD
 static void mctl_phy_init(u32 val)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
@@ -763,6 +787,6 @@ unsigned long long sunxi_dram_init(void)
 	mctl_auto_detect_dram_size(socid, &para);
 	mctl_set_cr(socid, &para);
 
-	return (1ULL << (para.row_bits + para.bank_bits)) * para.page_size *
-	       (para.dual_rank ? 2 : 1);
+	return sunxi_dram_recalculate_size();
 }
+#endif
-- 
2.15.1

[U-Boot] [PATCH v2 2/6] sunxi: add Kconfig option for the maximum accessible DRAM

[Maxime Ripard]

On Fri, Mar 23, 2018 at 04:18:53PM +0800, Icenowy Zheng wrote:
> Allwinner 64-bit SoCs can use 4GiB DRAM chip, however their memory map
> has only allocated 3GiB for DRAM, so only 3GiB of the DRAM is
> accessible.

How would that work in Linux?

> Add a Kconfig option for the maximum accessible DRAM.
> 
> For A80 it should be a much higher value (8GiB), but as I have no A80
> device to test and originally U-Boot only supports 2GiB DRAM on A80, it
> currently still falls under the 2GiB situation.

I guess we have LPAE on armv7 to deal with this nicely.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
> The get_ram_size() function in U-Boot can only deal with memory size
> smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit
> SoCs, an alternative way to detect DRAM size is needed.

Why not just fixing get_ram_size then?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
>> The get_ram_size() function in U-Boot can only deal with memory size
>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit
>> SoCs, an alternative way to detect DRAM size is needed.
>
>Why not just fixing get_ram_size then?

Even if it's fixed it won't support 3GiB DRAM at all.

>
>Maxime

[U-Boot] [PATCH v2 2/6] sunxi: add Kconfig option for the maximum accessible DRAM

[Chen-Yu Tsai]

On Fri, Mar 23, 2018 at 5:39 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Fri, Mar 23, 2018 at 04:18:53PM +0800, Icenowy Zheng wrote:
>> Allwinner 64-bit SoCs can use 4GiB DRAM chip, however their memory map
>> has only allocated 3GiB for DRAM, so only 3GiB of the DRAM is
>> accessible.
>
> How would that work in Linux?
>
>> Add a Kconfig option for the maximum accessible DRAM.
>>
>> For A80 it should be a much higher value (8GiB), but as I have no A80
>> device to test and originally U-Boot only supports 2GiB DRAM on A80, it
>> currently still falls under the 2GiB situation.
>
> I guess we have LPAE on armv7 to deal with this nicely.

AFAIK even though we do have LPAE, the hardware bus is not wired up
to take advantage of it. :(

ChenYu

[U-Boot] [PATCH v2 4/6] sunxi: restrict the ram_size to the accessible range in SPL

[Maxime Ripard]

On Fri, Mar 23, 2018 at 04:18:55PM +0800, Icenowy Zheng wrote:
> On newer Allwinner SoCs with the BROM start at 0x0 and the DRAM space at
> <0x40000000 0xc0000000>, some parts of DRAM will be inaccessible when
> 4GiB module is used.
> 
> Restrict the ram_size written to global_data in SPL.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> No changes in v2.
> 
>  board/sunxi/board.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 1c5e1f380a..73dd41437d 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -518,6 +518,7 @@ int board_mmc_init(bd_t *bis)
>  void sunxi_board_init(void)
>  {
>  	int power_failed = 0;
> +	unsigned long long dram_real_size;
>  
>  #ifdef CONFIG_SY8106A_POWER
>  	power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT);
> @@ -578,8 +579,16 @@ void sunxi_board_init(void)
>  #endif
>  #endif
>  	printf("DRAM:");
> -	gd->ram_size = (phys_size_t)sunxi_dram_init();
> -	printf(" %d MiB\n", (int)(gd->ram_size >> 20));
> +	dram_real_size = sunxi_dram_init();
> +	printf(" %d MiB", (int)(dram_real_size >> 20));
> +	if (dram_real_size > CONFIG_SUNXI_DRAM_MAX_SIZE) {
> +		gd->ram_size = CONFIG_SUNXI_DRAM_MAX_SIZE;
> +		printf(", %d MiB usable\n", (int)(gd->ram_size >> 20));
> +	} else {
> +		gd->ram_size = (phys_size_t)dram_real_size;
> +		printf("\n");
> +	}
> +

This really look like something that should be addressed generically
and not in one board.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 2/6] sunxi: add Kconfig option for the maximum accessible DRAM

[Icenowy Zheng]

于 2018年3月23日 GMT+08:00 下午5:39:25, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Fri, Mar 23, 2018 at 04:18:53PM +0800, Icenowy Zheng wrote:
>> Allwinner 64-bit SoCs can use 4GiB DRAM chip, however their memory
>map
>> has only allocated 3GiB for DRAM, so only 3GiB of the DRAM is
>> accessible.
>
>How would that work in Linux?

From CPU's view only 3GiB DRAM is attached, so Linux can
use the 3GiB space only.

Only the DRAM controller knows that 4GiB DRAM is connected :-)

>
>> Add a Kconfig option for the maximum accessible DRAM.
>> 
>> For A80 it should be a much higher value (8GiB), but as I have no A80
>> device to test and originally U-Boot only supports 2GiB DRAM on A80,
>it
>> currently still falls under the 2GiB situation.
>
>I guess we have LPAE on armv7 to deal with this nicely.
>
>Maxime

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
> >> The get_ram_size() function in U-Boot can only deal with memory size
> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit
> >> SoCs, an alternative way to detect DRAM size is needed.
> >
> >Why not just fixing get_ram_size then?
> 
> Even if it's fixed it won't support 3GiB DRAM at all.

Why?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180326/06f72371/attachment.sig>

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
><maxime.ripard@bootlin.com> 写到:
>> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
>> >> The get_ram_size() function in U-Boot can only deal with memory
>size
>> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
>64-bit
>> >> SoCs, an alternative way to detect DRAM size is needed.
>> >
>> >Why not just fixing get_ram_size then?
>> 
>> Even if it's fixed it won't support 3GiB DRAM at all.
>
>Why?

It has an assumption that the size is pow of 2.

>
>Maxime

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
> >> 
> >> 
> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
> ><maxime.ripard@bootlin.com> 写到:
> >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
> >> >> The get_ram_size() function in U-Boot can only deal with memory
> >size
> >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
> >64-bit
> >> >> SoCs, an alternative way to detect DRAM size is needed.
> >> >
> >> >Why not just fixing get_ram_size then?
> >> 
> >> Even if it's fixed it won't support 3GiB DRAM at all.
> >
> >Why?
> 
> It has an assumption that the size is pow of 2.

I guess this would be fixable too? (or one could create a variant
without that assumption).

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180328/9518bb48/attachment.sig>

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
><maxime.ripard@bootlin.com> 写到:
>> >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>> >> 
>> >> 
>> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
>> ><maxime.ripard@bootlin.com> 写到:
>> >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
>> >> >> The get_ram_size() function in U-Boot can only deal with memory
>> >size
>> >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
>> >64-bit
>> >> >> SoCs, an alternative way to detect DRAM size is needed.
>> >> >
>> >> >Why not just fixing get_ram_size then?
>> >> 
>> >> Even if it's fixed it won't support 3GiB DRAM at all.
>> >
>> >Why?
>> 
>> It has an assumption that the size is pow of 2.
>
>I guess this would be fixable too? (or one could create a variant
>without that assumption).

I don't think its principle allows such kind of fix, as it just
checks writing then reading at some offset that is pow if 2.

For hacking it, see my implementation in v1, which assumes the
only size supported bigger than 2GiB is 3GiB (which is
acceptable on sunxi, but might not work on other platforms).

As Andre said, that function has another big problem --
it detects memory with writing to it. This is risky.

>
>Maxime

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
> >> 
> >> 
> >> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
> ><maxime.ripard@bootlin.com> 写到:
> >> >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
> >> >> 
> >> >> 
> >> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
> >> ><maxime.ripard@bootlin.com> 写到:
> >> >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
> >> >> >> The get_ram_size() function in U-Boot can only deal with memory
> >> >size
> >> >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
> >> >64-bit
> >> >> >> SoCs, an alternative way to detect DRAM size is needed.
> >> >> >
> >> >> >Why not just fixing get_ram_size then?
> >> >> 
> >> >> Even if it's fixed it won't support 3GiB DRAM at all.
> >> >
> >> >Why?
> >> 
> >> It has an assumption that the size is pow of 2.
> >
> >I guess this would be fixable too? (or one could create a variant
> >without that assumption).
> 
> I don't think its principle allows such kind of fix, as it just
> checks writing then reading at some offset that is pow if 2.

You could do have a bunch of algorithm actually. One would be to write
the address in memory and try to detect where exactly it starts to
loop.

You could do a bisection in the opposite direction once you settled
for the upper limit (so you would have for example a workable 2G, a
non-workable 4G, and then you try intervals that you always divide by
two, so testing then 3G (that works), then halfway between 3G and 4G,
etc.

> For hacking it, see my implementation in v1, which assumes the
> only size supported bigger than 2GiB is 3GiB (which is
> acceptable on sunxi, but might not work on other platforms).
> 
> As Andre said, that function has another big problem -- it detects
> memory with writing to it. This is risky.

How is it risky when it's done by the SPL?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180329/9343859e/attachment.sig>

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Andre Przywara]

Hi,

On 29/03/18 10:37, Maxime Ripard wrote:
> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
>>>>
>>>>
>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
>>> <maxime.ripard@bootlin.com> 写到:
>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>>>>>>
>>>>>>
>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
>>>>>>>> The get_ram_size() function in U-Boot can only deal with memory
>>>>> size
>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
>>>>> 64-bit
>>>>>>>> SoCs, an alternative way to detect DRAM size is needed.
>>>>>>>
>>>>>>> Why not just fixing get_ram_size then?
>>>>>>
>>>>>> Even if it's fixed it won't support 3GiB DRAM at all.
>>>>>
>>>>> Why?
>>>>
>>>> It has an assumption that the size is pow of 2.
>>>
>>> I guess this would be fixable too? (or one could create a variant
>>> without that assumption).
>>
>> I don't think its principle allows such kind of fix, as it just
>> checks writing then reading at some offset that is pow if 2.
> 
> You could do have a bunch of algorithm actually. One would be to write
> the address in memory and try to detect where exactly it starts to
> loop.
> 
> You could do a bisection in the opposite direction once you settled
> for the upper limit (so you would have for example a workable 2G, a
> non-workable 4G, and then you try intervals that you always divide by
> two, so testing then 3G (that works), then halfway between 3G and 4G,
> etc.
> 
>> For hacking it, see my implementation in v1, which assumes the
>> only size supported bigger than 2GiB is 3GiB (which is
>> acceptable on sunxi, but might not work on other platforms).
>>
>> As Andre said, that function has another big problem -- it detects
>> memory with writing to it. This is risky.
> 
> How is it risky when it's done by the SPL?

Originally that was my confusion as well: It's not the SPL calling that
function. The DRAM controller init function in there knows very
precisely how much DRAM we have, but we don't communicate this to U-Boot
proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it
could step into secure memory, for instance. On sunxi64 we have the ATF
running between SPL and U-Boot, also all kind of secure payloads could
already have been registered.
So I wonder if it would be easier to somehow pass on this *one* word of
information between SPL and U-Boot proper to avoid calling this function
altogether?

Cheers,
Andre.

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote:
> Hi,
> 
> On 29/03/18 10:37, Maxime Ripard wrote:
> > On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
> >> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
> >>>>
> >>>>
> >>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
> >>> <maxime.ripard@bootlin.com> 写到:
> >>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
> >>>>>>
> >>>>>>
> >>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
> >>>>> <maxime.ripard@bootlin.com> 写到:
> >>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
> >>>>>>>> The get_ram_size() function in U-Boot can only deal with memory
> >>>>> size
> >>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
> >>>>> 64-bit
> >>>>>>>> SoCs, an alternative way to detect DRAM size is needed.
> >>>>>>>
> >>>>>>> Why not just fixing get_ram_size then?
> >>>>>>
> >>>>>> Even if it's fixed it won't support 3GiB DRAM at all.
> >>>>>
> >>>>> Why?
> >>>>
> >>>> It has an assumption that the size is pow of 2.
> >>>
> >>> I guess this would be fixable too? (or one could create a variant
> >>> without that assumption).
> >>
> >> I don't think its principle allows such kind of fix, as it just
> >> checks writing then reading at some offset that is pow if 2.
> > 
> > You could do have a bunch of algorithm actually. One would be to write
> > the address in memory and try to detect where exactly it starts to
> > loop.
> > 
> > You could do a bisection in the opposite direction once you settled
> > for the upper limit (so you would have for example a workable 2G, a
> > non-workable 4G, and then you try intervals that you always divide by
> > two, so testing then 3G (that works), then halfway between 3G and 4G,
> > etc.
> > 
> >> For hacking it, see my implementation in v1, which assumes the
> >> only size supported bigger than 2GiB is 3GiB (which is
> >> acceptable on sunxi, but might not work on other platforms).
> >>
> >> As Andre said, that function has another big problem -- it detects
> >> memory with writing to it. This is risky.
> > 
> > How is it risky when it's done by the SPL?
> 
> Originally that was my confusion as well: It's not the SPL calling that
> function. The DRAM controller init function in there knows very
> precisely how much DRAM we have, but we don't communicate this to U-Boot
> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it
> could step into secure memory, for instance. On sunxi64 we have the ATF
> running between SPL and U-Boot, also all kind of secure payloads could
> already have been registered.
> So I wonder if it would be easier to somehow pass on this *one* word of
> information between SPL and U-Boot proper to avoid calling this function
> altogether?

That would definitely make sense yes.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180403/6b5daae8/attachment.sig>

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Andre Przywara]

Hi,

On 03/04/18 10:29, Maxime Ripard wrote:
> On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote:
>> Hi,
>>
>> On 29/03/18 10:37, Maxime Ripard wrote:
>>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
>>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
>>>>>>
>>>>>>
>>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
>>>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote:
>>>>>>>>>> The get_ram_size() function in U-Boot can only deal with memory
>>>>>>> size
>>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer
>>>>>>> 64-bit
>>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed.
>>>>>>>>>
>>>>>>>>> Why not just fixing get_ram_size then?
>>>>>>>>
>>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all.
>>>>>>>
>>>>>>> Why?
>>>>>>
>>>>>> It has an assumption that the size is pow of 2.
>>>>>
>>>>> I guess this would be fixable too? (or one could create a variant
>>>>> without that assumption).
>>>>
>>>> I don't think its principle allows such kind of fix, as it just
>>>> checks writing then reading at some offset that is pow if 2.
>>>
>>> You could do have a bunch of algorithm actually. One would be to write
>>> the address in memory and try to detect where exactly it starts to
>>> loop.
>>>
>>> You could do a bisection in the opposite direction once you settled
>>> for the upper limit (so you would have for example a workable 2G, a
>>> non-workable 4G, and then you try intervals that you always divide by
>>> two, so testing then 3G (that works), then halfway between 3G and 4G,
>>> etc.
>>>
>>>> For hacking it, see my implementation in v1, which assumes the
>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>> acceptable on sunxi, but might not work on other platforms).
>>>>
>>>> As Andre said, that function has another big problem -- it detects
>>>> memory with writing to it. This is risky.
>>>
>>> How is it risky when it's done by the SPL?
>>
>> Originally that was my confusion as well: It's not the SPL calling that
>> function. The DRAM controller init function in there knows very
>> precisely how much DRAM we have, but we don't communicate this to U-Boot
>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it
>> could step into secure memory, for instance. On sunxi64 we have the ATF
>> running between SPL and U-Boot, also all kind of secure payloads could
>> already have been registered.
>> So I wonder if it would be easier to somehow pass on this *one* word of
>> information between SPL and U-Boot proper to avoid calling this function
>> altogether?
> 
> That would definitely make sense yes.

So since the SPL loads the DT anyway (from the FIT image) and puts it at
the end of the U-Boot (proper) binary, wouldn't it be the easiest to
just patch the actual DRAM size in there?
IIRC we don't have any FDT write code in the SPL at the moment, and
pulling it in would probably push it over the edge again, but:
We should be able to somewhat short-cut it, if it's just about to
actually *patch* an existing value, as of:
- make sure we have a memory node and a reg property in the DT
- in the SPL learn the fdt offset of that reg property
- <hack> patch in the memory size into the second word </hack>
- teach U-Boot proper to read the memory size from the DT
- optionally look at #address-cells and #size-cells to make this "second
word hack" more robust

This could actually be a rather generic patch, just with some "avoid
libfdt write library" hack to address our size issue. Maybe we already
have something like that for some platform (Rockchip comes to mind?)

How does that sound?

Cheers,
Andre.

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年4月3日 GMT+08:00 下午6:13:17, Andre Przywara <andre.przywara@arm.com> 写到:
>Hi,
>
>On 03/04/18 10:29, Maxime Ripard wrote:
>> On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 29/03/18 10:37, Maxime Ripard wrote:
>>>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
>>>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard
><maxime.ripard@bootlin.com> 写到:
>>>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
>>>>>>>
>>>>>>>
>>>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
>>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
>>>>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng
>wrote:
>>>>>>>>>>> The get_ram_size() function in U-Boot can only deal with
>memory
>>>>>>>> size
>>>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on
>newer
>>>>>>>> 64-bit
>>>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed.
>>>>>>>>>>
>>>>>>>>>> Why not just fixing get_ram_size then?
>>>>>>>>>
>>>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all.
>>>>>>>>
>>>>>>>> Why?
>>>>>>>
>>>>>>> It has an assumption that the size is pow of 2.
>>>>>>
>>>>>> I guess this would be fixable too? (or one could create a variant
>>>>>> without that assumption).
>>>>>
>>>>> I don't think its principle allows such kind of fix, as it just
>>>>> checks writing then reading at some offset that is pow if 2.
>>>>
>>>> You could do have a bunch of algorithm actually. One would be to
>write
>>>> the address in memory and try to detect where exactly it starts to
>>>> loop.
>>>>
>>>> You could do a bisection in the opposite direction once you settled
>>>> for the upper limit (so you would have for example a workable 2G, a
>>>> non-workable 4G, and then you try intervals that you always divide
>by
>>>> two, so testing then 3G (that works), then halfway between 3G and
>4G,
>>>> etc.
>>>>
>>>>> For hacking it, see my implementation in v1, which assumes the
>>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>>> acceptable on sunxi, but might not work on other platforms).
>>>>>
>>>>> As Andre said, that function has another big problem -- it detects
>>>>> memory with writing to it. This is risky.
>>>>
>>>> How is it risky when it's done by the SPL?
>>>
>>> Originally that was my confusion as well: It's not the SPL calling
>that
>>> function. The DRAM controller init function in there knows very
>>> precisely how much DRAM we have, but we don't communicate this to
>U-Boot
>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This
>means it
>>> could step into secure memory, for instance. On sunxi64 we have the
>ATF
>>> running between SPL and U-Boot, also all kind of secure payloads
>could
>>> already have been registered.
>>> So I wonder if it would be easier to somehow pass on this *one* word
>of
>>> information between SPL and U-Boot proper to avoid calling this
>function
>>> altogether?
>> 
>> That would definitely make sense yes.
>
>So since the SPL loads the DT anyway (from the FIT image) and puts it
>at
>the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>just patch the actual DRAM size in there?
>IIRC we don't have any FDT write code in the SPL at the moment, and
>pulling it in would probably push it over the edge again, but:
>We should be able to somewhat short-cut it, if it's just about to
>actually *patch* an existing value, as of:
>- make sure we have a memory node and a reg property in the DT
>- in the SPL learn the fdt offset of that reg property
>- <hack> patch in the memory size into the second word </hack>
>- teach U-Boot proper to read the memory size from the DT
>- optionally look at #address-cells and #size-cells to make this
>"second
>word hack" more robust
>
>This could actually be a rather generic patch, just with some "avoid
>libfdt write library" hack to address our size issue. Maybe we already
>have something like that for some platform (Rockchip comes to mind?)

Rockchip recalculates the memory size with the parameters
in GRF when running the main U-Boot, so both TPL/SPL
and miniloader can load mainline U-Boot.

>
>How does that sound?
>
>Cheers,
>Andre.

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Dr. Philipp Tomsich]

> On 3 Apr 2018, at 12:39, Icenowy Zheng <icenowy@aosc.io> wrote:
> 
> 
> 
> 于 2018年4月3日 GMT+08:00 下午6:13:17, Andre Przywara <andre.przywara at arm.com <mailto:andre.przywara@arm.com>> 写到:
>> Hi,
>> 
>> On 03/04/18 10:29, Maxime Ripard wrote:
>>> On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote:
>>>> Hi,
>>>> 
>>>> On 29/03/18 10:37, Maxime Ripard wrote:
>>>>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote:
>>>>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard
>> <maxime.ripard@bootlin.com> 写到:
>>>>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard
>>>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote:
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard
>>>>>>>>> <maxime.ripard@bootlin.com> 写到:
>>>>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng
>> wrote:
>>>>>>>>>>>> The get_ram_size() function in U-Boot can only deal with
>> memory
>>>>>>>>> size
>>>>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on
>> newer
>>>>>>>>> 64-bit
>>>>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed.
>>>>>>>>>>> 
>>>>>>>>>>> Why not just fixing get_ram_size then?
>>>>>>>>>> 
>>>>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all.
>>>>>>>>> 
>>>>>>>>> Why?
>>>>>>>> 
>>>>>>>> It has an assumption that the size is pow of 2.
>>>>>>> 
>>>>>>> I guess this would be fixable too? (or one could create a variant
>>>>>>> without that assumption).
>>>>>> 
>>>>>> I don't think its principle allows such kind of fix, as it just
>>>>>> checks writing then reading at some offset that is pow if 2.
>>>>> 
>>>>> You could do have a bunch of algorithm actually. One would be to
>> write
>>>>> the address in memory and try to detect where exactly it starts to
>>>>> loop.
>>>>> 
>>>>> You could do a bisection in the opposite direction once you settled
>>>>> for the upper limit (so you would have for example a workable 2G, a
>>>>> non-workable 4G, and then you try intervals that you always divide
>> by
>>>>> two, so testing then 3G (that works), then halfway between 3G and
>> 4G,
>>>>> etc.
>>>>> 
>>>>>> For hacking it, see my implementation in v1, which assumes the
>>>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>>>> acceptable on sunxi, but might not work on other platforms).
>>>>>> 
>>>>>> As Andre said, that function has another big problem -- it detects
>>>>>> memory with writing to it. This is risky.
>>>>> 
>>>>> How is it risky when it's done by the SPL?
>>>> 
>>>> Originally that was my confusion as well: It's not the SPL calling
>> that
>>>> function. The DRAM controller init function in there knows very
>>>> precisely how much DRAM we have, but we don't communicate this to
>> U-Boot
>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This
>> means it
>>>> could step into secure memory, for instance. On sunxi64 we have the
>> ATF
>>>> running between SPL and U-Boot, also all kind of secure payloads
>> could
>>>> already have been registered.
>>>> So I wonder if it would be easier to somehow pass on this *one* word
>> of
>>>> information between SPL and U-Boot proper to avoid calling this
>> function
>>>> altogether?
>>> 
>>> That would definitely make sense yes.
>> 
>> So since the SPL loads the DT anyway (from the FIT image) and puts it
>> at
>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>> just patch the actual DRAM size in there?
>> IIRC we don't have any FDT write code in the SPL at the moment, and
>> pulling it in would probably push it over the edge again, but:
>> We should be able to somewhat short-cut it, if it's just about to
>> actually *patch* an existing value, as of:
>> - make sure we have a memory node and a reg property in the DT
>> - in the SPL learn the fdt offset of that reg property
>> - <hack> patch in the memory size into the second word </hack>
>> - teach U-Boot proper to read the memory size from the DT
>> - optionally look at #address-cells and #size-cells to make this
>> "second
>> word hack" more robust
>> 
>> This could actually be a rather generic patch, just with some "avoid
>> libfdt write library" hack to address our size issue. Maybe we already
>> have something like that for some platform (Rockchip comes to mind?)
> 
> Rockchip recalculates the memory size with the parameters
> in GRF when running the main U-Boot, so both TPL/SPL
> and miniloader can load mainline U-Boot.

More accurately, the GRF currently is used to store an encoded DRAM
configuration and the next stage can rely on this being there.

If the memory is initialised by miniloader, U-Boot reads the GRF and calculates
the memory size from that.  If U-Boot sets up DRAM, then it also populates the
GRF registers.

With more and more SOCs moving away from miniloader, there’s a good chance
that this will change at some point in the future and the GRF may not necessarily
be populated anymore.

—Philipp.

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Maxime Ripard]

On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
> >>>> For hacking it, see my implementation in v1, which assumes the
> >>>> only size supported bigger than 2GiB is 3GiB (which is
> >>>> acceptable on sunxi, but might not work on other platforms).
> >>>>
> >>>> As Andre said, that function has another big problem -- it detects
> >>>> memory with writing to it. This is risky.
> >>>
> >>> How is it risky when it's done by the SPL?
> >>
> >> Originally that was my confusion as well: It's not the SPL calling that
> >> function. The DRAM controller init function in there knows very
> >> precisely how much DRAM we have, but we don't communicate this to U-Boot
> >> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it
> >> could step into secure memory, for instance. On sunxi64 we have the ATF
> >> running between SPL and U-Boot, also all kind of secure payloads could
> >> already have been registered.
> >> So I wonder if it would be easier to somehow pass on this *one* word of
> >> information between SPL and U-Boot proper to avoid calling this function
> >> altogether?
> > 
> > That would definitely make sense yes.
> 
> So since the SPL loads the DT anyway (from the FIT image) and puts it at
> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
> just patch the actual DRAM size in there?
> IIRC we don't have any FDT write code in the SPL at the moment, and
> pulling it in would probably push it over the edge again, but:

That assumes that you are loading a FIT image, which might or might
not be the case, and on most arm32 chips, most likely won't.

I guess we'd need to find another way (put some information in an
SRAM somewhere?) to try to do that for all variants.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180403/5b2a7698/attachment.sig>

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Andre Przywara]

Hi,

On 03/04/18 12:34, Maxime Ripard wrote:
> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
>>>>>> For hacking it, see my implementation in v1, which assumes the
>>>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>>>> acceptable on sunxi, but might not work on other platforms).
>>>>>>
>>>>>> As Andre said, that function has another big problem -- it detects
>>>>>> memory with writing to it. This is risky.
>>>>>
>>>>> How is it risky when it's done by the SPL?
>>>>
>>>> Originally that was my confusion as well: It's not the SPL calling that
>>>> function. The DRAM controller init function in there knows very
>>>> precisely how much DRAM we have, but we don't communicate this to U-Boot
>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it
>>>> could step into secure memory, for instance. On sunxi64 we have the ATF
>>>> running between SPL and U-Boot, also all kind of secure payloads could
>>>> already have been registered.
>>>> So I wonder if it would be easier to somehow pass on this *one* word of
>>>> information between SPL and U-Boot proper to avoid calling this function
>>>> altogether?
>>>
>>> That would definitely make sense yes.
>>
>> So since the SPL loads the DT anyway (from the FIT image) and puts it at
>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>> just patch the actual DRAM size in there?
>> IIRC we don't have any FDT write code in the SPL at the moment, and
>> pulling it in would probably push it over the edge again, but:
> 
> That assumes that you are loading a FIT image, which might or might
> not be the case, and on most arm32 chips, most likely won't.

That's true, but my understanding is that this >4GB is only relevant for
64-bit SoCs, where we mandate FIT images, don't we?

> I guess we'd need to find another way (put some information in an
> SRAM somewhere?) to try to do that for all variants.

Oh, so your aim at getting rid of the call to the memory probing
function at all? I was just assuming that we change it for the purpose
of this 3GB support, which would be 64-bit only?

Cheers,
Andre

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年4月3日 GMT+08:00 下午7:41:41, Andre Przywara <andre.przywara@arm.com> 写到:
>Hi,
>
>On 03/04/18 12:34, Maxime Ripard wrote:
>> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
>>>>>>> For hacking it, see my implementation in v1, which assumes the
>>>>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>>>>> acceptable on sunxi, but might not work on other platforms).
>>>>>>>
>>>>>>> As Andre said, that function has another big problem -- it
>detects
>>>>>>> memory with writing to it. This is risky.
>>>>>>
>>>>>> How is it risky when it's done by the SPL?
>>>>>
>>>>> Originally that was my confusion as well: It's not the SPL calling
>that
>>>>> function. The DRAM controller init function in there knows very
>>>>> precisely how much DRAM we have, but we don't communicate this to
>U-Boot
>>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This
>means it
>>>>> could step into secure memory, for instance. On sunxi64 we have
>the ATF
>>>>> running between SPL and U-Boot, also all kind of secure payloads
>could
>>>>> already have been registered.
>>>>> So I wonder if it would be easier to somehow pass on this *one*
>word of
>>>>> information between SPL and U-Boot proper to avoid calling this
>function
>>>>> altogether?
>>>>
>>>> That would definitely make sense yes.
>>>
>>> So since the SPL loads the DT anyway (from the FIT image) and puts
>it at
>>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>>> just patch the actual DRAM size in there?
>>> IIRC we don't have any FDT write code in the SPL at the moment, and
>>> pulling it in would probably push it over the edge again, but:
>> 
>> That assumes that you are loading a FIT image, which might or might
>> not be the case, and on most arm32 chips, most likely won't.
>
>That's true, but my understanding is that this >4GB is only relevant
>for
>64-bit SoCs, where we mandate FIT images, don't we?

It's also relevant with A80, theortically.

And Allwinner is continously producing Cortex-A7
SoCs, which might use this design.

>
>> I guess we'd need to find another way (put some information in an
>> SRAM somewhere?) to try to do that for all variants.
>
>Oh, so your aim at getting rid of the call to the memory probing
>function at all? I was just assuming that we change it for the purpose
>of this 3GB support, which would be 64-bit only?
>
>Cheers,
>Andre

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Icenowy Zheng]

于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
>> >>>> For hacking it, see my implementation in v1, which assumes the
>> >>>> only size supported bigger than 2GiB is 3GiB (which is
>> >>>> acceptable on sunxi, but might not work on other platforms).
>> >>>>
>> >>>> As Andre said, that function has another big problem -- it
>detects
>> >>>> memory with writing to it. This is risky.
>> >>>
>> >>> How is it risky when it's done by the SPL?
>> >>
>> >> Originally that was my confusion as well: It's not the SPL calling
>that
>> >> function. The DRAM controller init function in there knows very
>> >> precisely how much DRAM we have, but we don't communicate this to
>U-Boot
>> >> proper. So U-Boot *proper* goes ahead and probes the DRAM. This
>means it
>> >> could step into secure memory, for instance. On sunxi64 we have
>the ATF
>> >> running between SPL and U-Boot, also all kind of secure payloads
>could
>> >> already have been registered.
>> >> So I wonder if it would be easier to somehow pass on this *one*
>word of
>> >> information between SPL and U-Boot proper to avoid calling this
>function
>> >> altogether?
>> > 
>> > That would definitely make sense yes.
>> 
>> So since the SPL loads the DT anyway (from the FIT image) and puts it
>at
>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>> just patch the actual DRAM size in there?
>> IIRC we don't have any FDT write code in the SPL at the moment, and
>> pulling it in would probably push it over the edge again, but:
>
>That assumes that you are loading a FIT image, which might or might
>not be the case, and on most arm32 chips, most likely won't.
>
>I guess we'd need to find another way (put some information in an
>SRAM somewhere?) to try to do that for all variants.

Extend the SPL header again? If we found SPL v3+, use
the DRAM size encoded and bypass ram_get_size,
otherwise fallback to ram_get_size?

(Although it will lead to some days of mess on sunxi-tools,
this is a reasonable choice.)

>
>Maxime

[U-Boot] [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Andre Przywara]

Hi Icenowy,

On 03/04/18 12:51, Icenowy Zheng wrote:
> 
> 
> 于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:
>>>>>>> For hacking it, see my implementation in v1, which assumes the
>>>>>>> only size supported bigger than 2GiB is 3GiB (which is
>>>>>>> acceptable on sunxi, but might not work on other platforms).
>>>>>>>
>>>>>>> As Andre said, that function has another big problem -- it
>> detects
>>>>>>> memory with writing to it. This is risky.
>>>>>>
>>>>>> How is it risky when it's done by the SPL?
>>>>>
>>>>> Originally that was my confusion as well: It's not the SPL calling
>> that
>>>>> function. The DRAM controller init function in there knows very
>>>>> precisely how much DRAM we have, but we don't communicate this to
>> U-Boot
>>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This
>> means it
>>>>> could step into secure memory, for instance. On sunxi64 we have
>> the ATF
>>>>> running between SPL and U-Boot, also all kind of secure payloads
>> could
>>>>> already have been registered.
>>>>> So I wonder if it would be easier to somehow pass on this *one*
>> word of
>>>>> information between SPL and U-Boot proper to avoid calling this
>> function
>>>>> altogether?
>>>>
>>>> That would definitely make sense yes.
>>>
>>> So since the SPL loads the DT anyway (from the FIT image) and puts it
>> at
>>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
>>> just patch the actual DRAM size in there?
>>> IIRC we don't have any FDT write code in the SPL at the moment, and
>>> pulling it in would probably push it over the edge again, but:
>>
>> That assumes that you are loading a FIT image, which might or might
>> not be the case, and on most arm32 chips, most likely won't.
>>
>> I guess we'd need to find another way (put some information in an
>> SRAM somewhere?) to try to do that for all variants.
> 
> Extend the SPL header again? If we found SPL v3+, use
> the DRAM size encoded and bypass ram_get_size,
> otherwise fallback to ram_get_size?

Yes, that would be a possibility as well. Though I believe at the moment
we don't access the SPL header from U-Boot proper, do we? Not a real
show-stopper, but we probably need to document that the SPL header would
need to stay around.
But if we have a fallback anyway ...

> (Although it will lead to some days of mess on sunxi-tools,
> this is a reasonable choice.)

True, but actually I wonder why we have SPL_MAX_VERSION in there in the
first place. Can't we just postulate that every new SPL version stays
backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would
still be fine, you would just loose the v3 features (if at all)? We can
just put a warning in there, to ask users to upgrade.
That would have worked already with the v1/v2 transition, I believe.

Probably worth a separate discussion with some sunxi-tools stakeholders.

Cheers,
Andre.

[U-Boot] [linux-sunxi] Re: [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[Siarhei Siamashka]

On Tue, 3 Apr 2018 13:43:43 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

> Hi Icenowy,
> 
> On 03/04/18 12:51, Icenowy Zheng wrote:
> > 
> > 
> > 于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到:  
> >> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote:  
> >>>>>>> For hacking it, see my implementation in v1, which assumes the
> >>>>>>> only size supported bigger than 2GiB is 3GiB (which is
> >>>>>>> acceptable on sunxi, but might not work on other platforms).
> >>>>>>>
> >>>>>>> As Andre said, that function has another big problem -- it  
> >> detects  
> >>>>>>> memory with writing to it. This is risky.  
> >>>>>>
> >>>>>> How is it risky when it's done by the SPL?  
> >>>>>
> >>>>> Originally that was my confusion as well: It's not the SPL calling  
> >> that  
> >>>>> function. The DRAM controller init function in there knows very
> >>>>> precisely how much DRAM we have, but we don't communicate this to  
> >> U-Boot  
> >>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This  
> >> means it  
> >>>>> could step into secure memory, for instance. On sunxi64 we have  
> >> the ATF  
> >>>>> running between SPL and U-Boot, also all kind of secure payloads  
> >> could  
> >>>>> already have been registered.
> >>>>> So I wonder if it would be easier to somehow pass on this *one*  
> >> word of  
> >>>>> information between SPL and U-Boot proper to avoid calling this  
> >> function  
> >>>>> altogether?  
> >>>>
> >>>> That would definitely make sense yes.  
> >>>
> >>> So since the SPL loads the DT anyway (from the FIT image) and puts it  
> >> at  
> >>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to
> >>> just patch the actual DRAM size in there?
> >>> IIRC we don't have any FDT write code in the SPL at the moment, and
> >>> pulling it in would probably push it over the edge again, but:  
> >>
> >> That assumes that you are loading a FIT image, which might or might
> >> not be the case, and on most arm32 chips, most likely won't.
> >>
> >> I guess we'd need to find another way (put some information in an
> >> SRAM somewhere?) to try to do that for all variants.  
> > 
> > Extend the SPL header again? If we found SPL v3+, use
> > the DRAM size encoded and bypass ram_get_size,
> > otherwise fallback to ram_get_size?  
> 
> Yes, that would be a possibility as well. Though I believe at the moment
> we don't access the SPL header from U-Boot proper, do we?

We do. The boot device and also the boot.scr location (in the case of
FEL boot) is read from the SPL header by the main U-Boot part.

> Not a real show-stopper, but we probably need to document that the SPL
> header would need to stay around.

Maybe.

> But if we have a fallback anyway ...

Which fallback? Do you mean calling things like ram_get_size()
from U-Boot? We should never do this because this is very wrong.

The D-Cache may be already enabled, causing all kinds of weird
effects. Also modifying data in DRAM (even temporarily) while
our code is already running from DRAM is dangerous.

> > (Although it will lead to some days of mess on sunxi-tools,
> > this is a reasonable choice.)  
> 
> True, but actually I wonder why we have SPL_MAX_VERSION in there in the
> first place. Can't we just postulate that every new SPL version stays
> backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would
> still be fine, you would just loose the v3 features (if at all)?
> We can just put a warning in there, to ask users to upgrade.
> That would have worked already with the v1/v2 transition, I believe.

Yes, that's more or less how this was supposed to work in sunxi-tools
from the very beginning. Except that we unfortunately got a bug in
this code, which has been reported back in July 2017 and is still not
resolved due to various reasons:

   https://github.com/linux-sunxi/sunxi-tools/issues/104

But hopefully it can be fixed sooner or later.

> Probably worth a separate discussion with some sunxi-tools stakeholders.

On the U-Boot side we can just increase the version number as long as
the new header is a backwards compatible superset of the old one.

In the unlikely case if we suddenly have to introduce a compatibility
breaking change to the SPL header format, we can always change the SPL
header signature from 'SPL' to something else.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [linux-sunxi] Re: [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[André Przywara]

On 03/04/18 15:13, Siarhei Siamashka wrote:

Hi Siarhei,

thanks for chiming in!

> On Tue, 3 Apr 2018 13:43:43 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
>> On 03/04/18 12:51, Icenowy Zheng wrote:

....

>>>> I guess we'd need to find another way (put some information in an
>>>> SRAM somewhere?) to try to do that for all variants.  
>>>
>>> Extend the SPL header again? If we found SPL v3+, use
>>> the DRAM size encoded and bypass ram_get_size,
>>> otherwise fallback to ram_get_size?  
>>
>> Yes, that would be a possibility as well. Though I believe at the moment
>> we don't access the SPL header from U-Boot proper, do we?
> 
> We do. The boot device and also the boot.scr location (in the case of
> FEL boot) is read from the SPL header by the main U-Boot part.

Ah, true, forgot about that. Even better then.

>> Not a real show-stopper, but we probably need to document that the SPL
>> header would need to stay around.
> 
> Maybe.
> 
>> But if we have a fallback anyway ...
> 
> Which fallback? Do you mean calling things like ram_get_size()
> from U-Boot?

Yes, that was what Icenowy suggested: If SPL version > 2, we use the
information from there, otherwise we fall back to the current behaviour,
which is to ride through the DRAM and hope for the best.
Though I am not sure this is really needed, as I don't see a strong
reason to combine different versions of SPL and U-Boot proper (apart
from FEL, maybe).

> We should never do this because this is very wrong.

I mostly agree, though it's not too bad, since we have quite a
controlled environment. But if we can get rid of it: Yes, we should.

> The D-Cache may be already enabled, causing all kinds of weird
> effects. Also modifying data in DRAM (even temporarily) while
> our code is already running from DRAM is dangerous.

I don't see immediately how the D$ could get nasty here, but as I said
above, we should do it.

>>> (Although it will lead to some days of mess on sunxi-tools,
>>> this is a reasonable choice.)  
>>
>> True, but actually I wonder why we have SPL_MAX_VERSION in there in the
>> first place. Can't we just postulate that every new SPL version stays
>> backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would
>> still be fine, you would just loose the v3 features (if at all)?
>> We can just put a warning in there, to ask users to upgrade.
>> That would have worked already with the v1/v2 transition, I believe.
> 
> Yes, that's more or less how this was supposed to work in sunxi-tools
> from the very beginning. Except that we unfortunately got a bug in
> this code, which has been reported back in July 2017 and is still not
> resolved due to various reasons:

Well, that sounds more like a design issue to me: Defining the latest
currently supported version as the maximum.

>    https://github.com/linux-sunxi/sunxi-tools/issues/104
> 
> But hopefully it can be fixed sooner or later.

I think we can do it now, see below.

>> Probably worth a separate discussion with some sunxi-tools stakeholders.
> 
> On the U-Boot side we can just increase the version number as long as
> the new header is a backwards compatible superset of the old one.
> 
> In the unlikely case if we suddenly have to introduce a compatibility
> breaking change to the SPL header format, we can always change the SPL
> header signature from 'SPL' to something else.

Or we could introduce some major/minor scheme, with a major change
breaking compatibility, whereas a minor change does not.
Then we split the uint8_t version into 2 bits of major and 6 bits of
minor, for instance.
So we just document this and bump SPL_MAX_VERSION now to 0x3f and are
good for a while.

Thoughts?

Cheers,
Andre.