* [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp [not found] <1353769633-31374-1-git-send-email-zhizhou.zh@gmail.com> @ 2012-11-24 15:07 ` Zhi-zhou Zhang 2012-11-24 15:07 ` [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code Zhi-zhou Zhang ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Zhi-zhou Zhang @ 2012-11-24 15:07 UTC (permalink / raw) To: u-boot If bal is 8 bytes aligned, the _gp will not be 8 bytes aligned. then the following ld insntrustion generates a Adel exception. So here make _gp be always aligned in 8 bytes. Signed-off-by: Zhi-zhou Zhang <zhizhou.zh@gmail.com> --- arch/mips/cpu/mips64/start.S | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/mips/cpu/mips64/start.S b/arch/mips/cpu/mips64/start.S index 4112de7..8e8cc33 100644 --- a/arch/mips/cpu/mips64/start.S +++ b/arch/mips/cpu/mips64/start.S @@ -108,7 +108,10 @@ reset: mtc0 t0, CP0_CONFIG #endif - /* Initialize $gp */ + /* Initialize $gp, _gp must be 8 bytes algined. */ + .align 3 + nop + nop bal 1f nop .dword _gp -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code [not found] <1353769633-31374-1-git-send-email-zhizhou.zh@gmail.com> 2012-11-24 15:07 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Zhi-zhou Zhang @ 2012-11-24 15:07 ` Zhi-zhou Zhang 2012-11-25 20:30 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Daniel Schwierzeck [not found] ` <1353769633-31374-2-git-send-email-zhizhou.zh@gmail.com> 3 siblings, 0 replies; 7+ messages in thread From: Zhi-zhou Zhang @ 2012-11-24 15:07 UTC (permalink / raw) To: u-boot Because timestamp is declared as `static', we needn't initialize it by writing it a zero. If we do it before relocate_code, we will write into a flash address(0xffffffffbfc0xxxx). Signed-off-by: Zhi-zhou Zhang <zhizhou.zh@gmail.com> --- arch/mips/cpu/mips32/time.c | 1 - arch/mips/cpu/mips64/time.c | 1 - 2 files changed, 2 deletions(-) diff --git a/arch/mips/cpu/mips32/time.c b/arch/mips/cpu/mips32/time.c index 350896a..09fc842 100644 --- a/arch/mips/cpu/mips32/time.c +++ b/arch/mips/cpu/mips32/time.c @@ -36,7 +36,6 @@ static unsigned long timestamp; int timer_init(void) { /* Set up the timer for the first expiration. */ - timestamp = 0; write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY); return 0; diff --git a/arch/mips/cpu/mips64/time.c b/arch/mips/cpu/mips64/time.c index 5154280..720f7b7 100644 --- a/arch/mips/cpu/mips64/time.c +++ b/arch/mips/cpu/mips64/time.c @@ -37,7 +37,6 @@ static unsigned long timestamp; int timer_init(void) { /* Set up the timer for the first expiration. */ - timestamp = 0; write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY); return 0; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp [not found] <1353769633-31374-1-git-send-email-zhizhou.zh@gmail.com> 2012-11-24 15:07 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Zhi-zhou Zhang 2012-11-24 15:07 ` [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code Zhi-zhou Zhang @ 2012-11-25 20:30 ` Daniel Schwierzeck 2012-11-26 10:58 ` Zhi-zhou Zhang [not found] ` <1353769633-31374-2-git-send-email-zhizhou.zh@gmail.com> 3 siblings, 1 reply; 7+ messages in thread From: Daniel Schwierzeck @ 2012-11-25 20:30 UTC (permalink / raw) To: u-boot 2012/11/24 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > If bal is 8 bytes aligned, the _gp will not be 8 bytes aligned. > then the following ld insntrustion generates a Adel exception. > So here make _gp be always aligned in 8 bytes. which toolchain do you use? Actually _gp is aligned to 16 bytes in the linker script. Thus the instruction "ld gp, 0(ra)" and the address loaded into gp is always aligned to 8 bytes. This works at least with ELDK-5.2.1 and a self-built gcc-4.7.2. > > Signed-off-by: Zhi-zhou Zhang <zhizhou.zh@gmail.com> > --- > arch/mips/cpu/mips64/start.S | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/cpu/mips64/start.S b/arch/mips/cpu/mips64/start.S > index 4112de7..8e8cc33 100644 > --- a/arch/mips/cpu/mips64/start.S > +++ b/arch/mips/cpu/mips64/start.S > @@ -108,7 +108,10 @@ reset: > mtc0 t0, CP0_CONFIG > #endif > > - /* Initialize $gp */ > + /* Initialize $gp, _gp must be 8 bytes algined. */ > + .align 3 > + nop > + nop > bal 1f > nop > .dword _gp > -- > 1.7.9.5 > -- Best regards, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp 2012-11-25 20:30 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Daniel Schwierzeck @ 2012-11-26 10:58 ` Zhi-zhou Zhang 2012-11-28 13:33 ` Daniel Schwierzeck 0 siblings, 1 reply; 7+ messages in thread From: Zhi-zhou Zhang @ 2012-11-26 10:58 UTC (permalink / raw) To: u-boot On Sun, Nov 25, 2012 at 09:30:54PM +0100, Daniel Schwierzeck wrote: > 2012/11/24 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > > If bal is 8 bytes aligned, the _gp will not be 8 bytes aligned. > > then the following ld insntrustion generates a Adel exception. > > So here make _gp be always aligned in 8 bytes. > > which toolchain do you use? Actually _gp is aligned to 16 bytes in the > linker script. > Thus the instruction "ld gp, 0(ra)" and the address loaded into gp is > always aligned to 8 bytes. > This works at least with ELDK-5.2.1 and a self-built gcc-4.7.2. Thanks for review. I have do a simple test by modify this snip as following: .align 3 //nop nop bal 1f nop .dword _gp 1: ld gp, 0(ra) Qemu give me a Adel exception: (qemu) info registers pc=0xffffffffbfc00688 HI=0x0000000000000000 LO=0x0000000000000000 ds 0098 0000000000000000 0 GPR00: r0 0000000000000000 at 000000001000009f v0 0000000000000000 v1 0000000000000000 GPR04: a0 0000000000000000 a1 0000000000000000 a2 0000000000000000 a3 0000000000000000 GPR08: t0 0000000000000000 t1 0000000000000000 t2 0000000000000000 t3 0000000000000000 GPR12: t4 0000000000000002 t5 0000000000000000 t6 0000000000000000 t7 0000000000000000 GPR16: s0 0000000000000000 s1 0000000000000000 s2 0000000000000000 s3 0000000000000000 GPR20: s4 0000000000000000 s5 0000000000000000 s6 0000000000000000 s7 0000000000000000 GPR24: t8 0000000000000000 t9 0000000000000000 k0 0000000000000000 k1 0000000000000000 GPR28: gp 0000000000000000 sp 0000000000000000 s8 0000000000000000 ra ffffffffbfc00544 CP0 Status 0x10400082 Cause 0x00000410 EPC 0xffffffffbfc00550 Config0 0x80004482 Config1 0xfea3519b LLAddr 0x0000000000000000 We can also see the arrange with objdump: ffffffffbfc00538: 00000000 nop ffffffffbfc0053c: 04110004 bal ffffffffbfc00550 ffffffffbfc00540: 00000000 nop ffffffffbfc00544: 00000000 nop ffffffffbfc00548: bfc36ed0 cache 0x3,28368(s8) ffffffffbfc0054c: ffffffff sd ra,-1(ra) ffffffffbfc00550: dffc0000 ld gp,0(ra) It shows that although _gp is aligned to 8 bytes. but it's not in the address of 0(ra). > > > > > Signed-off-by: Zhi-zhou Zhang <zhizhou.zh@gmail.com> > > --- > > arch/mips/cpu/mips64/start.S | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/mips/cpu/mips64/start.S b/arch/mips/cpu/mips64/start.S > > index 4112de7..8e8cc33 100644 > > --- a/arch/mips/cpu/mips64/start.S > > +++ b/arch/mips/cpu/mips64/start.S > > @@ -108,7 +108,10 @@ reset: > > mtc0 t0, CP0_CONFIG > > #endif > > > > - /* Initialize $gp */ > > + /* Initialize $gp, _gp must be 8 bytes algined. */ > > + .align 3 > > + nop > > + nop > > bal 1f > > nop > > .dword _gp > > -- > > 1.7.9.5 > > > > -- > Best regards, > Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp 2012-11-26 10:58 ` Zhi-zhou Zhang @ 2012-11-28 13:33 ` Daniel Schwierzeck 2012-11-28 15:24 ` Zhi-zhou Zhang 0 siblings, 1 reply; 7+ messages in thread From: Daniel Schwierzeck @ 2012-11-28 13:33 UTC (permalink / raw) To: u-boot 2012/11/26 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > On Sun, Nov 25, 2012 at 09:30:54PM +0100, Daniel Schwierzeck wrote: >> 2012/11/24 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: >> > If bal is 8 bytes aligned, the _gp will not be 8 bytes aligned. >> > then the following ld insntrustion generates a Adel exception. >> > So here make _gp be always aligned in 8 bytes. >> >> which toolchain do you use? Actually _gp is aligned to 16 bytes in the >> linker script. >> Thus the instruction "ld gp, 0(ra)" and the address loaded into gp is >> always aligned to 8 bytes. >> This works at least with ELDK-5.2.1 and a self-built gcc-4.7.2. > Thanks for review. > I have do a simple test by modify this snip as following: > .align 3 > //nop > nop > bal 1f > nop > .dword _gp > 1: > ld gp, 0(ra) > Qemu give me a Adel exception: > (qemu) info registers > pc=0xffffffffbfc00688 HI=0x0000000000000000 LO=0x0000000000000000 ds > 0098 0000000000000000 0 > GPR00: r0 0000000000000000 at 000000001000009f v0 0000000000000000 v1 > 0000000000000000 > GPR04: a0 0000000000000000 a1 0000000000000000 a2 0000000000000000 a3 > 0000000000000000 > GPR08: t0 0000000000000000 t1 0000000000000000 t2 0000000000000000 t3 > 0000000000000000 > GPR12: t4 0000000000000002 t5 0000000000000000 t6 0000000000000000 t7 > 0000000000000000 > GPR16: s0 0000000000000000 s1 0000000000000000 s2 0000000000000000 s3 > 0000000000000000 > GPR20: s4 0000000000000000 s5 0000000000000000 s6 0000000000000000 s7 > 0000000000000000 > GPR24: t8 0000000000000000 t9 0000000000000000 k0 0000000000000000 k1 > 0000000000000000 > GPR28: gp 0000000000000000 sp 0000000000000000 s8 0000000000000000 ra > ffffffffbfc00544 > CP0 Status 0x10400082 Cause 0x00000410 EPC 0xffffffffbfc00550 > Config0 0x80004482 Config1 0xfea3519b LLAddr 0x0000000000000000 > > We can also see the arrange with objdump: > ffffffffbfc00538: 00000000 nop > ffffffffbfc0053c: 04110004 bal ffffffffbfc00550 > ffffffffbfc00540: 00000000 nop > ffffffffbfc00544: 00000000 nop > ffffffffbfc00548: bfc36ed0 cache 0x3,28368(s8) > ffffffffbfc0054c: ffffffff sd ra,-1(ra) > ffffffffbfc00550: dffc0000 ld gp,0(ra) > > It shows that although _gp is aligned to 8 bytes. but it's not in the > address of 0(ra). I did some experiments too and now I understand your problem. If you put additional instructions before "bal 1f", the compiler puts a nop between bal and _gp to fill the gap in order to keep _gp aligned. But then the link address of bal that will be stored in the gp register points to the additional nop and not to _gp as it was intended. I can't reproduce this with MIPS32. Maybe this is a compiler bug. I pushed a modified version of your patch to git://git.denx.de/u-boot-mips.git [1]. Could you check if this is okay for you? [1] http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=4bb639edce852c8ed2ae3219e436845495270453 -- Best regards, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp 2012-11-28 13:33 ` Daniel Schwierzeck @ 2012-11-28 15:24 ` Zhi-zhou Zhang 0 siblings, 0 replies; 7+ messages in thread From: Zhi-zhou Zhang @ 2012-11-28 15:24 UTC (permalink / raw) To: u-boot On Wed, Nov 28, 2012 at 02:33:10PM +0100, Daniel Schwierzeck wrote: > 2012/11/26 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > > On Sun, Nov 25, 2012 at 09:30:54PM +0100, Daniel Schwierzeck wrote: > >> 2012/11/24 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > >> > If bal is 8 bytes aligned, the _gp will not be 8 bytes aligned. > >> > then the following ld insntrustion generates a Adel exception. > >> > So here make _gp be always aligned in 8 bytes. > >> > >> which toolchain do you use? Actually _gp is aligned to 16 bytes in the > >> linker script. > >> Thus the instruction "ld gp, 0(ra)" and the address loaded into gp is > >> always aligned to 8 bytes. > >> This works at least with ELDK-5.2.1 and a self-built gcc-4.7.2. > > Thanks for review. > > I have do a simple test by modify this snip as following: > > .align 3 > > //nop > > nop > > bal 1f > > nop > > .dword _gp > > 1: > > ld gp, 0(ra) > > Qemu give me a Adel exception: > > (qemu) info registers > > pc=0xffffffffbfc00688 HI=0x0000000000000000 LO=0x0000000000000000 ds > > 0098 0000000000000000 0 > > GPR00: r0 0000000000000000 at 000000001000009f v0 0000000000000000 v1 > > 0000000000000000 > > GPR04: a0 0000000000000000 a1 0000000000000000 a2 0000000000000000 a3 > > 0000000000000000 > > GPR08: t0 0000000000000000 t1 0000000000000000 t2 0000000000000000 t3 > > 0000000000000000 > > GPR12: t4 0000000000000002 t5 0000000000000000 t6 0000000000000000 t7 > > 0000000000000000 > > GPR16: s0 0000000000000000 s1 0000000000000000 s2 0000000000000000 s3 > > 0000000000000000 > > GPR20: s4 0000000000000000 s5 0000000000000000 s6 0000000000000000 s7 > > 0000000000000000 > > GPR24: t8 0000000000000000 t9 0000000000000000 k0 0000000000000000 k1 > > 0000000000000000 > > GPR28: gp 0000000000000000 sp 0000000000000000 s8 0000000000000000 ra > > ffffffffbfc00544 > > CP0 Status 0x10400082 Cause 0x00000410 EPC 0xffffffffbfc00550 > > Config0 0x80004482 Config1 0xfea3519b LLAddr 0x0000000000000000 > > > > We can also see the arrange with objdump: > > ffffffffbfc00538: 00000000 nop > > ffffffffbfc0053c: 04110004 bal ffffffffbfc00550 > > ffffffffbfc00540: 00000000 nop > > ffffffffbfc00544: 00000000 nop > > ffffffffbfc00548: bfc36ed0 cache 0x3,28368(s8) > > ffffffffbfc0054c: ffffffff sd ra,-1(ra) > > ffffffffbfc00550: dffc0000 ld gp,0(ra) > > > > It shows that although _gp is aligned to 8 bytes. but it's not in the > > address of 0(ra). > > I did some experiments too and now I understand your problem. If you put > additional instructions before "bal 1f", the compiler puts a nop between bal > and _gp to fill the gap in order to keep _gp aligned. But then the link address > of bal that will be stored in the gp register points to the additional > nop and not > to _gp as it was intended. I can't reproduce this with MIPS32. > Maybe this is a compiler bug. Dear Daniel, I'm sorry for my poor Engilsh. I didn't describe the problem exactly. I think tlink script only set the value of _gp. and .dword do decide where _gp be linked. Because the compiler always align dword to 8 bytes, the gap comes and was filled with zero. The MIPS's ISA set 4 bytes per instruction. so MIPS's link address is always aligned to 4 bytes. MIPS32 won't occur this problem. But if the link address is not aligned to 8 bytes, MIPS64 goes wrong. We have half chance encountering it. Thanks. > > I pushed a modified version of your patch to > git://git.denx.de/u-boot-mips.git [1]. > Could you check if this is okay for you? > > [1] http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=4bb639edce852c8ed2ae3219e436845495270453 > > -- > Best regards, > Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <1353769633-31374-2-git-send-email-zhizhou.zh@gmail.com>]
* [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code [not found] ` <1353769633-31374-2-git-send-email-zhizhou.zh@gmail.com> @ 2012-11-25 20:56 ` Daniel Schwierzeck 0 siblings, 0 replies; 7+ messages in thread From: Daniel Schwierzeck @ 2012-11-25 20:56 UTC (permalink / raw) To: u-boot 2012/11/24 Zhi-zhou Zhang <zhizhou.zh@gmail.com>: > Because timestamp is declared as `static', we needn't initialize > it by writing it a zero. If we do it before relocate_code, we > will write into a flash address(0xffffffffbfc0xxxx). > > Signed-off-by: Zhi-zhou Zhang <zhizhou.zh@gmail.com> > --- > arch/mips/cpu/mips32/time.c | 1 - > arch/mips/cpu/mips64/time.c | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/arch/mips/cpu/mips32/time.c b/arch/mips/cpu/mips32/time.c > index 350896a..09fc842 100644 > --- a/arch/mips/cpu/mips32/time.c > +++ b/arch/mips/cpu/mips32/time.c > @@ -36,7 +36,6 @@ static unsigned long timestamp; > int timer_init(void) > { > /* Set up the timer for the first expiration. */ > - timestamp = 0; > write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY); > > return 0; > diff --git a/arch/mips/cpu/mips64/time.c b/arch/mips/cpu/mips64/time.c > index 5154280..720f7b7 100644 > --- a/arch/mips/cpu/mips64/time.c > +++ b/arch/mips/cpu/mips64/time.c > @@ -37,7 +37,6 @@ static unsigned long timestamp; > int timer_init(void) > { > /* Set up the timer for the first expiration. */ > - timestamp = 0; > write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY); > > return 0; > -- > 1.7.9.5 > applied to u-boot-mips/master with a tiny fixup of the commit summary, thanks -- Best regards, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-11-28 15:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1353769633-31374-1-git-send-email-zhizhou.zh@gmail.com>
2012-11-24 15:07 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Zhi-zhou Zhang
2012-11-24 15:07 ` [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code Zhi-zhou Zhang
2012-11-25 20:30 ` [U-Boot] [Patch 1/2] MIPS: fix a latent bug on initialize $gp Daniel Schwierzeck
2012-11-26 10:58 ` Zhi-zhou Zhang
2012-11-28 13:33 ` Daniel Schwierzeck
2012-11-28 15:24 ` Zhi-zhou Zhang
[not found] ` <1353769633-31374-2-git-send-email-zhizhou.zh@gmail.com>
2012-11-25 20:56 ` [U-Boot] [Patch 2/2] MIPS: do not modify variable before relocate_code Daniel Schwierzeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox