* [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses @ 2020-06-24 23:34 Angelo Dureghello 2020-06-25 8:13 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: Angelo Dureghello @ 2020-06-24 23:34 UTC (permalink / raw) To: gerg; +Cc: linux-m68k, Angelo Dureghello On mcf5441x, mmu enabled, using a small initramfs, the boot was recently hanging silently in the initial phase, inside arch/m68k/mm/init.c, m68k_setup_node() initrd at 0x47d2f000:0x47d82f64 overlap at 1073741889 for chunk 0 overlap at 1073746176 for chunk 0 overlap at 1073746736 for chunk 0 overlap at 1073746737 for chunk 0 overlap at 1073746738 for chunk 0 overlap at 1073746739 for chunk 0 overlap at 1073746740 for chunk 0 From some debugging, at m68k_setup_node(), the 25-bits shift value (virt_to_node_shift) seems unset, because applied to a wrong address (address previously set from m68k_fixup). Tried some fixes, but the more low-risk and hopefully correct seems to just add a volatile inside __virt_to_node_shift(). This seems to produce correct addresses for the next fixup to work. Cannot test this on other mmu ColdFire cpu's so every test is eventually welcome. Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> --- arch/m68k/include/asm/page_mm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/m68k/include/asm/page_mm.h b/arch/m68k/include/asm/page_mm.h index e6b75992192b..1eff386cc64b 100644 --- a/arch/m68k/include/asm/page_mm.h +++ b/arch/m68k/include/asm/page_mm.h @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) { int shift; - asm ( + asm volatile ( "1: moveq #0,%0\n" m68k_fixup(%c1, 1b) : "=d" (shift) -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses 2020-06-24 23:34 [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses Angelo Dureghello @ 2020-06-25 8:13 ` Geert Uytterhoeven 2020-06-26 20:14 ` Angelo Dureghello 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2020-06-25 8:13 UTC (permalink / raw) To: Angelo Dureghello; +Cc: Greg Ungerer, Linux/m68k Hi Angelo, Thanks for your patch! On Thu, Jun 25, 2020 at 1:28 AM Angelo Dureghello <angelo.dureghello@timesys.com> wrote: > On mcf5441x, mmu enabled, using a small initramfs, the boot was > recently hanging silently in the initial phase, inside > arch/m68k/mm/init.c, m68k_setup_node() > > initrd at 0x47d2f000:0x47d82f64 > overlap at 1073741889 for chunk 0 > overlap at 1073746176 for chunk 0 > overlap at 1073746736 for chunk 0 > overlap at 1073746737 for chunk 0 > overlap at 1073746738 for chunk 0 > overlap at 1073746739 for chunk 0 > overlap at 1073746740 for chunk 0 > > From some debugging, at m68k_setup_node(), the 25-bits shift value > (virt_to_node_shift) seems unset, because applied to a wrong > address (address previously set from m68k_fixup). > > Tried some fixes, but the more low-risk and hopefully correct seems > to just add a volatile inside __virt_to_node_shift(). This seems to > produce correct addresses for the next fixup to work. Cannot test this > on other mmu ColdFire cpu's so every test is eventually welcome. > > Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> > --- a/arch/m68k/include/asm/page_mm.h > +++ b/arch/m68k/include/asm/page_mm.h > @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) > { > int shift; > > - asm ( > + asm volatile ( > "1: moveq #0,%0\n" > m68k_fixup(%c1, 1b) > : "=d" (shift) Perhaps we should add the volatile to the other asm statements, too? Andreas? Which compiler version are you using? Have you compared the difference in generated code? With gcc version 8.4.0 (Ubuntu 8.4.0-1ubuntu1~18.04), the above makes no difference for m68k_setup_node(), and the compiler doesn't even optimize away the call to __virt_to_node_shift(), which is marked __attribute_const__. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses 2020-06-25 8:13 ` Geert Uytterhoeven @ 2020-06-26 20:14 ` Angelo Dureghello 2020-07-13 19:41 ` Angelo Dureghello 0 siblings, 1 reply; 7+ messages in thread From: Angelo Dureghello @ 2020-06-26 20:14 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Greg Ungerer, Linux/m68k Hi Geert ! thanks a lot for the feedbacks. On Thu, Jun 25, 2020 at 10:13 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Angelo, > > Thanks for your patch! > > On Thu, Jun 25, 2020 at 1:28 AM Angelo Dureghello > <angelo.dureghello@timesys.com> wrote: > > On mcf5441x, mmu enabled, using a small initramfs, the boot was > > recently hanging silently in the initial phase, inside > > arch/m68k/mm/init.c, m68k_setup_node() > > > > initrd at 0x47d2f000:0x47d82f64 > > overlap at 1073741889 for chunk 0 > > overlap at 1073746176 for chunk 0 > > overlap at 1073746736 for chunk 0 > > overlap at 1073746737 for chunk 0 > > overlap at 1073746738 for chunk 0 > > overlap at 1073746739 for chunk 0 > > overlap at 1073746740 for chunk 0 > > > > From some debugging, at m68k_setup_node(), the 25-bits shift value > > (virt_to_node_shift) seems unset, because applied to a wrong > > address (address previously set from m68k_fixup). > > > > Tried some fixes, but the more low-risk and hopefully correct seems > > to just add a volatile inside __virt_to_node_shift(). This seems to > > produce correct addresses for the next fixup to work. Cannot test this > > on other mmu ColdFire cpu's so every test is eventually welcome. > > > > Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> > > > --- a/arch/m68k/include/asm/page_mm.h > > +++ b/arch/m68k/include/asm/page_mm.h > > @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) > > { > > int shift; > > > > - asm ( > > + asm volatile ( > > "1: moveq #0,%0\n" > > m68k_fixup(%c1, 1b) > > : "=d" (shift) > > Perhaps we should add the volatile to the other asm statements, too? > Andreas? I see volatile is there in other similar m68k asm code chunks, it should not hurt. > > Which compiler version are you using? It's a toolchain i prepared up here years ago, export CROSS_COMPILE=/opt/toolchains/m68k/gcc-5.2.0-nolibc/bin/m68k-linux- If you have a link for a newer toolchain, happy to upgrade :) > Have you compared the difference in generated code? > > With gcc version 8.4.0 (Ubuntu 8.4.0-1ubuntu1~18.04), the above makes > no difference for m68k_setup_node(), and the compiler doesn't even > optimize away the call to __virt_to_node_shift(), which is marked > __attribute_const__. > Well, i only checked the assembly by objdump in object files, right now, at least looking the m68k_fixup section in both cases, opcodes seems ok. with volatile: 00000000 <.m68k_fixup>: 0: 4e71 nop ... 6: R_68K_32 .init.text+0x1e a: 4e71 nop c: 4e71 nop e: 0000 .short 0x0000 10: 0001 .short 0x0001 12: 0000 .short 0x0000 12: R_68K_32 .init.text+0x22 correct, points to 22: 7200 moveq #0,%d1 14: 0000 .short 0x0000 16: 4e71 nop 18: 4e71 nop ... 1e: R_68K_32 .init.text+0x46 22: 4e71 nop without 00000000 <.m68k_fixup>: 0: 4e71 nop 2: 0000 .short 0x0000 4: 0001 .short 0x0001 6: 0000 .short 0x0000 6: R_68K_32 .init.text+0x10, corrrect, points to 10: 7200 moveq #0,%d1 8: 0000 .short 0x0000 a: 4e71 nop c: 4e71 nop Do you see anything strange ? Or, how can i objdump better ? > Gr{oetje,eeting}s, > > Geert > Thanks, regards angelo > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses 2020-06-26 20:14 ` Angelo Dureghello @ 2020-07-13 19:41 ` Angelo Dureghello 2020-07-14 13:01 ` Greg Ungerer 0 siblings, 1 reply; 7+ messages in thread From: Angelo Dureghello @ 2020-07-13 19:41 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Greg Ungerer, Linux/m68k Hi Geert and all, tested now building by kernel.org toolchain 10.1.0, i can boot with or without the above added volatile, jfyi. Regards, angelo On Fri, Jun 26, 2020 at 10:14 PM Angelo Dureghello <angelo.dureghello@timesys.com> wrote: > > Hi Geert ! > > thanks a lot for the feedbacks. > > On Thu, Jun 25, 2020 at 10:13 AM Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > > > Hi Angelo, > > > > Thanks for your patch! > > > > On Thu, Jun 25, 2020 at 1:28 AM Angelo Dureghello > > <angelo.dureghello@timesys.com> wrote: > > > On mcf5441x, mmu enabled, using a small initramfs, the boot was > > > recently hanging silently in the initial phase, inside > > > arch/m68k/mm/init.c, m68k_setup_node() > > > > > > initrd at 0x47d2f000:0x47d82f64 > > > overlap at 1073741889 for chunk 0 > > > overlap at 1073746176 for chunk 0 > > > overlap at 1073746736 for chunk 0 > > > overlap at 1073746737 for chunk 0 > > > overlap at 1073746738 for chunk 0 > > > overlap at 1073746739 for chunk 0 > > > overlap at 1073746740 for chunk 0 > > > > > > From some debugging, at m68k_setup_node(), the 25-bits shift value > > > (virt_to_node_shift) seems unset, because applied to a wrong > > > address (address previously set from m68k_fixup). > > > > > > Tried some fixes, but the more low-risk and hopefully correct seems > > > to just add a volatile inside __virt_to_node_shift(). This seems to > > > produce correct addresses for the next fixup to work. Cannot test this > > > on other mmu ColdFire cpu's so every test is eventually welcome. > > > > > > Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> > > > > > --- a/arch/m68k/include/asm/page_mm.h > > > +++ b/arch/m68k/include/asm/page_mm.h > > > @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) > > > { > > > int shift; > > > > > > - asm ( > > > + asm volatile ( > > > "1: moveq #0,%0\n" > > > m68k_fixup(%c1, 1b) > > > : "=d" (shift) > > > > Perhaps we should add the volatile to the other asm statements, too? > > Andreas? > > I see volatile is there in other similar m68k asm code chunks, > it should not hurt. > > > > > Which compiler version are you using? > > It's a toolchain i prepared up here years ago, > export CROSS_COMPILE=/opt/toolchains/m68k/gcc-5.2.0-nolibc/bin/m68k-linux- > > If you have a link for a newer toolchain, happy to upgrade :) > > > Have you compared the difference in generated code? > > > > With gcc version 8.4.0 (Ubuntu 8.4.0-1ubuntu1~18.04), the above makes > > no difference for m68k_setup_node(), and the compiler doesn't even > > optimize away the call to __virt_to_node_shift(), which is marked > > __attribute_const__. > > > > Well, i only checked the assembly by objdump in object files, right now, > at least looking the m68k_fixup section in both cases, opcodes seems ok. > > with volatile: > > 00000000 <.m68k_fixup>: > 0: 4e71 nop > ... > 6: R_68K_32 .init.text+0x1e > a: 4e71 nop > c: 4e71 nop > e: 0000 .short 0x0000 > 10: 0001 .short 0x0001 > 12: 0000 .short 0x0000 > 12: R_68K_32 .init.text+0x22 correct, points to > 22: 7200 moveq #0,%d1 > > 14: 0000 .short 0x0000 > 16: 4e71 nop > 18: 4e71 nop > ... > 1e: R_68K_32 .init.text+0x46 > 22: 4e71 nop > > > without > > 00000000 <.m68k_fixup>: > 0: 4e71 nop > 2: 0000 .short 0x0000 > 4: 0001 .short 0x0001 > 6: 0000 .short 0x0000 > 6: R_68K_32 .init.text+0x10, corrrect, points to > 10: 7200 moveq #0,%d1 > > 8: 0000 .short 0x0000 > a: 4e71 nop > c: 4e71 nop > > Do you see anything strange ? Or, how can i objdump better ? > > > Gr{oetje,eeting}s, > > > > Geert > > > > Thanks, regards > angelo > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like that. > > -- Linus Torvalds -- Angelo Dureghello Timesys e. angelo.dureghello@timesys.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses 2020-07-13 19:41 ` Angelo Dureghello @ 2020-07-14 13:01 ` Greg Ungerer 2020-07-14 15:41 ` Angelo Dureghello 0 siblings, 1 reply; 7+ messages in thread From: Greg Ungerer @ 2020-07-14 13:01 UTC (permalink / raw) To: Angelo Dureghello, Geert Uytterhoeven; +Cc: Linux/m68k Hi Angelo, On 14/7/20 5:41 am, Angelo Dureghello wrote: > Hi Geert and all, > > tested now building by kernel.org toolchain 10.1.0, > i can boot with or without the above added volatile, jfyi. So is the net out that this was a compiler/toolchain problem? Regards Greg > Regards, > angelo > > On Fri, Jun 26, 2020 at 10:14 PM Angelo Dureghello > <angelo.dureghello@timesys.com> wrote: >> >> Hi Geert ! >> >> thanks a lot for the feedbacks. >> >> On Thu, Jun 25, 2020 at 10:13 AM Geert Uytterhoeven >> <geert@linux-m68k.org> wrote: >>> >>> Hi Angelo, >>> >>> Thanks for your patch! >>> >>> On Thu, Jun 25, 2020 at 1:28 AM Angelo Dureghello >>> <angelo.dureghello@timesys.com> wrote: >>>> On mcf5441x, mmu enabled, using a small initramfs, the boot was >>>> recently hanging silently in the initial phase, inside >>>> arch/m68k/mm/init.c, m68k_setup_node() >>>> >>>> initrd at 0x47d2f000:0x47d82f64 >>>> overlap at 1073741889 for chunk 0 >>>> overlap at 1073746176 for chunk 0 >>>> overlap at 1073746736 for chunk 0 >>>> overlap at 1073746737 for chunk 0 >>>> overlap at 1073746738 for chunk 0 >>>> overlap at 1073746739 for chunk 0 >>>> overlap at 1073746740 for chunk 0 >>>> >>>> From some debugging, at m68k_setup_node(), the 25-bits shift value >>>> (virt_to_node_shift) seems unset, because applied to a wrong >>>> address (address previously set from m68k_fixup). >>>> >>>> Tried some fixes, but the more low-risk and hopefully correct seems >>>> to just add a volatile inside __virt_to_node_shift(). This seems to >>>> produce correct addresses for the next fixup to work. Cannot test this >>>> on other mmu ColdFire cpu's so every test is eventually welcome. >>>> >>>> Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> >>> >>>> --- a/arch/m68k/include/asm/page_mm.h >>>> +++ b/arch/m68k/include/asm/page_mm.h >>>> @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) >>>> { >>>> int shift; >>>> >>>> - asm ( >>>> + asm volatile ( >>>> "1: moveq #0,%0\n" >>>> m68k_fixup(%c1, 1b) >>>> : "=d" (shift) >>> >>> Perhaps we should add the volatile to the other asm statements, too? >>> Andreas? >> >> I see volatile is there in other similar m68k asm code chunks, >> it should not hurt. >> >>> >>> Which compiler version are you using? >> >> It's a toolchain i prepared up here years ago, >> export CROSS_COMPILE=/opt/toolchains/m68k/gcc-5.2.0-nolibc/bin/m68k-linux- >> >> If you have a link for a newer toolchain, happy to upgrade :) >> >>> Have you compared the difference in generated code? >>> >>> With gcc version 8.4.0 (Ubuntu 8.4.0-1ubuntu1~18.04), the above makes >>> no difference for m68k_setup_node(), and the compiler doesn't even >>> optimize away the call to __virt_to_node_shift(), which is marked >>> __attribute_const__. >>> >> >> Well, i only checked the assembly by objdump in object files, right now, >> at least looking the m68k_fixup section in both cases, opcodes seems ok. >> >> with volatile: >> >> 00000000 <.m68k_fixup>: >> 0: 4e71 nop >> ... >> 6: R_68K_32 .init.text+0x1e >> a: 4e71 nop >> c: 4e71 nop >> e: 0000 .short 0x0000 >> 10: 0001 .short 0x0001 >> 12: 0000 .short 0x0000 >> 12: R_68K_32 .init.text+0x22 correct, points to >> 22: 7200 moveq #0,%d1 >> >> 14: 0000 .short 0x0000 >> 16: 4e71 nop >> 18: 4e71 nop >> ... >> 1e: R_68K_32 .init.text+0x46 >> 22: 4e71 nop >> >> >> without >> >> 00000000 <.m68k_fixup>: >> 0: 4e71 nop >> 2: 0000 .short 0x0000 >> 4: 0001 .short 0x0001 >> 6: 0000 .short 0x0000 >> 6: R_68K_32 .init.text+0x10, corrrect, points to >> 10: 7200 moveq #0,%d1 >> >> 8: 0000 .short 0x0000 >> a: 4e71 nop >> c: 4e71 nop >> >> Do you see anything strange ? Or, how can i objdump better ? >> >>> Gr{oetje,eeting}s, >>> >>> Geert >>> >> >> Thanks, regards >> angelo >> >>> -- >>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org >>> >>> In personal conversations with technical people, I call myself a hacker. But >>> when I'm talking to journalists I just say "programmer" or something like that. >>> -- Linus Torvalds > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses 2020-07-14 13:01 ` Greg Ungerer @ 2020-07-14 15:41 ` Angelo Dureghello 0 siblings, 0 replies; 7+ messages in thread From: Angelo Dureghello @ 2020-07-14 15:41 UTC (permalink / raw) To: Greg Ungerer; +Cc: Geert Uytterhoeven, Linux/m68k Hi Greg and all, On Tue, Jul 14, 2020 at 3:01 PM Greg Ungerer <gerg@linux-m68k.org> wrote: > > Hi Angelo, > > On 14/7/20 5:41 am, Angelo Dureghello wrote: > > Hi Geert and all, > > > > tested now building by kernel.org toolchain 10.1.0, > > i can boot with or without the above added volatile, jfyi. > > So is the net out that this was a compiler/toolchain problem? > yes seems so. This is no more an issue for me, so up to you how to proceed, anyway, keeping volatile seems to help in case someone uses older compilers (i was a 5.2.0 age btw, and toolchain was made by myself). > Regards > Greg > Regards, angelo -- Angelo Dureghello Timesys e. angelo.dureghello@timesys.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses @ 2020-11-08 16:34 Angelo Dureghello 0 siblings, 0 replies; 7+ messages in thread From: Angelo Dureghello @ 2020-11-08 16:34 UTC (permalink / raw) To: gerg; +Cc: linux-m68k, geert, Angelo Dureghello After rebasing today with mainline/master, i am hitting this issue again: https://www.spinics.net/lists/linux-m68k/msg15881.html Loading Ramdisk to 47ccc000, end 47d85544 ... OK ... initrd at 0x47ccc000:0x47d85544 overlap at 1073741889 for chunk 0 overlap at 1073746200 for chunk 0 overlap at 1073746760 for chunk 0 overlap at 1073746761 for chunk 0 overlap at 1073746762 for chunk 0 overlap at 1073746763 for chunk 0 overlap at 1073746764 for chunk 0 This time i am using gcc-10.1.0-nolibc, but the issue re-appeared. Add volatile again to solve the issue. Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com> --- arch/m68k/include/asm/page_mm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/m68k/include/asm/page_mm.h b/arch/m68k/include/asm/page_mm.h index e6b75992192b..1eff386cc64b 100644 --- a/arch/m68k/include/asm/page_mm.h +++ b/arch/m68k/include/asm/page_mm.h @@ -135,7 +135,7 @@ static inline __attribute_const__ int __virt_to_node_shift(void) { int shift; - asm ( + asm volatile ( "1: moveq #0,%0\n" m68k_fixup(%c1, 1b) : "=d" (shift) -- 2.28.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-08 16:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-24 23:34 [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses Angelo Dureghello 2020-06-25 8:13 ` Geert Uytterhoeven 2020-06-26 20:14 ` Angelo Dureghello 2020-07-13 19:41 ` Angelo Dureghello 2020-07-14 13:01 ` Greg Ungerer 2020-07-14 15:41 ` Angelo Dureghello 2020-11-08 16:34 Angelo Dureghello
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).