Hi Geert,
On Sat, Sep 01, 2018 at 09:23:42AM +0200, Geert Uytterhoeven wrote:
> Hi Angelo,
>
> On Sat, Sep 1, 2018 at 3:17 AM Angelo Dureghello <[email protected]> wrote:
> > Without MMU, when CONFIG_UBOOT is set, and CONFIG_BOOTPARAM
> > is not set, a wrong command-line was produced (boot hangs,
> > no console), due to an initial erroneus space appended to the
> > command line in process_uboot_commandline().
> >
> > In MMU mode, the m68k_command_line array was not initially
> > terminated to zero, and process_uboot_commandline() was still
> > producing an invalid command-line (boot hangs, no console).
>
> It should be all zeroes?
>
> > --- a/arch/m68k/kernel/setup_mm.c
> > +++ b/arch/m68k/kernel/setup_mm.c
> > @@ -265,6 +265,7 @@ void __init setup_arch(char **cmdline_p)
> > init_mm.end_data = (unsigned long)_edata;
> > init_mm.brk = (unsigned long)_end;
> >
> > + m68k_command_line[0] = 0;
>
> This should not be needed:
>
> static char m68k_command_line[CL_SIZE] __initdata;
>
> I.e. m68k_command_line[] should be all zeroes.
>
> Is there a bug in the linker script?
>
Ops, right, but without the zero termination the final commandline
results invalid producing no console.
Investigating further so.
Just noticed some other initialization to 0 ...
void (*mach_sched_init) (irq_handler_t handler) __initdata = NULL;
/* machine dependent irq functions */
void (*mach_init_IRQ) (void) __initdata = NULL;
Noticed another strange thing, if i trace the commandline with
if (m68k_command_line[0] != 0)
pr_info("%02x %02x %02x %02x %02x %02x %02x %02x\n",
m68k_command_line[0],
m68k_command_line[1],
m68k_command_line[2],
m68k_command_line[3],
m68k_command_line[4],
m68k_command_line[5],
m68k_command_line[6],
m68k_command_line[7]);
#if defined(CONFIG_BOOTPARAM)
strncpy(m68k_command_line, CONFIG_BOOTPARAM_STRING, CL_SIZE);
...
Then i see all 00 and command line is set correctly and the
system boots. So it seems the fact i just refer to the array fixes
the things ...
> > #if defined(CONFIG_BOOTPARAM)
> > strncpy(m68k_command_line, CONFIG_BOOTPARAM_STRING, CL_SIZE);
> > m68k_command_line[CL_SIZE - 1] = 0;
> > diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c
> > index cfd5475bfc31..d65bb433583c 100644
> > --- a/arch/m68k/kernel/setup_no.c
> > +++ b/arch/m68k/kernel/setup_no.c
> > @@ -94,6 +94,8 @@ void __init setup_arch(char **cmdline_p)
> > init_mm.end_data = (unsigned long) &_edata;
> > init_mm.brk = (unsigned long) 0;
> >
> > + command_line[0] = 0;
>
> Likewise:
>
> char __initdata command_line[COMMAND_LINE_SIZE];
>
I just added this for conformity with the above MMU case. But this issue
is only visible in the CONFIG_MMU.
> > +
> > config_BSP(&command_line[0], sizeof(command_line));
> >
> > #if defined(CONFIG_BOOTPARAM)
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> [email protected]
>
> 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
