[PATCH] Die properly if CBFS can't find a stage

[Carl-Daniel Hailfinger]

cbfs_and_run_core() did not check the return code of cbfs_load_stage()
and jumped to (void*)-1 on error.
Die properly instead.
I didn't use die() because that caused a linker error.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c
===================================================================
--- LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Revision 4589)
+++ LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Arbeitskopie)
@@ -11,6 +11,11 @@
    u8 *dst;
    print_debug("Jumping to image.\r\n");
    dst = cbfs_load_stage(filename);
+	if (dst == (void *) -1) {
+		/* We should use die() here. */
+		print_emerg("Loading stage failed!\n");
+		for (;;);
+	}
    print_debug("Jumping to image.\r\n");

__asm__ volatile (

[Marc Jones]

Hi Carl-Daniel,

On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel
Hailfinger wrote:
> cbfs_and_run_core() did not check the return code of cbfs_load_stage()
> and jumped to (void*)-1 on error.
> Die properly instead.
> I didn't use die() because that caused a linker error.

How about some inline asm to do a hlt instruction in the for loop?
That would be better than spinning full speed.

Marc

[ron minnich]

we should fix die() instead. Inline asm is not what we want, since the
PPC port may someday come back.

ron

[Carl-Daniel Hailfinger]

On 27.08.2009 05:46, ron minnich wrote:
> we should fix die() instead. Inline asm is not what we want, since the
> PPC port may someday come back.

There are two ways to fix die(). Either we compile it in both into the
bootblock and into coreboot_ram or we reuse the function like in v3.

How do we proceed? I'd like to fix this.

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> There are two ways to fix die(). Either we compile it in both into
> the bootblock and into coreboot_ram or we reuse the function like
> in v3.
>
> How do we proceed?

When die() only spins on hlt I think it's fine to duplicate the
function. Would be nice to only have one source however.

Once we have a capable panic room maybe it would be nicer to only
have that code in one place, however. It depends on the hassle and
code size duplicated.

//Peter

[Stefan Reinauer]

There are many other places where this is hit.. I am currently preparing
a more extensive patch, stay tuned

On 8/27/09 5:30 AM, Carl-Daniel Hailfinger wrote:
> cbfs_and_run_core() did not check the return code of cbfs_load_stage()
> and jumped to (void*)-1 on error.
> Die properly instead.
> I didn't use die() because that caused a linker error.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c
>
> --- LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Revision 4589)
> +++ LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Arbeitskopie)
> @@ -11,6 +11,11 @@
>    u8 *dst;
>    print_debug("Jumping to image.\r\n");
>    dst = cbfs_load_stage(filename);
>
> if (dst == (void *) -1) {
> /* We should use die() here. */
> print_emerg("Loading stage failed!\n");
> for (;;);
>
> }
> print_debug("Jumping to image.\r\n");
>
> __asm__ volatile (

[Carl-Daniel Hailfinger]

Hi Stefan,

any updates on that patch?

Regards,
Carl-Daniel

On 27.08.2009 13:15, Stefan Reinauer wrote:
> There are many other places where this is hit.. I am currently preparing
> a more extensive patch, stay tuned
>
> On 8/27/09 5:30 AM, Carl-Daniel Hailfinger wrote:
> > cbfs_and_run_core() did not check the return code of cbfs_load_stage()
> > and jumped to (void*)-1 on error.
> > Die properly instead.
> > I didn't use die() because that caused a linker error.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
> > Index: LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c
> >
> > --- LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Revision 4589)
> > +++ LinuxBIOSv2-asus_m2a-vm/src/arch/i386/lib/cbfs_and_run.c	(Arbeitskopie)
> > @@ -11,6 +11,11 @@
> >    u8 *dst;
> >    print_debug("Jumping to image.\r\n");
> >    dst = cbfs_load_stage(filename);
> >
> > if (dst == (void *) -1) {
> > /* We should use die() here. */
> > print_emerg("Loading stage failed!\n");
> > for (;;);
> >
> > }
> > print_debug("Jumping to image.\r\n");
> >
> > __asm__ volatile (

[Myles Watson]

On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel Hailfinger
 wrote:
> cbfs_and_run_core() did not check the return code of cbfs_load_stage()
> and jumped to (void*)-1 on error.
> Die properly instead.
> I didn't use die() because that caused a linker error.
>
> Signed-off-by: Carl-Daniel Hailfinger

Acked-by: Myles Watson

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 09.10.2009 02:50, Myles Watson wrote:
> On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel Hailfinger
>  wrote:
> > cbfs_and_run_core() did not check the return code of cbfs_load_stage()
> > and jumped to (void*)-1 on error.
> > Die properly instead.
> > I didn't use die() because that caused a linker error.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Acked-by: Myles Watson

Thanks, r4776.

Regards,
Carl-Daniel