[PATCH v1] MIPS: remove .bss to __rel_start overlay

[PATCH v1] MIPS: remove .bss to __rel_start overlay

[Oleksij Rempel]

.bss __rel_start (OVERLAY) was used to optimize RAM size used by
barebox. Since .bss and __rel_start overlap, we should clear bss only
after __rel_start was used.

There is a choice of moving .bss clear sequence after __rel_start or
remove this optimization. Since the use of this optimization is minimal
and danger to trap in to similar issue is still high, i prefer to remove
this optimization.

Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/mips/lib/barebox.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
index 693a778980..c954df41f3 100644
--- a/arch/mips/lib/barebox.lds.S
+++ b/arch/mips/lib/barebox.lds.S
@@ -59,7 +59,7 @@ SECTIONS
 
 	_end = .;
 
-	.bss __rel_start (OVERLAY) : {
+	.bss : {
 		__bss_start = .;
 		*(.sbss.*)
 		*(.bss.*)
-- 
2.25.0


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

Re: [PATCH v1] MIPS: remove .bss to __rel_start overlay

[Antony Pavlov]

On Tue, 28 Jan 2020 10:28:32 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

Hi!

Looks like this patch has some undocummented side effects
(or in other words, it makes undocummented fixups).

I have tested this patch on qemu-malta with current master,
ee8960aec018df2de ("ARM: imx6: properly check for IPU presence").
It works fine: memtest works, iomem output looks good.

But the patch fixes several issues and commit message does not contain
any information about it.

I rebuild current master with qemu-malta_defconfig and MMU enabled.

I used this qemu cmdline to enable qemu monitor:

qemu-system-mips -nodefaults -M malta -m 256 -nographic -bios barebox-flash-image
   -net user -net nic,model=rtl8139 -serial mon:stdio

With '-serial mon:stdio' one can invoke qemu monitor with "ctrl-a c" keypress sequence.

So I tried 'info registers' qemu monitor command after barebox startup:

Hit any to stop autoboot:    3
barebox@qemu malta:/ 
barebox@qemu malta:/ 
barebox@qemu malta:/ iomem
0x00000000 - 0xffffffff (size 0x00000000) iomem
  0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
  0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
  0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
  0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
  0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
barebox@qemu malta:/ QEMU 4.2.0 monitor - type 'help' for more information
(qemu) info registers 
pc=0xa081232c HI=0x00000026 LO=0x20000000 ds 0090 a081232c 1
GPR00: r0 00000000 at 00000000 v0 b80003f8 v1 00000000
GPR04: a0 a0408668 a1 b80003fd a2 b80003f8 a3 ffffffff
GPR08: t0 00000001 t1 00000000 t2 0000005a t3 00000023
GPR12: t4 00000000 t5 00010000 t6 00000040 t7 00000010
GPR16: s0 a0408668 s1 a085cd20 s2 a0860000 s3 a0860000
GPR20: s4 a0860000 s5 00000400 s6 a08613c0 s7 00000001
GPR24: t8 00000008 t9 a0812340 k0 00400000 k1 fffffffa
GPR28: gp 00000000 sp 8fb8fd10 s8 ffffffff ra a0812420
CP0 Status  0x00000000 Cause   0x00000400 EPC    0x00000000
    Config0 0x80008482 Config1 0x9e190c8f LLAddr 0x0000000000000000
    Config2 0x80000000 Config3 0x00000000
    Config4 0x00000000 Config5 0x00000000
(qemu) quit

So I see several issues in current master at once:

  * iomem output has no information on sdram regions, so memtest is unusable;
  * pc=0xa081232c, relocation does not work, barebox is located with 8M offset
    from start of RAM. The board has 256M and relocation routine
    should move barebox code much higher;
  * pc=0xa081232c, so barebox code works from KSEG1 not from KSEG0 as MMU=y option implies.

Your patch fixes all these symptoms at ones however the commit message
says nothing about them.

> .bss __rel_start (OVERLAY) was used to optimize RAM size used by
> barebox. Since .bss and __rel_start overlap, we should clear bss only
> after __rel_start was used.
> 
> There is a choice of moving .bss clear sequence after __rel_start or
> remove this optimization. Since the use of this optimization is minimal
> and danger to trap in to similar issue is still high, i prefer to remove
> this optimization.
> 
> Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  arch/mips/lib/barebox.lds.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
> index 693a778980..c954df41f3 100644
> --- a/arch/mips/lib/barebox.lds.S
> +++ b/arch/mips/lib/barebox.lds.S
> @@ -59,7 +59,7 @@ SECTIONS
>  
>  	_end = .;
>  
> -	.bss __rel_start (OVERLAY) : {
> +	.bss : {
>  		__bss_start = .;
>  		*(.sbss.*)
>  		*(.bss.*)
> -- 
> 2.25.0
> 


-- 
Best regards,
  Antony Pavlov

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

Re: [PATCH v1] MIPS: remove .bss to __rel_start overlay

[Oleksij Rempel]

On 28.01.20 12:55, Antony Pavlov wrote:
> On Tue, 28 Jan 2020 10:28:32 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> Hi!
> 
> Looks like this patch has some undocummented side effects
> (or in other words, it makes undocummented fixups).
> 
> I have tested this patch on qemu-malta with current master,
> ee8960aec018df2de ("ARM: imx6: properly check for IPU presence").
> It works fine: memtest works, iomem output looks good.
> 
> But the patch fixes several issues and commit message does not contain
> any information about it.
> 
> I rebuild current master with qemu-malta_defconfig and MMU enabled.
> 
> I used this qemu cmdline to enable qemu monitor:
> 
> qemu-system-mips -nodefaults -M malta -m 256 -nographic -bios barebox-flash-image
>     -net user -net nic,model=rtl8139 -serial mon:stdio
> 
> With '-serial mon:stdio' one can invoke qemu monitor with "ctrl-a c" keypress sequence.
> 
> So I tried 'info registers' qemu monitor command after barebox startup:
> 
> Hit any to stop autoboot:    3
> barebox@qemu malta:/
> barebox@qemu malta:/
> barebox@qemu malta:/ iomem
> 0x00000000 - 0xffffffff (size 0x00000000) iomem
>    0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
>    0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
>    0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
>    0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
>    0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
> barebox@qemu malta:/ QEMU 4.2.0 monitor - type 'help' for more information
> (qemu) info registers
> pc=0xa081232c HI=0x00000026 LO=0x20000000 ds 0090 a081232c 1
> GPR00: r0 00000000 at 00000000 v0 b80003f8 v1 00000000
> GPR04: a0 a0408668 a1 b80003fd a2 b80003f8 a3 ffffffff
> GPR08: t0 00000001 t1 00000000 t2 0000005a t3 00000023
> GPR12: t4 00000000 t5 00010000 t6 00000040 t7 00000010
> GPR16: s0 a0408668 s1 a085cd20 s2 a0860000 s3 a0860000
> GPR20: s4 a0860000 s5 00000400 s6 a08613c0 s7 00000001
> GPR24: t8 00000008 t9 a0812340 k0 00400000 k1 fffffffa
> GPR28: gp 00000000 sp 8fb8fd10 s8 ffffffff ra a0812420
> CP0 Status  0x00000000 Cause   0x00000400 EPC    0x00000000
>      Config0 0x80008482 Config1 0x9e190c8f LLAddr 0x0000000000000000
>      Config2 0x80000000 Config3 0x00000000
>      Config4 0x00000000 Config5 0x00000000
> (qemu) quit
> 
> So I see several issues in current master at once:
> 
>    * iomem output has no information on sdram regions, so memtest is unusable;
>    * pc=0xa081232c, relocation does not work, barebox is located with 8M offset
>      from start of RAM. The board has 256M and relocation routine
>      should move barebox code much higher;
>    * pc=0xa081232c, so barebox code works from KSEG1 not from KSEG0 as MMU=y option implies.
> 
> Your patch fixes all these symptoms at ones however the commit message
> says nothing about them.

OK, thx! I'll update commit message.

>> .bss __rel_start (OVERLAY) was used to optimize RAM size used by
>> barebox. Since .bss and __rel_start overlap, we should clear bss only
>> after __rel_start was used.
>>
>> There is a choice of moving .bss clear sequence after __rel_start or
>> remove this optimization. Since the use of this optimization is minimal
>> and danger to trap in to similar issue is still high, i prefer to remove
>> this optimization.
>>
>> Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>   arch/mips/lib/barebox.lds.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
>> index 693a778980..c954df41f3 100644
>> --- a/arch/mips/lib/barebox.lds.S
>> +++ b/arch/mips/lib/barebox.lds.S
>> @@ -59,7 +59,7 @@ SECTIONS
>>   
>>   	_end = .;
>>   
>> -	.bss __rel_start (OVERLAY) : {
>> +	.bss : {
>>   		__bss_start = .;
>>   		*(.sbss.*)
>>   		*(.bss.*)
>> -- 
>> 2.25.0
>>
> 
> 

Kind regards,
Oleksij Rempel

-- 
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 v1] MIPS: remove .bss to __rel_start overlay

[Peter Mamonov]

On Tue, Jan 28, 2020 at 02:55:13PM +0300, Antony Pavlov wrote:
> On Tue, 28 Jan 2020 10:28:32 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> Hi!
> 
> Looks like this patch has some undocummented side effects
> (or in other words, it makes undocummented fixups).
> 
> I have tested this patch on qemu-malta with current master,
> ee8960aec018df2de ("ARM: imx6: properly check for IPU presence").
> It works fine: memtest works, iomem output looks good.
> 
> But the patch fixes several issues and commit message does not contain
> any information about it.
> 
> I rebuild current master with qemu-malta_defconfig and MMU enabled.
> 
> I used this qemu cmdline to enable qemu monitor:
> 
> qemu-system-mips -nodefaults -M malta -m 256 -nographic -bios barebox-flash-image
>    -net user -net nic,model=rtl8139 -serial mon:stdio
> 
> With '-serial mon:stdio' one can invoke qemu monitor with "ctrl-a c" keypress sequence.
> 
> So I tried 'info registers' qemu monitor command after barebox startup:
> 
> Hit any to stop autoboot:    3
> barebox@qemu malta:/ 
> barebox@qemu malta:/ 
> barebox@qemu malta:/ iomem
> 0x00000000 - 0xffffffff (size 0x00000000) iomem
>   0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
>   0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
>   0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
>   0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
>   0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
> barebox@qemu malta:/ QEMU 4.2.0 monitor - type 'help' for more information
> (qemu) info registers 
> pc=0xa081232c HI=0x00000026 LO=0x20000000 ds 0090 a081232c 1
> GPR00: r0 00000000 at 00000000 v0 b80003f8 v1 00000000
> GPR04: a0 a0408668 a1 b80003fd a2 b80003f8 a3 ffffffff
> GPR08: t0 00000001 t1 00000000 t2 0000005a t3 00000023
> GPR12: t4 00000000 t5 00010000 t6 00000040 t7 00000010
> GPR16: s0 a0408668 s1 a085cd20 s2 a0860000 s3 a0860000
> GPR20: s4 a0860000 s5 00000400 s6 a08613c0 s7 00000001
> GPR24: t8 00000008 t9 a0812340 k0 00400000 k1 fffffffa
> GPR28: gp 00000000 sp 8fb8fd10 s8 ffffffff ra a0812420
> CP0 Status  0x00000000 Cause   0x00000400 EPC    0x00000000
>     Config0 0x80008482 Config1 0x9e190c8f LLAddr 0x0000000000000000
>     Config2 0x80000000 Config3 0x00000000
>     Config4 0x00000000 Config5 0x00000000
> (qemu) quit
> 
> So I see several issues in current master at once:
> 
>   * iomem output has no information on sdram regions, so memtest is unusable;
>   * pc=0xa081232c, relocation does not work, barebox is located with 8M offset
>     from start of RAM. The board has 256M and relocation routine
>     should move barebox code much higher;
>   * pc=0xa081232c, so barebox code works from KSEG1 not from KSEG0 as MMU=y option implies.
> 
> Your patch fixes all these symptoms at ones however the commit message
> says nothing about them.

I've tested the patch, everything looks right now:

	barebox@qemu malta:/ iomem
	0x00000000 - 0xffffffff (size 0x00000000) iomem
	  0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
	  0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
	  0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
	  0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
	  0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
	    0x8fb89000 - 0x8fb8ffff (size 0x00007000) stack
	    0x8fb90000 - 0x8ff8ffff (size 0x00400000) malloc space
	    0x8ff90000 - 0x8ffeae9f (size 0x0005aea0) barebox
	    0x8ffeaea0 - 0x8ffefadf (size 0x00004c40) barebox data
	    0x8fff7ae0 - 0x8fffbbdf (size 0x00004100) bss
	barebox@qemu malta:/ whereami
	code   @ 8ffb9bcc (8ffb9bc0)
	data   @ 8ffef3c0
	bss    @ 8fff9ae0
	heap   @ 8fbb9518
	stack  @ 8fb8fd50
	barebox@qemu malta:/ 

However I have to enable CONFIG_MMU after `make qemu-malta_defconfig`, or I get 
`ERROR: Error: Cannot request SDRAM region for stack`, and no barebox memory 
areas in `iomem` output.  CONFIG_MMU seems to be enabled for ath79 only:

	$ git grep CONFIG_MMU arch/mips/configs/
	arch/mips/configs/ath79_defconfig:CONFIG_MMU=y
	$

Regards,
Peter


> 
> > .bss __rel_start (OVERLAY) was used to optimize RAM size used by
> > barebox. Since .bss and __rel_start overlap, we should clear bss only
> > after __rel_start was used.
> > 
> > There is a choice of moving .bss clear sequence after __rel_start or
> > remove this optimization. Since the use of this optimization is minimal
> > and danger to trap in to similar issue is still high, i prefer to remove
> > this optimization.
> > 
> > Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  arch/mips/lib/barebox.lds.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
> > index 693a778980..c954df41f3 100644
> > --- a/arch/mips/lib/barebox.lds.S
> > +++ b/arch/mips/lib/barebox.lds.S
> > @@ -59,7 +59,7 @@ SECTIONS
> >  
> >  	_end = .;
> >  
> > -	.bss __rel_start (OVERLAY) : {
> > +	.bss : {
> >  		__bss_start = .;
> >  		*(.sbss.*)
> >  		*(.bss.*)
> > -- 
> > 2.25.0
> > 
> 
> 
> -- 
> Best regards,
>   Antony Pavlov

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

Re: [PATCH v1] MIPS: remove .bss to __rel_start overlay

[Oleksij Rempel]

On 28.01.20 14:06, Peter Mamonov wrote:
> On Tue, Jan 28, 2020 at 02:55:13PM +0300, Antony Pavlov wrote:
>> On Tue, 28 Jan 2020 10:28:32 +0100
>> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>>
>> Hi!
>>
>> Looks like this patch has some undocummented side effects
>> (or in other words, it makes undocummented fixups).
>>
>> I have tested this patch on qemu-malta with current master,
>> ee8960aec018df2de ("ARM: imx6: properly check for IPU presence").
>> It works fine: memtest works, iomem output looks good.
>>
>> But the patch fixes several issues and commit message does not contain
>> any information about it.
>>
>> I rebuild current master with qemu-malta_defconfig and MMU enabled.
>>
>> I used this qemu cmdline to enable qemu monitor:
>>
>> qemu-system-mips -nodefaults -M malta -m 256 -nographic -bios barebox-flash-image
>>     -net user -net nic,model=rtl8139 -serial mon:stdio
>>
>> With '-serial mon:stdio' one can invoke qemu monitor with "ctrl-a c" keypress sequence.
>>
>> So I tried 'info registers' qemu monitor command after barebox startup:
>>
>> Hit any to stop autoboot:    3
>> barebox@qemu malta:/
>> barebox@qemu malta:/
>> barebox@qemu malta:/ iomem
>> 0x00000000 - 0xffffffff (size 0x00000000) iomem
>>    0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
>>    0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
>>    0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
>>    0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
>>    0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
>> barebox@qemu malta:/ QEMU 4.2.0 monitor - type 'help' for more information
>> (qemu) info registers
>> pc=0xa081232c HI=0x00000026 LO=0x20000000 ds 0090 a081232c 1
>> GPR00: r0 00000000 at 00000000 v0 b80003f8 v1 00000000
>> GPR04: a0 a0408668 a1 b80003fd a2 b80003f8 a3 ffffffff
>> GPR08: t0 00000001 t1 00000000 t2 0000005a t3 00000023
>> GPR12: t4 00000000 t5 00010000 t6 00000040 t7 00000010
>> GPR16: s0 a0408668 s1 a085cd20 s2 a0860000 s3 a0860000
>> GPR20: s4 a0860000 s5 00000400 s6 a08613c0 s7 00000001
>> GPR24: t8 00000008 t9 a0812340 k0 00400000 k1 fffffffa
>> GPR28: gp 00000000 sp 8fb8fd10 s8 ffffffff ra a0812420
>> CP0 Status  0x00000000 Cause   0x00000400 EPC    0x00000000
>>      Config0 0x80008482 Config1 0x9e190c8f LLAddr 0x0000000000000000
>>      Config2 0x80000000 Config3 0x00000000
>>      Config4 0x00000000 Config5 0x00000000
>> (qemu) quit
>>
>> So I see several issues in current master at once:
>>
>>    * iomem output has no information on sdram regions, so memtest is unusable;
>>    * pc=0xa081232c, relocation does not work, barebox is located with 8M offset
>>      from start of RAM. The board has 256M and relocation routine
>>      should move barebox code much higher;
>>    * pc=0xa081232c, so barebox code works from KSEG1 not from KSEG0 as MMU=y option implies.
>>
>> Your patch fixes all these symptoms at ones however the commit message
>> says nothing about them.
> 
> I've tested the patch, everything looks right now:
> 
> 	barebox@qemu malta:/ iomem
> 	0x00000000 - 0xffffffff (size 0x00000000) iomem
> 	  0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
> 	  0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
> 	  0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
> 	  0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
> 	  0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
> 	    0x8fb89000 - 0x8fb8ffff (size 0x00007000) stack
> 	    0x8fb90000 - 0x8ff8ffff (size 0x00400000) malloc space
> 	    0x8ff90000 - 0x8ffeae9f (size 0x0005aea0) barebox
> 	    0x8ffeaea0 - 0x8ffefadf (size 0x00004c40) barebox data
> 	    0x8fff7ae0 - 0x8fffbbdf (size 0x00004100) bss
> 	barebox@qemu malta:/ whereami
> 	code   @ 8ffb9bcc (8ffb9bc0)
> 	data   @ 8ffef3c0
> 	bss    @ 8fff9ae0
> 	heap   @ 8fbb9518
> 	stack  @ 8fb8fd50
> 	barebox@qemu malta:/
> 
> However I have to enable CONFIG_MMU after `make qemu-malta_defconfig`, or I get
> `ERROR: Error: Cannot request SDRAM region for stack`, and no barebox memory
> areas in `iomem` output.  CONFIG_MMU seems to be enabled for ath79 only:
> 
> 	$ git grep CONFIG_MMU arch/mips/configs/
> 	arch/mips/configs/ath79_defconfig:CONFIG_MMU=y
> 	$

Yes, this should be fixed in a separate patch.


Kind regards,
Oleksij Rempel

-- 
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 v1] MIPS: remove .bss to __rel_start overlay

[Oleksij Rempel]

On 28.01.20 14:43, Oleksij Rempel wrote:
> 
> 
> On 28.01.20 14:06, Peter Mamonov wrote:
>> On Tue, Jan 28, 2020 at 02:55:13PM +0300, Antony Pavlov wrote:
>>> On Tue, 28 Jan 2020 10:28:32 +0100
>>> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>>>
>>> Hi!
>>>
>>> Looks like this patch has some undocummented side effects
>>> (or in other words, it makes undocummented fixups).
>>>
>>> I have tested this patch on qemu-malta with current master,
>>> ee8960aec018df2de ("ARM: imx6: properly check for IPU presence").
>>> It works fine: memtest works, iomem output looks good.
>>>
>>> But the patch fixes several issues and commit message does not contain
>>> any information about it.
>>>
>>> I rebuild current master with qemu-malta_defconfig and MMU enabled.
>>>
>>> I used this qemu cmdline to enable qemu monitor:
>>>
>>> qemu-system-mips -nodefaults -M malta -m 256 -nographic -bios barebox-flash-image
>>>     -net user -net nic,model=rtl8139 -serial mon:stdio
>>>
>>> With '-serial mon:stdio' one can invoke qemu monitor with "ctrl-a c" keypress sequence.
>>>
>>> So I tried 'info registers' qemu monitor command after barebox startup:
>>>
>>> Hit any to stop autoboot:    3
>>> barebox@qemu malta:/
>>> barebox@qemu malta:/
>>> barebox@qemu malta:/ iomem
>>> 0x00000000 - 0xffffffff (size 0x00000000) iomem
>>>    0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
>>>    0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
>>>    0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
>>>    0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
>>>    0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
>>> barebox@qemu malta:/ QEMU 4.2.0 monitor - type 'help' for more information
>>> (qemu) info registers
>>> pc=0xa081232c HI=0x00000026 LO=0x20000000 ds 0090 a081232c 1
>>> GPR00: r0 00000000 at 00000000 v0 b80003f8 v1 00000000
>>> GPR04: a0 a0408668 a1 b80003fd a2 b80003f8 a3 ffffffff
>>> GPR08: t0 00000001 t1 00000000 t2 0000005a t3 00000023
>>> GPR12: t4 00000000 t5 00010000 t6 00000040 t7 00000010
>>> GPR16: s0 a0408668 s1 a085cd20 s2 a0860000 s3 a0860000
>>> GPR20: s4 a0860000 s5 00000400 s6 a08613c0 s7 00000001
>>> GPR24: t8 00000008 t9 a0812340 k0 00400000 k1 fffffffa
>>> GPR28: gp 00000000 sp 8fb8fd10 s8 ffffffff ra a0812420
>>> CP0 Status  0x00000000 Cause   0x00000400 EPC    0x00000000
>>>      Config0 0x80008482 Config1 0x9e190c8f LLAddr 0x0000000000000000
>>>      Config2 0x80000000 Config3 0x00000000
>>>      Config4 0x00000000 Config5 0x00000000
>>> (qemu) quit
>>>
>>> So I see several issues in current master at once:
>>>
>>>    * iomem output has no information on sdram regions, so memtest is unusable;
>>>    * pc=0xa081232c, relocation does not work, barebox is located with 8M offset
>>>      from start of RAM. The board has 256M and relocation routine
>>>      should move barebox code much higher;
>>>    * pc=0xa081232c, so barebox code works from KSEG1 not from KSEG0 as MMU=y option 
>>> implies.
>>>
>>> Your patch fixes all these symptoms at ones however the commit message
>>> says nothing about them.
>>
>> I've tested the patch, everything looks right now:
>>
>>     barebox@qemu malta:/ iomem
>>     0x00000000 - 0xffffffff (size 0x00000000) iomem
>>       0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
>>       0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
>>       0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
>>       0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
>>       0x80000000 - 0x8fffffff (size 0x10000000) kseg0_ram0
>>         0x8fb89000 - 0x8fb8ffff (size 0x00007000) stack
>>         0x8fb90000 - 0x8ff8ffff (size 0x00400000) malloc space
>>         0x8ff90000 - 0x8ffeae9f (size 0x0005aea0) barebox
>>         0x8ffeaea0 - 0x8ffefadf (size 0x00004c40) barebox data
>>         0x8fff7ae0 - 0x8fffbbdf (size 0x00004100) bss
>>     barebox@qemu malta:/ whereami
>>     code   @ 8ffb9bcc (8ffb9bc0)
>>     data   @ 8ffef3c0
>>     bss    @ 8fff9ae0
>>     heap   @ 8fbb9518
>>     stack  @ 8fb8fd50
>>     barebox@qemu malta:/
>>
>> However I have to enable CONFIG_MMU after `make qemu-malta_defconfig`, or I get
>> `ERROR: Error: Cannot request SDRAM region for stack`, and no barebox memory
>> areas in `iomem` output.  CONFIG_MMU seems to be enabled for ath79 only:
>>
>>     $ git grep CONFIG_MMU arch/mips/configs/
>>     arch/mips/configs/ath79_defconfig:CONFIG_MMU=y
>>     $
> 
> Yes, this should be fixed in a separate patch.

Note:
memtest on ar9331 works only with this patch:

diff --git a/arch/mips/lib/cpu-probe.c b/arch/mips/lib/cpu-probe.c
index cbde43a595..0d2dcf8b03 100644
--- a/arch/mips/lib/cpu-probe.c
+++ b/arch/mips/lib/cpu-probe.c
@@ -177,6 +177,9 @@ static int mips_request_stack(void)
         if (!request_sdram_region("stack", mips_stack_top - STACK_SIZE, STACK_SIZE))
                 pr_err("Error: Cannot request SDRAM region for stack\n");

+       if (!request_sdram_region("vector", 0x80000000, 0x8000))
+               pr_err("Error: Cannot request SDRAM region for vector\n");
+
         return 0;
  }
  coredevice_initcall(mips_request_stack);


Kind regards,
Oleksij Rempel

-- 
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 v1] MIPS: remove .bss to __rel_start overlay

[Antony Pavlov]

On Tue, 28 Jan 2020 14:53:07 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Note:
> memtest on ar9331 works only with this patch:
> 
> diff --git a/arch/mips/lib/cpu-probe.c b/arch/mips/lib/cpu-probe.c
> index cbde43a595..0d2dcf8b03 100644
> --- a/arch/mips/lib/cpu-probe.c
> +++ b/arch/mips/lib/cpu-probe.c
> @@ -177,6 +177,9 @@ static int mips_request_stack(void)
>          if (!request_sdram_region("stack", mips_stack_top - STACK_SIZE, STACK_SIZE))
>                  pr_err("Error: Cannot request SDRAM region for stack\n");
> 
> +       if (!request_sdram_region("vector", 0x80000000, 0x8000))
> +               pr_err("Error: Cannot request SDRAM region for vector\n");
> +
>          return 0;
>   }
>   coredevice_initcall(mips_request_stack);


Can we put this request_sdram_region() into main_entry.c to keep it close to trap_init?
Thereby we have a change to reuse ebase value instead of the 0x80000000 magic constant.
Also 0x8000 size is too large. See MIPS Run, 2nd Edition by Dominic Sweetman states 
general exception entry point is BASE+0x180 (used by barebox), and Interrupt Special
starts at BASE+0x200 (not used by barebox), so 0x200 is just enough.

-- 
Best regards,
  Antony Pavlov

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

Re: [PATCH v1] MIPS: remove .bss to __rel_start overlay

[Antony Pavlov]

On Tue, 28 Jan 2020 10:28:32 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

Hi!

Have you tested the patch on real hardware?

-- 
Best regards,
  Antony Pavlov

> .bss __rel_start (OVERLAY) was used to optimize RAM size used by
> barebox. Since .bss and __rel_start overlap, we should clear bss only
> after __rel_start was used.
> 
> There is a choice of moving .bss clear sequence after __rel_start or
> remove this optimization. Since the use of this optimization is minimal
> and danger to trap in to similar issue is still high, i prefer to remove
> this optimization.
> 
> Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  arch/mips/lib/barebox.lds.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
> index 693a778980..c954df41f3 100644
> --- a/arch/mips/lib/barebox.lds.S
> +++ b/arch/mips/lib/barebox.lds.S
> @@ -59,7 +59,7 @@ SECTIONS
>  
>  	_end = .;
>  
> -	.bss __rel_start (OVERLAY) : {
> +	.bss : {
>  		__bss_start = .;
>  		*(.sbss.*)
>  		*(.bss.*)
> -- 
> 2.25.0
> 


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

Re: [PATCH v1] MIPS: remove .bss to __rel_start overlay

[Oleksij Rempel]

On Tue, Jan 28, 2020 at 03:54:13PM +0300, Antony Pavlov wrote:
> On Tue, 28 Jan 2020 10:28:32 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> Hi!
> 
> Have you tested the patch on real hardware?

here is a log from ar9331 based DPTechnics DPT-Module:
barebox 2020.01.0-00101-g70fcc51b10-dirty #815 Tue Jan 28 10:10:42 CET 2020


Board: DPTechnics DPT-Module
mdio_bus: miibus0: probed
ag71xx-gmac 19000000.ethernet@19000000.of: network device registered
ag71xx-gmac 1a000000.ethernet@1a000000.of: probe failed: No such file or directory
m25p80 w25q128@00: w25q128 (16384 Kbytes)
netconsole: registered as netconsole-1
malloc space: 0x837a0000 -> 0x83f9ffff (size 8 MiB)
qca-art chosen:art@0.of: bad MAC addr
qca-art chosen:art@0.of: probe failed: error 84

Hit any to stop autoboot:    3
barebox@DPTechnics DPT-Module:/
barebox@DPTechnics DPT-Module:/
barebox@DPTechnics DPT-Module:/ iomem 
0x00000000 - 0xffffffff (size 0x00000000) iomem
  0x18020000 - 0x18020013 (size 0x00000014) 18020000.uart@18020000.of 
  0x18040000 - 0x18040033 (size 0x00000034) 18040000.gpio@18040000.of
  0x18050000 - 0x180500ff (size 0x00000100) 18050000.pll-controller@18050000.of
  0x18060008 - 0x1806000f (size 0x00000008) 18060008.wdt@18060008.of
  0x18070000 - 0x180700ff (size 0x00000100) 19000000.ethernet@19000000.of
  0x19000000 - 0x190001ff (size 0x00000200) 19000000.ethernet@19000000.of
  0x1f000000 - 0x1f00000f (size 0x00000010) 1f000000.spi@1f000000.of
  0x80000000 - 0x83ffffff (size 0x04000000) kseg0_ram0
    0x83798000 - 0x8379ffff (size 0x00008000) stack
    0x837a0000 - 0x83f9ffff (size 0x00800000) malloc space
    0x83fa0000 - 0x83ff509f (size 0x000550a0) barebox
    0x83ff50a0 - 0x83ff99ff (size 0x00004960) barebox data
barebox@DPTechnics DPT-Module:/ go 0
## Starting application at 0x00000000 ...

Ooops, TLB miss on load or ifetch!

$ 0   : 00000000 00000020 00000000 80010000
$ 4   : 00000001 837d1930 00010000 00008000
$ 8   : 837a0008 837a9c68 837a9c68 837a9e14
$12   : 837a0000 0000000c 0000000c 837d1924
$16   : 00000000 83ff8764 00000002 837d192c
$20   : 00000001 8379feb8 00000002 837d261c
$24   : 00000000 00000000                  
$28   : 810b4520 8379fd38 00000000 83fc3568
Hi    : 00048150
Lo    : 20000000
epc   : 00000000
ra    : 83fc3568
Status: 10000002
Cause : 50008008
Config: 80208483

### ERROR ### Please RESET the board ###

The oops should confirm that iomem show correct information.

> 
> -- 
> Best regards,
>   Antony Pavlov
> 
> > .bss __rel_start (OVERLAY) was used to optimize RAM size used by
> > barebox. Since .bss and __rel_start overlap, we should clear bss only
> > after __rel_start was used.
> > 
> > There is a choice of moving .bss clear sequence after __rel_start or
> > remove this optimization. Since the use of this optimization is minimal
> > and danger to trap in to similar issue is still high, i prefer to remove
> > this optimization.
> > 
> > Fixes: 1e5aef61fc6a444 ("MIPS: reloc: init bss and cpu")
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  arch/mips/lib/barebox.lds.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
> > index 693a778980..c954df41f3 100644
> > --- a/arch/mips/lib/barebox.lds.S
> > +++ b/arch/mips/lib/barebox.lds.S
> > @@ -59,7 +59,7 @@ SECTIONS
> >  
> >  	_end = .;
> >  
> > -	.bss __rel_start (OVERLAY) : {
> > +	.bss : {
> >  		__bss_start = .;
> >  		*(.sbss.*)
> >  		*(.bss.*)
> > -- 
> > 2.25.0
> > 
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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