* [U-Boot] [PATCH v2 0/2] While trying to compile u-boot in thumb due space constraints on a mxs @ 2018-04-20 19:51 Klaus Goger 2018-04-20 19:51 ` [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible Klaus Goger 2018-04-20 19:51 ` [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe Klaus Goger 0 siblings, 2 replies; 11+ messages in thread From: Klaus Goger @ 2018-04-20 19:51 UTC (permalink / raw) To: u-boot platform it was observed that there are some thumb-interwork issues in the handwritten assembly files. Since the first patch only applies to ARM926EJS and no board on that platform has thumb enabled for now,it was probably never observed. The second one applies to the ARM specific assembly memcpy implementation that is not enabled on any of the boards enabling thumb. grep -l CONFIG_SYS_THUMB_BUILD=y configs/* | \ xargs grep -c CONFIG_USE_ARCH_MEMCPY configs/apalis_imx6_nospl_com_defconfig:0 configs/apalis_imx6_nospl_it_defconfig:0 configs/bk4r1_defconfig:0 configs/colibri_imx6_nospl_defconfig:0 configs/colibri_imx7_defconfig:0 configs/colibri_vf_defconfig:0 configs/highbank_defconfig:0 configs/openrd_base_defconfig:0 configs/openrd_client_defconfig:0 configs/openrd_ultimate_defconfig:0 configs/pcm052_defconfig:0 configs/tbs2910_defconfig:0 configs/x600_defconfig:0 With CONFIG_USE_ARCH_MEMCPY on our mxs platform the speedup for memcopy was about 100%. Changes in v2: - use bl instead of blx to call lowlevel_init - remove mxs tag as it apply to all arm926ejs platforms Klaus Goger (2): arm: make arm926ejs startup code thumb compatible arm: Make arch specific memcpy thumb-safe. arch/arm/cpu/arm926ejs/start.S | 6 +++--- arch/arm/lib/memcpy.S | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible 2018-04-20 19:51 [U-Boot] [PATCH v2 0/2] While trying to compile u-boot in thumb due space constraints on a mxs Klaus Goger @ 2018-04-20 19:51 ` Klaus Goger 2018-04-20 21:55 ` Marek Vasut 2018-04-21 13:10 ` Måns Rullgård 2018-04-20 19:51 ` [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe Klaus Goger 1 sibling, 2 replies; 11+ messages in thread From: Klaus Goger @ 2018-04-20 19:51 UTC (permalink / raw) To: u-boot When building the mxs platform in thumb mode gcc generates code using the intra procedure call scratch register (ip/r12) for the calling the lowlevel_init function. This modifies the lr in flush_dcache which causes u-boot proper to end in an endless loop. 40002334: e1a0c00e mov ip, lr 40002338: eb00df4c bl 4003a070 <__lowlevel_init_from_arm> 4000233c: e1a0e00c mov lr, ip 40002340: e1a0f00e mov pc, lr [...] 4003a070 <__lowlevel_init_from_arm>: 4003a070: e59fc004 ldr ip, [pc, #4] ; 4003a07c <__lowlevel_init_from_arm+0xc> 4003a074: e08fc00c add ip, pc, ip 4003a078: e12fff1c bx ip 4003a07c: fffc86cd .word 0xfffc86cd Instead of using the the ip/r12 register we use sl/r10 to preserve the link register. According to "Procedure Call Standard for the ARM Architecture" by ARM subroutines have to preserve the contents of register r4-r8, r10, r11 and SP. So using r10 instead of r12 should be save. Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> --- Changes in v2: - use bl instead of blx to call lowlevel_init - remove mxs tag as it apply to all arm926ejs platforms arch/arm/cpu/arm926ejs/start.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 959d1ed86d..317df5c401 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -105,9 +105,9 @@ flush_dcache: /* * Go setup Memory and board specific bits prior to relocation. */ - mov ip, lr /* perserve link reg across call */ + mov sl, lr /* perserve link reg across call */ bl lowlevel_init /* go setup pll,mux,memory */ - mov lr, ip /* restore link */ + mov lr, sl /* restore link */ #endif - mov pc, lr /* back to my caller */ + bx lr /* back to my caller */ #endif /* CONFIG_SKIP_LOWLEVEL_INIT */ -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible 2018-04-20 19:51 ` [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible Klaus Goger @ 2018-04-20 21:55 ` Marek Vasut 2018-04-20 22:11 ` klaus.goger at theobroma-systems.com 2018-04-21 13:10 ` Måns Rullgård 1 sibling, 1 reply; 11+ messages in thread From: Marek Vasut @ 2018-04-20 21:55 UTC (permalink / raw) To: u-boot On 04/20/2018 09:51 PM, Klaus Goger wrote: > When building the mxs platform in thumb mode gcc generates code using > the intra procedure call scratch register (ip/r12) for the calling the > lowlevel_init function. This modifies the lr in flush_dcache which > causes u-boot proper to end in an endless loop. > > 40002334: e1a0c00e mov ip, lr > 40002338: eb00df4c bl 4003a070 > <__lowlevel_init_from_arm> > 4000233c: e1a0e00c mov lr, ip > 40002340: e1a0f00e mov pc, lr > [...] > 4003a070 <__lowlevel_init_from_arm>: > 4003a070: e59fc004 ldr ip, [pc, #4] ; 4003a07c > <__lowlevel_init_from_arm+0xc> > 4003a074: e08fc00c add ip, pc, ip > 4003a078: e12fff1c bx ip > 4003a07c: fffc86cd .word 0xfffc86cd > > Instead of using the the ip/r12 register we use sl/r10 to preserve the > link register. > > According to "Procedure Call Standard for the ARM Architecture" by ARM > subroutines have to preserve the contents of register r4-r8, r10, r11 > and SP. So using r10 instead of r12 should be save. > > Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > --- > > Changes in v2: > - use bl instead of blx to call lowlevel_init > - remove mxs tag as it apply to all arm926ejs platforms What is lowlevel_init() is implemented as a C code and therefore thumb ? :-) doesn't ie. armv7a handle this somehow already ? > arch/arm/cpu/arm926ejs/start.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 959d1ed86d..317df5c401 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -105,9 +105,9 @@ flush_dcache: > /* > * Go setup Memory and board specific bits prior to relocation. > */ > - mov ip, lr /* perserve link reg across call */ > + mov sl, lr /* perserve link reg across call */ > bl lowlevel_init /* go setup pll,mux,memory */ > - mov lr, ip /* restore link */ > + mov lr, sl /* restore link */ > #endif > - mov pc, lr /* back to my caller */ > + bx lr /* back to my caller */ > #endif /* CONFIG_SKIP_LOWLEVEL_INIT */ > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible 2018-04-20 21:55 ` Marek Vasut @ 2018-04-20 22:11 ` klaus.goger at theobroma-systems.com 0 siblings, 0 replies; 11+ messages in thread From: klaus.goger at theobroma-systems.com @ 2018-04-20 22:11 UTC (permalink / raw) To: u-boot > On 20.04.2018, at 23:55, Marek Vasut <marex@denx.de> wrote: > > On 04/20/2018 09:51 PM, Klaus Goger wrote: >> When building the mxs platform in thumb mode gcc generates code using >> the intra procedure call scratch register (ip/r12) for the calling the >> lowlevel_init function. This modifies the lr in flush_dcache which >> causes u-boot proper to end in an endless loop. >> >> 40002334: e1a0c00e mov ip, lr >> 40002338: eb00df4c bl 4003a070 >> <__lowlevel_init_from_arm> >> 4000233c: e1a0e00c mov lr, ip >> 40002340: e1a0f00e mov pc, lr >> [...] >> 4003a070 <__lowlevel_init_from_arm>: >> 4003a070: e59fc004 ldr ip, [pc, #4] ; 4003a07c >> <__lowlevel_init_from_arm+0xc> >> 4003a074: e08fc00c add ip, pc, ip >> 4003a078: e12fff1c bx ip >> 4003a07c: fffc86cd .word 0xfffc86cd >> >> Instead of using the the ip/r12 register we use sl/r10 to preserve the >> link register. >> >> According to "Procedure Call Standard for the ARM Architecture" by ARM >> subroutines have to preserve the contents of register r4-r8, r10, r11 >> and SP. So using r10 instead of r12 should be save. >> >> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >> >> --- >> >> Changes in v2: >> - use bl instead of blx to call lowlevel_init >> - remove mxs tag as it apply to all arm926ejs platforms > > What is lowlevel_init() is implemented as a C code and therefore thumb ? > :-) doesn't ie. armv7a handle this somehow already ? lowlevel_init() is implemented in C (you wrote it :)) and therefore thumb if compiled as thumb. armv7 lowlevel_init is implemented in assembly. As the compiler will generate the necessary interwork code there was no need to change the branch instruction to blx. So I changed it back to bl as the original code as this will work with both generated codes (arm and thumb) and also if someone decides to reimplement lowlevel_init as arm assembly. > >> arch/arm/cpu/arm926ejs/start.S | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 959d1ed86d..317df5c401 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -105,9 +105,9 @@ flush_dcache: >> /* >> * Go setup Memory and board specific bits prior to relocation. >> */ >> - mov ip, lr /* perserve link reg across call */ >> + mov sl, lr /* perserve link reg across call */ >> bl lowlevel_init /* go setup pll,mux,memory */ >> - mov lr, ip /* restore link */ >> + mov lr, sl /* restore link */ >> #endif >> - mov pc, lr /* back to my caller */ >> + bx lr /* back to my caller */ >> #endif /* CONFIG_SKIP_LOWLEVEL_INIT */ >> > > > -- > Best regards, > Marek Vasut ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible 2018-04-20 19:51 ` [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible Klaus Goger 2018-04-20 21:55 ` Marek Vasut @ 2018-04-21 13:10 ` Måns Rullgård 2018-04-21 16:45 ` Christoph Müllner 1 sibling, 1 reply; 11+ messages in thread From: Måns Rullgård @ 2018-04-21 13:10 UTC (permalink / raw) To: u-boot Klaus Goger <klaus.goger@theobroma-systems.com> writes: > When building the mxs platform in thumb mode gcc generates code using > the intra procedure call scratch register (ip/r12) for the calling the > lowlevel_init function. This modifies the lr in flush_dcache which > causes u-boot proper to end in an endless loop. > > 40002334: e1a0c00e mov ip, lr > 40002338: eb00df4c bl 4003a070 > <__lowlevel_init_from_arm> > 4000233c: e1a0e00c mov lr, ip > 40002340: e1a0f00e mov pc, lr > [...] > 4003a070 <__lowlevel_init_from_arm>: > 4003a070: e59fc004 ldr ip, [pc, #4] ; 4003a07c > <__lowlevel_init_from_arm+0xc> > 4003a074: e08fc00c add ip, pc, ip > 4003a078: e12fff1c bx ip > 4003a07c: fffc86cd .word 0xfffc86cd > > Instead of using the the ip/r12 register we use sl/r10 to preserve the > link register. > > According to "Procedure Call Standard for the ARM Architecture" by ARM > subroutines have to preserve the contents of register r4-r8, r10, r11 > and SP. So using r10 instead of r12 should be save. > > Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> This problem isn't specific to Thumb mode. An ARM build would also break if the lowlevel_init function happened to clobber r12, which it is permitted to do. It's just dumb luck that this hasn't happened yet. > --- > > Changes in v2: > - use bl instead of blx to call lowlevel_init > - remove mxs tag as it apply to all arm926ejs platforms > > arch/arm/cpu/arm926ejs/start.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 959d1ed86d..317df5c401 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -105,9 +105,9 @@ flush_dcache: > /* > * Go setup Memory and board specific bits prior to relocation. > */ > - mov ip, lr /* perserve link reg across call */ > + mov sl, lr /* perserve link reg across call */ > bl lowlevel_init /* go setup pll,mux,memory */ > - mov lr, ip /* restore link */ > + mov lr, sl /* restore link */ I prefer to use plain register names (r10) rather than the aliases (sl) when not using them for the special functions indicated by the latter. > #endif > - mov pc, lr /* back to my caller */ > + bx lr /* back to my caller */ This change seems unrelated. Yes, bx is the preferred instruction, but using mov here isn't breaking anything. If it bothers you, feel free to make a separate patch fixing all the instances of mov to the pc register, not just this one. -- Måns Rullgård ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible 2018-04-21 13:10 ` Måns Rullgård @ 2018-04-21 16:45 ` Christoph Müllner 0 siblings, 0 replies; 11+ messages in thread From: Christoph Müllner @ 2018-04-21 16:45 UTC (permalink / raw) To: u-boot > On 21 Apr 2018, at 15:10, Måns Rullgård <mans@mansr.com> wrote: > > Klaus Goger <klaus.goger at theobroma-systems.com <mailto:klaus.goger@theobroma-systems.com>> writes: > >> When building the mxs platform in thumb mode gcc generates code using >> the intra procedure call scratch register (ip/r12) for the calling the >> lowlevel_init function. This modifies the lr in flush_dcache which >> causes u-boot proper to end in an endless loop. >> >> 40002334: e1a0c00e mov ip, lr >> 40002338: eb00df4c bl 4003a070 >> <__lowlevel_init_from_arm> >> 4000233c: e1a0e00c mov lr, ip >> 40002340: e1a0f00e mov pc, lr >> [...] >> 4003a070 <__lowlevel_init_from_arm>: >> 4003a070: e59fc004 ldr ip, [pc, #4] ; 4003a07c >> <__lowlevel_init_from_arm+0xc> >> 4003a074: e08fc00c add ip, pc, ip >> 4003a078: e12fff1c bx ip >> 4003a07c: fffc86cd .word 0xfffc86cd >> >> Instead of using the the ip/r12 register we use sl/r10 to preserve the >> link register. >> >> According to "Procedure Call Standard for the ARM Architecture" by ARM >> subroutines have to preserve the contents of register r4-r8, r10, r11 >> and SP. So using r10 instead of r12 should be save. >> >> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > This problem isn't specific to Thumb mode. An ARM build would also > break if the lowlevel_init function happened to clobber r12, which it is > permitted to do. It's just dumb luck that this hasn't happened yet. True. Since we are in low-level code and are not called by any C-functions (we are startup code), using sl/r10 is ok. > >> --- >> >> Changes in v2: >> - use bl instead of blx to call lowlevel_init >> - remove mxs tag as it apply to all arm926ejs platforms >> >> arch/arm/cpu/arm926ejs/start.S | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 959d1ed86d..317df5c401 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -105,9 +105,9 @@ flush_dcache: >> /* >> * Go setup Memory and board specific bits prior to relocation. >> */ >> - mov ip, lr /* perserve link reg across call */ >> + mov sl, lr /* perserve link reg across call */ >> bl lowlevel_init /* go setup pll,mux,memory */ >> - mov lr, ip /* restore link */ >> + mov lr, sl /* restore link */ > > I prefer to use plain register names (r10) rather than the aliases (sl) > when not using them for the special functions indicated by the latter. Ok, will change that. > >> #endif >> - mov pc, lr /* back to my caller */ >> + bx lr /* back to my caller */ > > This change seems unrelated. Yes, bx is the preferred instruction, but > using mov here isn't breaking anything. If it bothers you, feel free to > make a separate patch fixing all the instances of mov to the pc > register, not just this one. We’ll remove it from the patch. Thanks, Christoph -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 874 bytes Desc: Message signed with OpenPGP using GPGMail URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180421/acafbc3b/attachment.sig> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe. 2018-04-20 19:51 [U-Boot] [PATCH v2 0/2] While trying to compile u-boot in thumb due space constraints on a mxs Klaus Goger 2018-04-20 19:51 ` [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible Klaus Goger @ 2018-04-20 19:51 ` Klaus Goger 2018-04-21 13:24 ` Måns Rullgård 1 sibling, 1 reply; 11+ messages in thread From: Klaus Goger @ 2018-04-20 19:51 UTC (permalink / raw) To: u-boot The current arch implementation of memcpy cannot be called from thumb code, because it does not use bx instructions on return. This patch addresses that. Note, that this patch does not touch the hot loop of memcpy, so performance is not affected. Tested on MXS (arm926ejs) with and without thumb-mode enabled. Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> --- Changes in v2: None arch/arm/lib/memcpy.S | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S index 588b3f8971..9e9a193c2a 100644 --- a/arch/arm/lib/memcpy.S +++ b/arch/arm/lib/memcpy.S @@ -62,7 +62,7 @@ #endif ENTRY(memcpy) cmp r0, r1 - moveq pc, lr + bxeq lr enter r4, lr @@ -150,7 +150,8 @@ ENTRY(memcpy) str1b r0, r4, cs, abort=21f str1b r0, ip, cs, abort=21f - exit r4, pc + exit r4, lr + bx lr 9: rsb ip, ip, #4 cmp ip, #2 @@ -259,7 +260,8 @@ ENTRY(memcpy) .endm .macro copy_abort_end - ldmfd sp!, {r4, pc} + ldmfd sp!, {r4, lr} + bx lr .endm ENDPROC(memcpy) -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe. 2018-04-20 19:51 ` [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe Klaus Goger @ 2018-04-21 13:24 ` Måns Rullgård 2018-04-21 16:45 ` Christoph Müllner 0 siblings, 1 reply; 11+ messages in thread From: Måns Rullgård @ 2018-04-21 13:24 UTC (permalink / raw) To: u-boot Klaus Goger <klaus.goger@theobroma-systems.com> writes: > The current arch implementation of memcpy cannot be called > from thumb code, because it does not use bx instructions on return. > This patch addresses that. Note, that this patch does not touch > the hot loop of memcpy, so performance is not affected. > > Tested on MXS (arm926ejs) with and without thumb-mode enabled. > > Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> There are many more instances of mov to pc that ought to be fixed too. Why not do them all at once rather than picking them off one by one as they happen to break things? -- Måns Rullgård ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe. 2018-04-21 13:24 ` Måns Rullgård @ 2018-04-21 16:45 ` Christoph Müllner 2018-04-21 17:00 ` Måns Rullgård 0 siblings, 1 reply; 11+ messages in thread From: Christoph Müllner @ 2018-04-21 16:45 UTC (permalink / raw) To: u-boot > On 21 Apr 2018, at 15:24, Måns Rullgård <mans@mansr.com> wrote: > > Klaus Goger <klaus.goger@theobroma-systems.com> writes: > >> The current arch implementation of memcpy cannot be called >> from thumb code, because it does not use bx instructions on return. >> This patch addresses that. Note, that this patch does not touch >> the hot loop of memcpy, so performance is not affected. >> >> Tested on MXS (arm926ejs) with and without thumb-mode enabled. >> >> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > There are many more instances of mov to pc that ought to be fixed too. > Why not do them all at once rather than picking them off one by one as > they happen to break things? I could not find exit points within memcpy other than those which we fixed. The many other mov pc, $lr instructions are just branches within memcpy. Am I overseeing anything here? Thanks, Christoph -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 874 bytes Desc: Message signed with OpenPGP using GPGMail URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180421/b00e3e48/attachment.sig> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe. 2018-04-21 16:45 ` Christoph Müllner @ 2018-04-21 17:00 ` Måns Rullgård 2018-04-21 17:21 ` Christoph Müllner 0 siblings, 1 reply; 11+ messages in thread From: Måns Rullgård @ 2018-04-21 17:00 UTC (permalink / raw) To: u-boot Christoph Müllner <christoph.muellner@theobroma-systems.com> writes: >> On 21 Apr 2018, at 15:24, Måns Rullgård <mans@mansr.com> wrote: >> >> Klaus Goger <klaus.goger@theobroma-systems.com> writes: >> >>> The current arch implementation of memcpy cannot be called >>> from thumb code, because it does not use bx instructions on return. >>> This patch addresses that. Note, that this patch does not touch >>> the hot loop of memcpy, so performance is not affected. >>> >>> Tested on MXS (arm926ejs) with and without thumb-mode enabled. >>> >>> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> >>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >> >> There are many more instances of mov to pc that ought to be fixed too. >> Why not do them all at once rather than picking them off one by one as >> they happen to break things? > > I could not find exit points within memcpy other than those which we fixed. > The many other mov pc, $lr instructions are just branches within memcpy. > Am I overseeing anything here? I meant elsewhere, not just in memcpy. -- Måns Rullgård ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe. 2018-04-21 17:00 ` Måns Rullgård @ 2018-04-21 17:21 ` Christoph Müllner 0 siblings, 0 replies; 11+ messages in thread From: Christoph Müllner @ 2018-04-21 17:21 UTC (permalink / raw) To: u-boot > On 21 Apr 2018, at 19:00, Måns Rullgård <mans@mansr.com> wrote: > > Christoph Müllner <christoph.muellner at theobroma-systems.com <mailto:christoph.muellner@theobroma-systems.com>> writes: > >>> On 21 Apr 2018, at 15:24, Måns Rullgård <mans@mansr.com> wrote: >>> >>> Klaus Goger <klaus.goger@theobroma-systems.com> writes: >>> >>>> The current arch implementation of memcpy cannot be called >>>> from thumb code, because it does not use bx instructions on return. >>>> This patch addresses that. Note, that this patch does not touch >>>> the hot loop of memcpy, so performance is not affected. >>>> >>>> Tested on MXS (arm926ejs) with and without thumb-mode enabled. >>>> >>>> Signed-off-by: Klaus Goger <klaus.goger@theobroma-systems.com> >>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >>> >>> There are many more instances of mov to pc that ought to be fixed too. >>> Why not do them all at once rather than picking them off one by one as >>> they happen to break things? >> >> I could not find exit points within memcpy other than those which we fixed. >> The many other mov pc, $lr instructions are just branches within memcpy. >> Am I overseeing anything here? > > I meant elsewhere, not just in memcpy. We just bumped in to the start.S and the memcpy code when running with C-code compiled to thumb. We were able to boot a Linux kernel with those changes. Why should we touch code, which we don’t use (and therefore don’t test)? Background is, that we have to meet restricted size limitations for the resulting U-Boot image. Turning off all not-required features was not enough, so we had the idea to compile C-code to thumb (which U-Boot offers via Kconfig option). This did the trick and we wanted to share what’s needed to get this configuration up and running. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 874 bytes Desc: Message signed with OpenPGP using GPGMail URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180421/53b552bd/attachment.sig> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-04-21 17:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-20 19:51 [U-Boot] [PATCH v2 0/2] While trying to compile u-boot in thumb due space constraints on a mxs Klaus Goger 2018-04-20 19:51 ` [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible Klaus Goger 2018-04-20 21:55 ` Marek Vasut 2018-04-20 22:11 ` klaus.goger at theobroma-systems.com 2018-04-21 13:10 ` Måns Rullgård 2018-04-21 16:45 ` Christoph Müllner 2018-04-20 19:51 ` [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe Klaus Goger 2018-04-21 13:24 ` Måns Rullgård 2018-04-21 16:45 ` Christoph Müllner 2018-04-21 17:00 ` Måns Rullgård 2018-04-21 17:21 ` Christoph Müllner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.