Patchworkβ Die properly if CBFS can't find a stage

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2009-08-27 03:30:04
Message ID <4A95FDBC.2050205@gmx.net>
Download mbox | patch
Permalink /patch/203/
State Accepted
Commit r4776
Headers show

Comments

Carl-Daniel Hailfinger - 2009-08-27 03:30:04
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 <c-d.hailfinger.devel.2006@gmx.net>
Marc Jones - 2009-08-27 03:39:11
Hi Carl-Daniel,

On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel
Hailfinger<c-d.hailfinger.devel.2006@gmx.net> 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 - 2009-08-27 03:46:38
we should fix die() instead. Inline asm is not what we want, since the
PPC port may someday come back.

ron
Stefan Reinauer - 2009-08-27 11:15:38
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 <c-d.hailfinger.devel.2006@gmx.net>
>
> 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 - 2009-09-26 19:49:37
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 <c-d.hailfinger.devel.2006@gmx.net>
>>
>> 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 - 2009-09-26 21:24:05
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 - 2009-09-27 01:13:31
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
Myles Watson - 2009-10-09 00:50:19
On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006@gmx.net> 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 <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Myles Watson <mylesgw@gmail.com>

Thanks,
Myles
Carl-Daniel Hailfinger - 2009-10-14 23:51:30
On 09.10.2009 02:50, Myles Watson wrote:
> On Wed, Aug 26, 2009 at 9:30 PM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006@gmx.net> 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 <c-d.hailfinger.devel.2006@gmx.net>
>>     
> Acked-by: Myles Watson <mylesgw@gmail.com>
>   

Thanks, r4776.

Regards,
Carl-Daniel

Patch

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 (