RBTX4927 with VxWorks boot loader crashes in prom_getenv()

RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5334 bytes --]

	Hi Nemoto-san,

My RBTX4927 with VxWorks boot loader crashes in prom_getenv() since commit
860e546c19d88c21819c7f0861c505debd2d6eed ("MIPS: TXx9: Early command-line
preprocessing"):

| Tlb Load Exception
| Exception Program Counter: 0x80324e60
| Status Register: 0x36008700
| Cause Register: 0x00000008
| Access Address : 0x00002000
| Task: 0x87fe7fc0 ""
| 8031e714: 80328cc0 (0, 1, 0, 0)
| 80328ce4: 80325238 (80126350, 0, 80340000, 0)
| 8032550c: 80324e0c (0, 0, 0, 736e6f63)
| 
| $0    =                0   t0    = ffffffff8030fc40   s0    = ffffffff80340000
| at    =               6d   t1    = ffffffff80310000   s1    = ffffffff80310000
| v0    =             2000   t2    = ffffffff80350000   s2    = ffffffff802e0000
| v1    = ffffffff8030f7c7   t3    = ffffffff80350000   s3    = ffffffff80002000
| a0    = ffffffff802dfe1c   t4    = ffffffff80350000   s4    =                1
| a1    =             2000   t5    =                0   s5    = ffffffff802e0000
| a2    = ffffffff802dfe1c   t6    =                0   s6    = ffffffff80789384
| a3    =                0   t7    = ffffffffbc020280   s7    =              1e0
| s8    = ffffffff80104430   k0    =                0                           
| gp    = ffffffff8030c000   k1    =                0   t8    =                0
| ra    = ffffffff80325514   sp    = ffffffff8030de38   t9    = ffffffffffffffff
| divlo =                2   divhi =                4   sr    = 36008700        
| pc    = 80324e60        

After making the following change:

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index 5526375..96692b6 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -416,6 +416,11 @@ const char *__init prom_getenv(const char *name)
 {
 	const s32 *str = (const s32 *)fw_arg2;
 
+printk("fw_arg0 = 0x%lx\n", fw_arg0);
+printk("fw_arg1 = 0x%lx\n", fw_arg1);
+printk("fw_arg2 = 0x%lx\n", fw_arg2);
+printk("fw_arg3 = 0x%lx\n", fw_arg3);
+return NULL;
 	if (!str)
 		return NULL;
 	/* YAMON style ("name", "value" pairs) */

it boots again and prints:

| fw_arg0 = 0x80002000
| fw_arg1 = 0x80001fe0
| fw_arg2 = 0x2000
| fw_arg3 = 0x20

So your assumption that bootloaders other than YAMON pass NULL for fw_arg2 is
apparently wrong.

BTW, after making this change, I still have another issue with the network.
I get

| ne.c:v1.10 9/23/94 Donald Becker (becker@scyld.com)
| Last modified Nov 1, 2000 by Paul Gortmaker
| NE*000 ethercard probe at 0x6020280:<4>eth%d: interrupt from stopped card
| irq 13: nobody cared (try booting with the "irqpoll" option)
| Call Trace:
| [<8010cd90>] dump_stack+0x8/0x34
| [<8014d944>] __report_bad_irq+0x4c/0xb0
| [<8014db28>] note_interrupt+0x180/0x200
| [<8014ebbc>] handle_level_irq+0xc4/0x114
| [<80107cb4>] plat_irq_dispatch+0x74/0x94
| [<80100424>] ret_from_irq+0x0/0x4
| [<8012bab0>] __do_softirq+0x54/0x140
| [<8012bbf8>] do_softirq+0x5c/0x90
| [<8012c094>] irq_exit+0x40/0x90
| [<80100424>] ret_from_irq+0x0/0x4
| [<8014d0a0>] __setup_irq+0x214/0x2a4
| [<8014d210>] request_irq+0xe0/0x128
| [<80340e50>] ne_probe1+0x4c8/0x660
| [<80341234>] ne_drv_probe+0xd4/0x170
| [<80229214>] driver_probe_device+0xf0/0x1a8
| [<8022933c>] __driver_attach+0x70/0xa8
| [<80228700>] bus_for_each_dev+0x54/0xa0
| [<80228bb4>] bus_add_driver+0xc4/0x1e8
| [<80229594>] driver_register+0xb4/0x15c
| [<8022a330>] platform_driver_probe+0x18/0x74
| [<80340968>] ne_init+0x20/0x40
| [<8010759c>] __kprobes_text_end+0x64/0x1c4
| [<8032a5fc>] kernel_init+0x8c/0xfc
| [<80109124>] kernel_thread_helper+0x10/0x18
| 
| handlers:
| [<802348b8>] (eip_interrupt+0x0/0x360)
| Disabling IRQ #13
| 00:60:0a:00:45:33
| eth0: RBHMA4X00/RTL8019 found at 0x6020280, using IRQ 13.

and

| WARNING: at net/sched/sch_generic.c:226 dev_watchdog+0x168/0x25c()
| NETDEV WATCHDOG: eth0 (): transmit timed out
| Modules linked in:
| Call Trace:
| [<8010cd90>] dump_stack+0x8/0x34
| [<801262f8>] warn_slowpath+0x80/0xac
| [<80259ab4>] dev_watchdog+0x168/0x25c
| [<80130ab0>] run_timer_softirq+0x168/0x20c
| [<8012bae0>] __do_softirq+0x84/0x140
| [<8012bbf8>] do_softirq+0x5c/0x90
| [<8012c094>] irq_exit+0x40/0x90
| [<80100424>] ret_from_irq+0x0/0x4
| [<80108620>] r4k_wait_irqoff+0x4c/0x54
| [<80109300>] cpu_idle+0x2c/0x54
| [<8032aa1c>] start_kernel+0x3b0/0x3e4
| 
| ---[ end trace ecf0edc63f731c6d ]---
| ..<7>eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=1873.
| ..<7>eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=5533.
|  timed out!
| IP-Config: Reopening network devices...
| Sending DHCP requests ..<7>eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=733.
| ..<7>eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=2319.
| ..<7>eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=6709.
|  timed out!
| IP-Config: Reopening network devices...

This is plain 2.6.27+ as of yesterday. I'll see if it goes away with today's
linux-mips git tree.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 727 bytes --]

On Fri, 24 Oct 2008, Geert Uytterhoeven wrote:
> BTW, after making this change, I still have another issue with the network.
> I get

> This is plain 2.6.27+ as of yesterday. I'll see if it goes away with today's
> linux-mips git tree.

Seems to be fixed in 2.6.28-rc1 (both mainline and linux-mips git).

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Atsushi Nemoto]

