[PATCH 1/2] memtest: fix 4GB overflow fail

[PATCH 1/2] memtest: fix 4GB overflow fail

[Dmitry Lavnikevich]

Add condition for checking size_t overflow. This fixes memtest fail
> Memtest failed. Error: -22
which appears when 4GB RAM is present.

Was tested on phyFLEX-i.MX6 modules with 4GB and 1GB RAM.

Signed-off-by: Dmitry Lavnikevich <d.lavnikevich@sam-solutions.com>
---
 commands/memtest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/memtest.c b/commands/memtest.c
index d384436afb73..5e7896cc711c 100644
--- a/commands/memtest.c
+++ b/commands/memtest.c
@@ -109,7 +109,7 @@ static int request_memtest_regions(struct list_head *list)
 		start = PAGE_ALIGN(r->end);
 		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
 		size = end - start + 1;
-		if (start < end) {
+		if (start < end && start > r->end) {
 			ret = alloc_memtest_region(list, start, size);
 			if (ret < 0)
 				return ret;
-- 
2.2.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] dts:imx6:pfla02: Align nand partitions by 512k

[Dmitry Lavnikevich]

NAND chip used on phyFLEX-i.MX6 with 4GB NAND (MT29F32G08AFACAWP) has
512k eraseblock size.

Align NAND partitions by 512k to fix erase fails.

Signed-off-by: Dmitry Lavnikevich <d.lavnikevich@sam-solutions.com>
---
 arch/arm/dts/imx6qdl-phytec-pfla02.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
index 32ce088fee48..52b279057a33 100644
--- a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
+++ b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
@@ -95,22 +95,22 @@
 
 	partition@1 {
 		label = "barebox-environment";
-		reg = <0x400000 0x20000>;
+		reg = <0x400000 0x80000>;
 	};
 
 	partition@2 {
 		label = "oftree";
-		reg = <0x420000 0x20000>;
+		reg = <0x480000 0x80000>;
 	};
 
 	partition@3 {
 		label = "kernel";
-		reg = <0x440000 0x800000>;
+		reg = <0x500000 0x800000>;
 	};
 
 	partition@4 {
 		label = "root";
-		reg = <0xC40000 0x0>;
+		reg = <0xD00000 0x0>;
 	};
 };
 
-- 
2.2.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] memtest: fix 4GB overflow fail

[Sascha Hauer]

(Adding Alexander Aring)

