Hi Mike,
On Mon, Feb 27, 2023 at 12:34 PM Mike Rapoport <r...@kernel.org> wrote:
> On Mon, Feb 27, 2023 at 10:42:34PM +1300, Michael Schmitz wrote:
> > Am 27.02.2023 um 21:26 schrieb Geert Uytterhoeven:
> > > FTR, here is the diff of the dmesg between good and bad:
> > >
> > > +initrd: 07f61166 - 08000000
> > >
> > > This is wrong (note the 6 trailing zeros), as phys_to_virt() is not
> > > working correctly yet (module_fixup() is called from paging_init()).
> > >
> > > Zone ranges:
> > > DMA [mem 0x0000000007400000-0x0000007fffffffff]
> > > Normal empty
> > > Movable zone start for each node
> > > Early memory node ranges
> > > node 0: [mem 0x0000000007400000-0x0000000007ffffff]
> > > Initmem setup node 0 [mem 0x0000000007400000-0x0000000007ffffff]
> > > -initrd: 00b61166 - 00c00000
> > >
> > > This is correct (note the 5 trailing zeros).
> > >
> > > -pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
> > > -pcpu-alloc: [0] 0
> > > [...]
> > > +Unable to handle kernel access at virtual address (ptrval)
> > > +Oops: 00000000
> > > +Modules linked in:
> > > +PC: [<002c11be>] memcmp+0x2c/0x5c
> > > +SR: 2700 SP: (ptrval) a2: 003bd560
> > > +d0: 0035eb83 d1: 07fffff8 d2: 002c1192 d3: 000000e6
> > > +d4: 000684e8 d5: 00447000 a0: 0000000c a1: 07fffff4
> > > +Process swapper (pid: 0, task=(ptrval))
> > > +Frame format=7 eff addr=003bbfbc ssw=0505 faddr=07fffff4
> > > +wb 1 stat/addr/data: 0005 00447000 07401000
> > > +wb 2 stat/addr/data: 0005 000000e6 000684e8
> > > +wb 3 stat/addr/data: 0005 003bbfb4 002c1192
> > > +push data: 07401000 002c7d82 07401000 074a2cf4
> > > +Stack from 003bbfb4:
> > > +002c1192 000000e6 002c7d82 00428eda 07fffff4 0035eb7f 0000000c
> > > 00447000
> > > +000000e6 000684e8 00447000 07401000 074bec08 07401000 074a2cf4
> > > 07fffff0
> > > +00440406 00000000 00428322
> > > +Call Trace: [<002c1192>] memcmp+0x0/0x5c
> > > +[<002c7d82>] _printk+0x0/0x18
> > > +[<00428eda>] start_kernel+0x80/0x5b0
> > > +[<000684e8>] pcpu_alloc+0x88/0x3b4
> > > +[<00428322>] _sinittext+0x322/0x9b0
> > >
> > > On Mon, Feb 27, 2023 at 7:30 AM Finn Thain <fth...@linux-m68k.org> wrote:
> > > > On Mon, 27 Feb 2023, Michael Schmitz wrote:
> > > > > I wonder whether Finn's memtest patch merely exposed another MM bug
> > > >
> > > > A kernel patch may be easier than a bootloader patch (even if this is a
> > > > bootloader bug) particularly if it affects multiple platforms.
> > > >
> > > > A partial revert of my patch (below) will probably avoid the issue, but
> > > > with the side effect that use of memtest will clobber the initrd.
> > >
> > > Which we can avoid, by moving the ramdisk handling inside paging_init().
> > >
> > > > The initrd and memtest features aren't usually needed together. At the
> > > > time when I needed the memtest feature I did not have confidence in the
> > > > hardeare. An initrd wasn't very useful at that point.
> > > >
> > > > diff --git a/arch/m68k/kernel/setup_mm.c b/arch/m68k/kernel/setup_mm.c
> > > > index 3a2bb2e8fdad..92f1b9268dff 100644
> > > > --- a/arch/m68k/kernel/setup_mm.c
> > > > +++ b/arch/m68k/kernel/setup_mm.c
> > > > @@ -326,6 +326,8 @@ void __init setup_arch(char **cmdline_p)
> > > > panic("No configuration setup");
> > > > }
> > > >
> > > > + paging_init();
> > > > +
> > > > #ifdef CONFIG_BLK_DEV_INITRD
> > > > if (m68k_ramdisk.size) {
> > > > memblock_reserve(m68k_ramdisk.addr, m68k_ramdisk.size);
> > >
> > > Presumably something in memblock_reserve() relies on having
> > > called paging_init() before?
>
> memblock_reserve() does not rely on paging_init() as it operates on
> physical addresses and it does not care if memory was already registered.
>
> What does rely on paging_init() it's phys_to_virt() in the line after
> memblock_reserve():
>
> initrd_start = (unsigned long)phys_to_virt(m68k_ramdisk.addr);
> initrd_end = initrd_start + m68k_ramdisk.size;
>
> So to have both memtest and initrd we'd need something like
>
> memblock_reserve(m68k_ramdisk.addr, m68k_ramdisk.size);
>
> paging_init() {
> /* setup page tables and memblock */
> early_memtest();
> }
>
> initrd_start = (unsigned long)phys_to_virt(m68k_ramdisk.addr);
>
> or
>
> paging_init(); /* without early_memtest() */
>
> memblock_reserve(m68k_ramdisk.addr, m68k_ramdisk.size);
> initrd_start = (unsigned long)phys_to_virt(m68k_ramdisk.addr);
>
> early_memtest();
Of course... /me bangs his head against the TFT for not having
realized before the values saved into initrd_{start,end} are not just
for printing in the pr_info() line...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