On Fri, 24 Oct 2008 11:39:07 +0200 (CEST), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> My RBTX4927 with VxWorks boot loader crashes in prom_getenv() since commit
> 860e546c19d88c21819c7f0861c505debd2d6eed ("MIPS: TXx9: Early command-line
> preprocessing"):
...
> | fw_arg0 = 0x80002000
> | fw_arg1 = 0x80001fe0
> | fw_arg2 = 0x2000
> | fw_arg3 = 0x20
> 
> So your assumption that bootloaders other than YAMON pass NULL for fw_arg2 is
> apparently wrong.

Indeed.  We should know what sort of value was passed by fw_arg2, and
I hope auto-detection.

Do you know what value the boot loader passes via fw_arg2?  If fw_arg2
is always small integer (i.e. a not KSEG0/KSEG1 address), we can
autodetect fw_arg2 was a pointer or not.

---
Atsushi Nemoto

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1341 bytes --]

	Hi Nemoto-san,

On Fri, 24 Oct 2008, Atsushi Nemoto wrote:
> On Fri, 24 Oct 2008 11:39:07 +0200 (CEST), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > My RBTX4927 with VxWorks boot loader crashes in prom_getenv() since commit
> > 860e546c19d88c21819c7f0861c505debd2d6eed ("MIPS: TXx9: Early command-line
> > preprocessing"):
> ...
> > | fw_arg0 = 0x80002000
> > | fw_arg1 = 0x80001fe0
> > | fw_arg2 = 0x2000
> > | fw_arg3 = 0x20
> > 
> > So your assumption that bootloaders other than YAMON pass NULL for fw_arg2 is
> > apparently wrong.
> 
> Indeed.  We should know what sort of value was passed by fw_arg2, and
> I hope auto-detection.
> 
> Do you know what value the boot loader passes via fw_arg2?  If fw_arg2

Unfortunately not. I'll try to Google a bit for it...

> is always small integer (i.e. a not KSEG0/KSEG1 address), we can
> autodetect fw_arg2 was a pointer or not.

Exactly my thoughts.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Atsushi Nemoto]

On Fri, 24 Oct 2008 16:06:28 +0200 (CEST), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > > | fw_arg0 = 0x80002000
> > > | fw_arg1 = 0x80001fe0
> > > | fw_arg2 = 0x2000
> > > | fw_arg3 = 0x20
> > > 
> > > So your assumption that bootloaders other than YAMON pass NULL for fw_arg2 is
> > > apparently wrong.
> > 
> > Indeed.  We should know what sort of value was passed by fw_arg2, and
> > I hope auto-detection.
> > 
> > Do you know what value the boot loader passes via fw_arg2?  If fw_arg2
> 
> Unfortunately not. I'll try to Google a bit for it...

BTW, it looks fw_arg0 is not 'argc'.  Fortunately current code just
ignores argv if argc was negative, but it is not intentional
behaviour, just a luck ;)

You can put a string starting with "-" in CONFIG_CMDLINE, so that
fw_arg0 is ignored regardless of its value.  Hmm... putting "noenv" or
something in CONFIG_CMDLINE (and check it in preprocess_cmdline()) can
be an another workaround for getenv problem...

---
Atsushi Nemoto

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3646 bytes --]

	Hi Nemoto-san,

On Fri, 24 Oct 2008, Atsushi Nemoto wrote:
> On Fri, 24 Oct 2008 16:06:28 +0200 (CEST), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > > > | fw_arg0 = 0x80002000
> > > > | fw_arg1 = 0x80001fe0
> > > > | fw_arg2 = 0x2000
> > > > | fw_arg3 = 0x20
> > > > 
> > > > So your assumption that bootloaders other than YAMON pass NULL for fw_arg2 is
> > > > apparently wrong.
> > > 
> > > Indeed.  We should know what sort of value was passed by fw_arg2, and
> > > I hope auto-detection.
> > > 
> > > Do you know what value the boot loader passes via fw_arg2?  If fw_arg2
> > 
> > Unfortunately not. I'll try to Google a bit for it...
> 
> BTW, it looks fw_arg0 is not 'argc'.  Fortunately current code just
> ignores argv if argc was negative, but it is not intentional
> behaviour, just a luck ;)

Woops...

The memory pointed to by fw_arg0 and fw_arg1 contains just zeroes.

According to the sources, the VxWorks bootloader just jumps to the kernel's
entry point, without passing any parameters in a0-a3 at all. So they're just
leftovers.

> You can put a string starting with "-" in CONFIG_CMDLINE, so that
> fw_arg0 is ignored regardless of its value.  Hmm... putting "noenv" or
> something in CONFIG_CMDLINE (and check it in preprocess_cmdline()) can
> be an another workaround for getenv problem...

Prepending my command line in CONFIG_CMDLINE with `-' doesn't help.
prom_getenv() is still called.

So I came up with the patch below.

Subject: [PATCH] txx9: Make firmware parameter passing more robust

When booting Linux on a txx9 board with VxWorks boot loader, it crashes in
prom_getenv(), as VxWorks doesn't pass firmware parameters in a0-a3 (in my
case, the actual leftover values in these registers were 0x80002000,
0x80001fe0, 0x2000, and 0x20).

Make the parsing of argc, argv, and envp a bit more robust by checking if argc
is a number below CKSEG0, and argv/envp point to CKSEG0.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index 5526375..f4486a5 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -156,11 +156,20 @@ static struct txx9_board_vec *__init find_board_byname(const char *name)
 
 static void __init prom_init_cmdline(void)
 {
-	int argc = (int)fw_arg0;
-	int *argv32 = (int *)fw_arg1;
+	int argc;
+	int *argv32;
 	int i;			/* Always ignore the "-c" at argv[0] */
 	char builtin[CL_SIZE];
 
+	if (fw_arg0 >= CKSEG0 || fw_arg1 < CKSEG0) {
+	    /* argc is not a valid number, or argv32 is not a valid pointer */
+	    argc = 0;
+	    argv32 = NULL;
+	} else {
+	    argc = (int)fw_arg0;
+	    argv32 = (int *)fw_arg1;
+	}
+
 	/* ignore all built-in args if any f/w args given */
 	/*
 	 * But if built-in strings was started with '+', append them
@@ -414,10 +423,12 @@ char * __init prom_getcmdline(void)
 
 const char *__init prom_getenv(const char *name)
 {
-	const s32 *str = (const s32 *)fw_arg2;
+	const s32 *str;
 
-	if (!str)
+	if (fw_arg2 < CKSEG0)
 		return NULL;
+
+	str = (const s32 *)fw_arg2;
 	/* YAMON style ("name", "value" pairs) */
 	while (str[0] && str[1]) {
 		if (!strcmp((const char *)(unsigned long)str[0], name))

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Atsushi Nemoto]

On Mon, 27 Oct 2008 14:39:32 +0100 (CET), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > BTW, it looks fw_arg0 is not 'argc'.  Fortunately current code just
> > ignores argv if argc was negative, but it is not intentional
> > behaviour, just a luck ;)
> 
> Woops...
> 
> The memory pointed to by fw_arg0 and fw_arg1 contains just zeroes.
> 
> According to the sources, the VxWorks bootloader just jumps to the kernel's
> entry point, without passing any parameters in a0-a3 at all. So they're just
> leftovers.

Wow, what a simple loader ...

> > You can put a string starting with "-" in CONFIG_CMDLINE, so that
> > fw_arg0 is ignored regardless of its value.  Hmm... putting "noenv" or
> > something in CONFIG_CMDLINE (and check it in preprocess_cmdline()) can
> > be an another workaround for getenv problem...
> 
> Prepending my command line in CONFIG_CMDLINE with `-' doesn't help.
> prom_getenv() is still called.
>
> So I came up with the patch below.

