Re: Kernel versions 6.x don't boot on Amiga 4000

[Mike Rapoport <rppt@kernel.org>]

Hi Geert,

On Mon, Feb 27, 2023 at 01:31:23PM +0100, Geert Uytterhoeven wrote:
> Hi Mike,
> 
> On Mon, Feb 27, 2023 at 12:34 PM Mike Rapoport <rppt@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:
> > > > > 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...

Happens to the best of us :)
 
> 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

-- 
Sincerely yours,
Mike.