On Fri, Jan 23, 2015 at 02:41:41PM +0300, Dmitry Lavnikevich wrote:
> Add condition for checking size_t overflow. This fixes memtest fail
> > Memtest failed. Error: -22
> which appears when 4GB RAM is present.
> 
> Was tested on phyFLEX-i.MX6 modules with 4GB and 1GB RAM.
> 
> Signed-off-by: Dmitry Lavnikevich <d.lavnikevich@sam-solutions.com>
> ---
>  commands/memtest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/commands/memtest.c b/commands/memtest.c
> index d384436afb73..5e7896cc711c 100644
> --- a/commands/memtest.c
> +++ b/commands/memtest.c
> @@ -109,7 +109,7 @@ static int request_memtest_regions(struct list_head *list)
>  		start = PAGE_ALIGN(r->end);
>  		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
>  		size = end - start + 1;
> -		if (start < end) {
> +		if (start < end && start > r->end) {

This is still wrong since the calculation of end and size before that is
already wrong.

Could you give the following patch a test?

Sascha

-----------------------------8<-------------------------------

From 33ddaad19a253468ca5c67782b8d26bb9aadba14 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Mon, 26 Jan 2015 09:59:32 +0100
Subject: [PATCH] memtest: Fix SDRAM size calculations

Calculating the size and end of the test region as

  end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
  size = end - start + 1;

is wrong. For an example resource of

  start = 0x80000000, end = 0x8fffffff

end results in:

  end = PAGE_ALIGN_DOWN(0x8fffffff) - 1 = 0x8fffefff

instead of 0x8fffffff. The size is then calculated to

  size = end - start + 1 = 0x8fffefff - 0x80000000 + 1 = 0x0ffff000

instead of 0x10000000

The correct way to do this is to calculate the real size and apply a
PAGE_ALIGN_DOWN afterwards:

  size = PAGE_ALIGN_DOWN(bank->res->end - start + 1) = 0x10000000

Fix this in three different places.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/memtest.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/commands/memtest.c b/commands/memtest.c
index d384436..379d82e 100644
--- a/commands/memtest.c
+++ b/commands/memtest.c
@@ -60,8 +60,7 @@ static int request_memtest_regions(struct list_head *list)
 		 */
 		if (list_empty(&bank->res->children)) {
 			start = PAGE_ALIGN(bank->res->start);
-			end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
-			size = end - start + 1;
+			size = PAGE_ALIGN_DOWN(bank->res->end - start + 1);
 
 			ret = alloc_memtest_region(list, start, size);
 			if (ret < 0)
@@ -89,13 +88,13 @@ static int request_memtest_regions(struct list_head *list)
 		 * Between used regions. Start from second entry.
 		 */
 		list_for_each_entry_from(r, &bank->res->children, sibling) {
-			start = PAGE_ALIGN(r_prev->end);
-			end = PAGE_ALIGN_DOWN(r->start);
+			start = PAGE_ALIGN(r_prev->end + 1);
+			end = r->start - 1;
 			r_prev = r;
 			if (start >= end)
 				continue;
 
-			size = end - start;
+			size = PAGE_ALIGN_DOWN(end - start + 1);
 			ret = alloc_memtest_region(list, start, size);
 			if (ret < 0)
 				return ret;
@@ -107,8 +106,8 @@ static int request_memtest_regions(struct list_head *list)
 		r = list_last_entry(&bank->res->children,
 				     struct resource, sibling);
 		start = PAGE_ALIGN(r->end);
-		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
-		size = end - start + 1;
+		end = bank->res->end;
+		size = PAGE_ALIGN_DOWN(end - start + 1);
 		if (start < end) {
 			ret = alloc_memtest_region(list, start, size);
 			if (ret < 0)
-- 
2.1.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] memtest: fix 4GB overflow fail

[Dmitry Lavnikevich]

On 26/01/15 12:09, Sascha Hauer wrote:
> (Adding Alexander Aring)
> @@ -107,8 +106,8 @@ static int request_memtest_regions(struct list_head *list)
>   		r = list_last_entry(&bank->res->children,
>   				     struct resource, sibling);
>   		start = PAGE_ALIGN(r->end);
still here we have overflow.
Last region (stack) on 4GB systems ends on 0xfffffff7. So aligned up, 
start=0x00000000.
After that
> -		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
> -		size = end - start + 1;
> +		end = bank->res->end;
> +		size = PAGE_ALIGN_DOWN(end - start + 1);
>   		if (start < end) {
This check detects that there are some space after last region 
(start=0x00000000) till end of bank (0xffffffff) and then
>   			ret = alloc_memtest_region(list, start, size);
>   			if (ret < 0)
fails to allocate region from 0x00000000 for size 0x00000000.


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] dts:imx6:pfla02: Align nand partitions by 512k

[Sascha Hauer]

+ Teresa, Jan and Christian

On Fri, Jan 23, 2015 at 02:48:55PM +0300, Dmitry Lavnikevich wrote:
> NAND chip used on phyFLEX-i.MX6 with 4GB NAND (MT29F32G08AFACAWP) has
> 512k eraseblock size.
> 
> Align NAND partitions by 512k to fix erase fails.
> 
> Signed-off-by: Dmitry Lavnikevich <d.lavnikevich@sam-solutions.com>
> ---
>  arch/arm/dts/imx6qdl-phytec-pfla02.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> index 32ce088fee48..52b279057a33 100644
> --- a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> +++ b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> @@ -95,22 +95,22 @@
>  
>  	partition@1 {
>  		label = "barebox-environment";
> -		reg = <0x400000 0x20000>;
> +		reg = <0x400000 0x80000>;
>  	};
>  
>  	partition@2 {
>  		label = "oftree";
> -		reg = <0x420000 0x20000>;
> +		reg = <0x480000 0x80000>;
>  	};

The partition size for barebox-environment and the devicetree used to be
one eraseblock only. Now it's again only one eraseblock at least for the
4k Page size variants. If one of those blocks is bad the board becomes
unusable. I think the partitions should be at least two eraseblocks.
That of course would make them quite big.

Any opinions about this?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] memtest: fix 4GB overflow fail

[Sascha Hauer]

On Mon, Jan 26, 2015 at 02:09:03PM +0300, Dmitry Lavnikevich wrote:
> On 26/01/15 12:09, Sascha Hauer wrote:
> >(Adding Alexander Aring)
> >@@ -107,8 +106,8 @@ static int request_memtest_regions(struct list_head *list)
> >  		r = list_last_entry(&bank->res->children,
> >  				     struct resource, sibling);
> >  		start = PAGE_ALIGN(r->end);
> still here we have overflow.
> Last region (stack) on 4GB systems ends on 0xfffffff7. So aligned
> up, start=0x00000000.
> After that
> >-		end = PAGE_ALIGN_DOWN(bank->res->end) - 1;
> >-		size = end - start + 1;
> >+		end = bank->res->end;
> >+		size = PAGE_ALIGN_DOWN(end - start + 1);
> >  		if (start < end) {
> This check detects that there are some space after last region
> (start=0x00000000) till end of bank (0xffffffff) and then
> >  			ret = alloc_memtest_region(list, start, size);
> >  			if (ret < 0)
> fails to allocate region from 0x00000000 for size 0x00000000.

Ok, so your original patch is still necessary. I applied both my patch
and yours to -next.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] dts:imx6:pfla02: Align nand partitions by 512k

[Christian Hemp]

Hello Sascha,

Am Dienstag, 27. Januar 2015, 09:09:27 schrieb Sascha Hauer:
> + Teresa, Jan and Christian
> 
> On Fri, Jan 23, 2015 at 02:48:55PM +0300, Dmitry Lavnikevich wrote:
> > NAND chip used on phyFLEX-i.MX6 with 4GB NAND (MT29F32G08AFACAWP) has
> > 512k eraseblock size.
> > 
> > Align NAND partitions by 512k to fix erase fails.
> > 
> > Signed-off-by: Dmitry Lavnikevich <d.lavnikevich@sam-solutions.com>
> > ---
> > 
> >  arch/arm/dts/imx6qdl-phytec-pfla02.dtsi | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> > b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi index
> > 32ce088fee48..52b279057a33 100644
> > --- a/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> > +++ b/arch/arm/dts/imx6qdl-phytec-pfla02.dtsi
> > @@ -95,22 +95,22 @@
> > 
> >  	partition@1 {
> >  	
> >  		label = "barebox-environment";
> > 
> > -		reg = <0x400000 0x20000>;
> > +		reg = <0x400000 0x80000>;
> > 
> >  	};
> >  	
> >  	partition@2 {
> >  	
> >  		label = "oftree";
> > 
> > -		reg = <0x420000 0x20000>;
> > +		reg = <0x480000 0x80000>;
> > 
> >  	};
> 
> The partition size for barebox-environment and the devicetree used to be
> one eraseblock only. Now it's again only one eraseblock at least for the
> 4k Page size variants. If one of those blocks is bad the board becomes
> unusable. I think the partitions should be at least two eraseblocks.
> That of course would make them quite big.
> 
> Any opinions about this?

we agree with your opinion.

Chritian

> 
> Sascha


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] dts:imx6:pfla02: Align nand partitions by 512k

[Dmitry Lavnikevich]

On 27/01/15 12:35, Christian Hemp wrote:
> Hello Sascha,
>
> Am Dienstag, 27. Januar 2015, 09:09:27 schrieb Sascha Hauer:
>>
>> The partition size for barebox-environment and the devicetree used to be
>> one eraseblock only. Now it's again only one eraseblock at least for the
>> 4k Page size variants. If one of those blocks is bad the board becomes
>> unusable. I think the partitions should be at least two eraseblocks.
>> That of course would make them quite big.
>>
>> Any opinions about this?
>
> we agree with your opinion.
Ok, then I will correct partitions to be aligned by two eraseblocks and 
will resend patch.


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox