* [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
* 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
* 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-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-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-24 23:34 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
* [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
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-11-08 16:34 [PATCH] m68k: mm: fix wrong m68k_fixup_info addresses Angelo Dureghello
-- strict thread matches above, loose matches on Subject: below --
2020-06-24 23:34 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
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).