Oh, yes, the '-' does not prevent prom_getenv().  It just prevents
argc/argv parsing.  Anyway, checking invalid fw_argN should be good.

> Subject: [PATCH] txx9: Make firmware parameter passing more robust
> 
> When booting Linux on a txx9 board with VxWorks boot loader, it crashes in
> prom_getenv(), as VxWorks doesn't pass firmware parameters in a0-a3 (in my
> case, the actual leftover values in these registers were 0x80002000,
> 0x80001fe0, 0x2000, and 0x20).
> 
> Make the parsing of argc, argv, and envp a bit more robust by checking if argc
> is a number below CKSEG0, and argv/envp point to CKSEG0.
> 
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Look OK for me, except for coding style :)

If TAB was used for indent,
Acked-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3094 bytes --]

On Mon, 27 Oct 2008, Atsushi Nemoto wrote:
> On Mon, 27 Oct 2008 14:39:32 +0100 (CET), Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > Subject: [PATCH] txx9: Make firmware parameter passing more robust
> > 
> > When booting Linux on a txx9 board with VxWorks boot loader, it crashes in
> > prom_getenv(), as VxWorks doesn't pass firmware parameters in a0-a3 (in my
> > case, the actual leftover values in these registers were 0x80002000,
> > 0x80001fe0, 0x2000, and 0x20).
> > 
> > Make the parsing of argc, argv, and envp a bit more robust by checking if argc
> > is a number below CKSEG0, and argv/envp point to CKSEG0.
> > 
> > Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> 
> Look OK for me, except for coding style :)

Sorry, how could I have missed that...

> If TAB was used for indent,
> Acked-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

Thx!

Subject: [PATCH] txx9: Make firmware parameter passing more robust

When booting Linux on a txx9 board with VxWorks boot loader, it crashes in
prom_getenv(), as VxWorks doesn't pass firmware parameters in a0-a3 (in my
case, the actual leftover values in these registers were 0x80002000,
0x80001fe0, 0x2000, and 0x20).

Make the parsing of argc, argv, and envp a bit more robust by checking if argc
is a number below CKSEG0, and argv/envp point to CKSEG0.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Acked-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

---
 arch/mips/txx9/generic/setup.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -156,11 +156,23 @@ static struct txx9_board_vec *__init fin
 
 static void __init prom_init_cmdline(void)
 {
-	int argc = (int)fw_arg0;
-	int *argv32 = (int *)fw_arg1;
+	int argc;
+	int *argv32;
 	int i;			/* Always ignore the "-c" at argv[0] */
 	char builtin[CL_SIZE];
 
+	if (fw_arg0 >= CKSEG0 || fw_arg1 < CKSEG0) {
+		/*
+		 * argc is not a valid number, or argv32 is not a valid
+		 * pointer
+		 */
+		argc = 0;
+		argv32 = NULL;
+	} else {
+		argc = (int)fw_arg0;
+		argv32 = (int *)fw_arg1;
+	}
+
 	/* ignore all built-in args if any f/w args given */
 	/*
 	 * But if built-in strings was started with '+', append them
@@ -414,10 +426,12 @@ char * __init prom_getcmdline(void)
 
 const char *__init prom_getenv(const char *name)
 {
-	const s32 *str = (const s32 *)fw_arg2;
+	const s32 *str;
 
-	if (!str)
+	if (fw_arg2 < CKSEG0)
 		return NULL;
+
+	str = (const s32 *)fw_arg2;
 	/* YAMON style ("name", "value" pairs) */
 	while (str[0] && str[1]) {
 		if (!strcmp((const char *)(unsigned long)str[0], name))

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: RBTX4927 with VxWorks boot loader crashes in prom_getenv()

[Ralf Baechle]

On Mon, Oct 27, 2008 at 03:25:49PM +0100, Geert Uytterhoeven wrote:

Thanks, applied!

  Ralf