| 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
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
we should fix die() instead. Inline asm is not what we want, since the PPC port may someday come back. ron
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 ( > > >
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 ( >> >> >> >> > > >
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
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
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
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 (
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>