[U-Boot] [RESEND PATCH v2 1/2] imx: mx7: fix potential overflow in imx_ddr_size()

From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
---
Changes in v2: None
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach-imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
+ /* cap to max 2 GB */ + if (bits > 31) + bits = 31; + return 1 << bits; }

From: Fabio Estevam festevam@gmail.com
Rather than passing a hardcoded maxsize to the generic get_ram_size() function use the i.MX 7 specific imx_ddr_size() function, which extracts the memory size at runtime by reading the DDR controller registers.
This is a purely cosmetic change as the generic get_ram_size() function already took care of properly automatically detecting 256MB, 512MB or 1GB modules.
Signed-off-by: Fabio Estevam festevam@gmail.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com Acked-by: Stefan Agner stefan.agner@toradex.com
---
Changes in v2: - Added Fabio's and Stefan's acks resp. reviewed-bys.
board/toradex/colibri_imx7/colibri_imx7.c | 2 +- include/configs/colibri_imx7.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/board/toradex/colibri_imx7/colibri_imx7.c b/board/toradex/colibri_imx7/colibri_imx7.c index 2b7591eb00..a4c99626b4 100644 --- a/board/toradex/colibri_imx7/colibri_imx7.c +++ b/board/toradex/colibri_imx7/colibri_imx7.c @@ -52,7 +52,7 @@ DECLARE_GLOBAL_DATA_PTR;
int dram_init(void) { - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); + gd->ram_size = get_ram_size((void *)PHYS_SDRAM, imx_ddr_size());
return 0; } diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h index ff6bd678cf..02849ba35f 100644 --- a/include/configs/colibri_imx7.h +++ b/include/configs/colibri_imx7.h @@ -14,7 +14,6 @@ #include "mx7_common.h"
/*#define CONFIG_DBG_MONITOR*/ -#define PHYS_SDRAM_SIZE SZ_1G
/* Size of malloc() pool */ #define CONFIG_SYS_MALLOC_LEN (32 * SZ_1M)

Hi Marcel,
On 16/10/2018 08:46, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Changes in v2: None
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach-imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
- /* cap to max 2 GB */
- if (bits > 31)
bits = 31;
- return 1 << bits;
}
This is a good catch, thanks for it ! The two patches in V2 (the second for colibri) are already in my list. Does this mean that you want I drop "colibri_imx7: prime get_ram_size() using imx_ddr_size() " ?
Thanks, Stefano

Hi Stefano
On Tue, 2018-10-16 at 10:06 +0200, Stefano Babic wrote:
Hi Marcel,
On 16/10/2018 08:46, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Changes in v2: None
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach- imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
- /* cap to max 2 GB */
- if (bits > 31)
bits = 31;
- return 1 << bits;
}
This is a good catch, thanks for it ! The two patches in V2 (the second for colibri) are already in my list. Does this mean that you want I drop "colibri_imx7: prime get_ram_size() using imx_ddr_size() " ?
No, no. I just resent both as I did not see any activity on it but if you already have them queued that's completely fine. Thanks!
Thanks, Stefano
Cheers
Marcel

On 16/10/2018 10:25, Marcel Ziswiler wrote:
Hi Stefano
On Tue, 2018-10-16 at 10:06 +0200, Stefano Babic wrote:
Hi Marcel,
On 16/10/2018 08:46, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Changes in v2: None
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach- imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
- /* cap to max 2 GB */
- if (bits > 31)
bits = 31;
- return 1 << bits;
}
This is a good catch, thanks for it ! The two patches in V2 (the second for colibri) are already in my list. Does this mean that you want I drop "colibri_imx7: prime get_ram_size() using imx_ddr_size() " ?
No, no. I just resent both as I did not see any activity on it but if you already have them queued that's completely fine. Thanks!
Ah, ok - I had already applied the two patches and you see them on u-boot-imx. However, I have not sent a PR to Tom.
Regards, Stefano

On Tue, 2018-10-16 at 10:37 +0200, Stefano Babic wrote:
On 16/10/2018 10:25, Marcel Ziswiler wrote:
Hi Stefano
On Tue, 2018-10-16 at 10:06 +0200, Stefano Babic wrote:
Hi Marcel,
On 16/10/2018 08:46, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
The imx_ddr_size() function may overflow as it is possible to kind of over provision the DDR controller. Fix this by capping it to 2 GB which is the maximum allowed size as per reference manual.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com
Changes in v2: None
arch/arm/mach-imx/mx7/ddr.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/ddr.c b/arch/arm/mach- imx/mx7/ddr.c index f19aeb8042..9713835bf2 100644 --- a/arch/arm/mach-imx/mx7/ddr.c +++ b/arch/arm/mach-imx/mx7/ddr.c @@ -196,5 +196,9 @@ unsigned int imx_ddr_size(void) if (field_val <= 29) bits++;
- /* cap to max 2 GB */
- if (bits > 31)
bits = 31;
- return 1 << bits;
}
This is a good catch, thanks for it ! The two patches in V2 (the second for colibri) are already in my list. Does this mean that you want I drop "colibri_imx7: prime get_ram_size() using imx_ddr_size() " ?
No, no. I just resent both as I did not see any activity on it but if you already have them queued that's completely fine. Thanks!
Ah, ok - I had already applied the two patches and you see them on u-boot-imx. However, I have not sent a PR to Tom.
Ah, yeah. Sorry, I forgot to check there. Perfect!
Regards, Stefano
Cheers
Marcel
participants (3)
-
Marcel Ziswiler
-
Marcel Ziswiler
-
Stefano Babic