Re: [PATCH] m68k: fix command-line parsing when passed from u-boot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <angelo@xxxxxxxx> 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 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux