SeaBIOS

seabios@seabios.org

4 participants
3801 discussions

Re: [SeaBIOS] SeaBIOS crashes on CBFS without legacy header
by Kevin O'Connor 29 Dec '15

29 Dec '15

On Thu, Dec 24, 2015 at 03:45:03PM -0800, Alex Gagniuc wrote: > When SeaBIOS is run on an FMAP-based CBFS (AKA, without a CBFS master > header, it crashes). The problem is in coreboot_cbfs_init. A > disassembly with -Mintel shows the following: > > 00000000 <coreboot_cbfs_init>: > ... > 7: 8b 35 fc ff ff ff mov esi,DWORD PTR ds:0xfffffffc > d: 81 3e 4f 52 42 43 cmp DWORD PTR [esi],0x4342524f > > > 7: 8b 35 fc ff ff ff mov esi,DWORD PTR ds:0xfffffffc > > Move the contents of memory address 0xfffffffc to esi. Since there's > no legacy CBFS pointer, the region is all 1's. esi now contains > 0xffffffff > > > d: 81 3e 4f 52 42 43 cmp DWORD PTR [esi],0x4342524f > > 32-bit dereference esi (0xffffffff) -- causes general protection > fault. The dereference is not because of unaligned access, but because > the access goes over the 32-bit address space in flat mode. The way > around this iss to sanitize the value at [0xffffffff] before > dereferencing it. A sane check is: > > if(address_of_hdr > (0xfffffff0 - sizeof(*hdr))) > fail(); > This is sane check because because the x86 reset vector is resides at > 0xfffffff0 (there are cases where this isn't true, but that's besides > the point here). Thanks. Can you put together a patch, or do you want me to? > There's also an unrelated problem with the C code that generates this assembly: > > > struct cbfs_header *hdr = *(void **)(CONFIG_CBFS_LOCATION - 4); > > With CONFIG_CBFS_LOCATION = 0, this places a negative value (-4) in an > unsigned type (pointer). The behavior of this is undefined according > to the C standard. Are you suggesting that an (unsigned) cast should be added? I doubt it would matter. -Kevin

2 2

[PATCH] build: fix typo in buildversion.py
by Roger Pau Monne 28 Dec '15

28 Dec '15

Fixes the following build error: Building ld scripts Traceback (most recent call last): File "./scripts/buildversion.py", line 134, in <module> main() File "./scripts/buildversion.py", line 114, in main cleanbuild, toolstr = tool_versions(options.tools) File "./scripts/buildversion.py", line 90, in tool_versions vers[isbinutils] = "mixed" NameError: global name 'vers' is not defined Makefile:160: recipe for target 'out/romlayout16.lds' failed Signed-off-by: Roger Pau Monné <roger.pau(a)citrix.com> --- Cc: Kevin O'Connor <kevin(a)koconnor.net> --- scripts/buildversion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/buildversion.py b/scripts/buildversion.py index ad47b90..4692898 100755 --- a/scripts/buildversion.py +++ b/scripts/buildversion.py @@ -87,7 +87,7 @@ def tool_versions(tools): if versions[isbinutils] and versions[isbinutils] != ver: logging.debug("Mixed version %s vs %s" % ( repr(versions[isbinutils]), repr(ver))) - vers[isbinutils] = "mixed" + versions[isbinutils] = "mixed" continue versions[isbinutils] = ver success += 1 -- 1.9.5 (Apple Git-50.3)

3 2

[PATCH 1/2] scsi: Do not call printf() from scsi_is_ready()
by Kevin O'Connor 28 Dec '15

28 Dec '15

The scsi_is_ready() function may be called from a thread, and it is not valid to call printf() from a thread. Convert printf() to dprintf() to avoid this possibility. This does mean that cdrom detection (from cdrom_boot() ) may not give notification of slow cdrom drives to a user. However, the extra medium detection time is unlikely to be large anyway. Reported-by: Tobias Diedrich <tobiasdiedrich(a)gmail.com> Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> --- src/hw/blockcmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c index e20e3fc..0725b46 100644 --- a/src/hw/blockcmd.c +++ b/src/hw/blockcmd.c @@ -168,7 +168,7 @@ scsi_is_ready(struct disk_op_s *op) if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) { /* IN PROGRESS OF BECOMING READY */ - printf("Waiting for device to detect medium... "); + dprintf(1, "Waiting for device to detect medium... "); /* Allow 30 seconds more */ end = timer_calc(30000); in_progress = 1; -- 2.5.0

1 2

[PATCH] nmi: Don't try to switch onto extra stack in NMI handler
by Kevin O'Connor 28 Dec '15

28 Dec '15

The NMI could occur when already on the extra stack, which would corrupt it. Always use the current stack on an NMI to avoid this. Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> --- src/romlayout.S | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/romlayout.S b/src/romlayout.S index fedadfe..53cc0f5 100644 --- a/src/romlayout.S +++ b/src/romlayout.S @@ -548,7 +548,10 @@ entry_post: ENTRY_INTO32 _cfunc32flat_handle_post // Normal entry point ORG 0xe2c3 - IRQ_ENTRY 02 + .global entry_02 +entry_02: + ENTRY handle_02 // NMI handler does not switch onto extra stack + iretw ORG 0xe3fe .global entry_13_official -- 2.5.0

1 1

SeaBIOS crashes on CBFS without legacy header
by Alex Gagniuc 25 Dec '15

25 Dec '15

When SeaBIOS is run on an FMAP-based CBFS (AKA, without a CBFS master header, it crashes). The problem is in coreboot_cbfs_init. A disassembly with -Mintel shows the following: 00000000 <coreboot_cbfs_init>: ... 7: 8b 35 fc ff ff ff mov esi,DWORD PTR ds:0xfffffffc d: 81 3e 4f 52 42 43 cmp DWORD PTR [esi],0x4342524f > 7: 8b 35 fc ff ff ff mov esi,DWORD PTR ds:0xfffffffc Move the contents of memory address 0xfffffffc to esi. Since there's no legacy CBFS pointer, the region is all 1's. esi now contains 0xffffffff > d: 81 3e 4f 52 42 43 cmp DWORD PTR [esi],0x4342524f 32-bit dereference esi (0xffffffff) -- causes general protection fault. The dereference is not because of unaligned access, but because the access goes over the 32-bit address space in flat mode. The way around this iss to sanitize the value at [0xffffffff] before dereferencing it. A sane check is: if(address_of_hdr > (0xfffffff0 - sizeof(*hdr))) fail(); This is sane check because because the x86 reset vector is resides at 0xfffffff0 (there are cases where this isn't true, but that's besides the point here). There's also an unrelated problem with the C code that generates this assembly: > struct cbfs_header *hdr = *(void **)(CONFIG_CBFS_LOCATION - 4); With CONFIG_CBFS_LOCATION = 0, this places a negative value (-4) in an unsigned type (pointer). The behavior of this is undefined according to the C standard. Hope this helps, Alex

1 0

[PATCH] xhci: Wait for port enable even for USB3 devices
by Kevin O'Connor 23 Dec '15

23 Dec '15

Some USB3 controllers (and/or devices) need additional time after the device is detected to place the port in an enabled state. Wait for the controller to report enabled before proceeding. This fixes several reports of devices that showed a "stall" error (cc 4) during set address. Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> --- src/hw/usb-xhci.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index 945b462..654feba 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -350,42 +350,41 @@ xhci_hub_reset(struct usbhub_s *hub, u32 port) { struct usb_xhci_s *xhci = container_of(hub->cntl, struct usb_xhci_s, usb); u32 portsc = readl(&xhci->pr[port].portsc); - int rc; if (!(portsc & XHCI_PORTSC_CCS)) // Device no longer connected?! return -1; switch (xhci_get_field(portsc, XHCI_PORTSC_PLS)) { case PLS_U0: - // A USB3 port - no reset necessary. - rc = speed_from_xhci[xhci_get_field(portsc, XHCI_PORTSC_SPEED)]; + // A USB3 port - controller automatically performs reset break; case PLS_POLLING: - // A USB2 port - perform device reset and wait for completion + // A USB2 port - perform device reset xhci_print_port_state(3, __func__, port, portsc); writel(&xhci->pr[port].portsc, portsc | XHCI_PORTSC_PR); - u32 end = timer_calc(100); - for (;;) { - portsc = readl(&xhci->pr[port].portsc); - if (!(portsc & XHCI_PORTSC_CCS)) - // Device disconnected during reset - return -1; - if (portsc & XHCI_PORTSC_PED) - // Reset complete - break; - if (timer_check(end)) { - warn_timeout(); - return -1; - } - yield(); - } - rc = speed_from_xhci[xhci_get_field(portsc, XHCI_PORTSC_SPEED)]; break; default: - rc = -1; - break; + return -1; + } + + // Wait for device to complete reset and be enabled + u32 end = timer_calc(100); + for (;;) { + portsc = readl(&xhci->pr[port].portsc); + if (!(portsc & XHCI_PORTSC_CCS)) + // Device disconnected during reset + return -1; + if (portsc & XHCI_PORTSC_PED) + // Reset complete + break; + if (timer_check(end)) { + warn_timeout(); + return -1; + } + yield(); } + int rc = speed_from_xhci[xhci_get_field(portsc, XHCI_PORTSC_SPEED)]; xhci_print_port_state(1, "XHCI", port, portsc); return rc; } -- 2.5.0

1 1

Re: [SeaBIOS] [Qemu-devel] [PATCH] SeaBios: Fix reset procedure reentrancy problem on qemu-kvm platform
by Kevin O'Connor 23 Dec '15

23 Dec '15

On Wed, Dec 23, 2015 at 06:40:12AM +0000, Gonglei (Arei) wrote: > > From: Kevin O'Connor [mailto:kevin@koconnor.net] > > On Tue, Dec 22, 2015 at 02:14:12AM +0000, Gonglei (Arei) wrote: > > > Sorry, it doesn't work. What's worse is we cannot stop SeaBIOS stuck by > > > Setting "CONFIG_ENTRY_EXTRASTACK=n" after applying this patch. > > > > Oops, can you try with the patch below instead? > > > > It works now. Thanks! > > But do we need to check other possible situations > that maybe cause *extra stack* broken or overridden? I believe the issue is that an NMI could occur while SeaBIOS is already using its extra stack. The code is not prepared to switch into the extra stack while already on the extra stack. SeaBIOS is careful to always disable IRQs while running C code to prevent this issue, but disabling normal IRQs does not disable NMIs. So, I believe this issue is specific to the nature of NMIs. -Kevin

2 2

Re: [SeaBIOS] WD Elements hang SeaBIOS at boot with "call16 with invalid stack"
by Kevin O'Connor 23 Dec '15

23 Dec '15

On Wed, Dec 23, 2015 at 09:14:44PM +0100, Tobias Diedrich wrote: > Final diff attached (without the src/stacks.c change, with %d->%u change > for sector counts). > Bootlog with debug_level 1 attached. Great! Patches sent - if no further comments I'll commit in a few days. -Kevin

1 0

Re: [SeaBIOS] WD Elements hang SeaBIOS at boot with "call16 with invalid stack"
by Tobias Diedrich 23 Dec '15

23 Dec '15

Final diff attached (without the src/stacks.c change, with %d->%u change for sector counts). Bootlog with debug_level 1 attached. On Wed, Dec 23, 2015 at 8:56 PM, Tobias Diedrich <tobiasdiedrich(a)gmail.com> wrote: > I replaced the printf(...) with dprintf(1, ...) (since a dprintf earlier > in the function was working fine). > > This appears to have fixed the issue (though PCHS seems to be wrong, maybe > because sector count is negative when parsed as a signed int). Full log > attached. > > |dfc02000| USB MSC vendor='WD' product='Elements 10B8' rev='1012' type=0 > removable=0 > \dfc04000/ End thread > |dfbfd000| USB MSC vendor='Multiple' product='Card Reader' rev='1.00' > type=0 removable=1 > |dfbfd000| Device reports MEDIUM NOT PRESENT > |dfbfd000| scsi_is_ready returned -1 > |dfbfd000| Unable to configure USB MSC drive. > |dfbfd000| Unable to configure USB MSC device. > \dfbfd000/ End thread > |dfc02000| Waiting for device to detect medium... \dfc03000/ End thread > |dfc02000| USB MSC blksize=512 sectors=-388003840 > |dfc02000| Registering bootable: USB MSC Drive WD Elements 10B8 1012 > (type:2 prio:103 data:f6110) > \dfc02000/ End thread > \dfc08000/ End thread > All threads complete. > Scan for option roms > > Press ESC for boot menu. > > > On Wed, Dec 23, 2015 at 7:58 PM, Kevin O'Connor <kevin(a)koconnor.net> > wrote: > >> On Wed, Dec 23, 2015 at 07:39:56PM +0100, Tobias Diedrich wrote: >> > On Wed, Dec 23, 2015 at 7:10 PM, Kevin O'Connor <kevin(a)koconnor.net> >> wrote: >> > >> > > On Wed, Dec 23, 2015 at 07:02:37PM +0100, Tobias Diedrich wrote: >> > > > Wcall16 with invalid stack: eax=dfbf3e62 edx=0 func=0x000f9563 >> > > esp=dfbf3e50 >> > > > >> > > > .text._farcall16 0x9563 : { *(.text._farcall16) } >> > > > >> > > >> > > Can you include the full log? It looks like the code tried to call a >> > > 16bit function while in a "thread", and I need to figure out which >> > > thread did that. >> > >> > >> > From the esp address it looks like it'sin the USB probe for the MSC >> device >> > (full log attached): >> > /dfc02000\ Start thread func=dfc135f1 >> > |dfc02000| set_address 0xdfc0a200 >> > |dfc02000| config_usb: 0xdfc0a0d0 >> > |dfc02000| device rev=0210 cls=00 sub=00 proto=00 size=64 >> > |dfc02000| Searching bootorder for: /pci@i0cf8/usb@12,2/storage@5 >> /*@0/*@0,0 >> > |dfc02000| Searching bootorder for: /pci@i0cf8/usb@12,2/usb-*@5 >> > |dfc02000| USB MSC vendor='WD' product='Elements 10B8' rev='1012' type=0 >> > removable=0 >> > Wcall16 with invalid stack: eax=dfc02e66 edx=0 func=0x000f91f0 >> esp=dfc02e54 >> > ret=0x000f0604 >> >> What happens if you apply the patch below? (If it looks like it is >> hanging, give it at least a minute before stopping it.) >> >> -Kevin >> >> >> --- a/src/hw/blockcmd.c >> +++ b/src/hw/blockcmd.c >> @@ -168,7 +168,7 @@ scsi_is_ready(struct disk_op_s *op) >> >> if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) { >> /* IN PROGRESS OF BECOMING READY */ >> - printf("Waiting for device to detect medium... "); >> + //printf("Waiting for device to detect medium... "); >> /* Allow 30 seconds more */ >> end = timer_calc(30000); >> in_progress = 1; >> > >

1 0

Re: [SeaBIOS] WD Elements hang SeaBIOS at boot with "call16 with invalid stack"
by Kevin O'Connor 23 Dec '15

23 Dec '15

On Wed, Dec 23, 2015 at 07:39:56PM +0100, Tobias Diedrich wrote: > On Wed, Dec 23, 2015 at 7:10 PM, Kevin O'Connor <kevin(a)koconnor.net> wrote: > > > On Wed, Dec 23, 2015 at 07:02:37PM +0100, Tobias Diedrich wrote: > > > Wcall16 with invalid stack: eax=dfbf3e62 edx=0 func=0x000f9563 > > esp=dfbf3e50 > > > > > > .text._farcall16 0x9563 : { *(.text._farcall16) } > > > > > > > Can you include the full log? It looks like the code tried to call a > > 16bit function while in a "thread", and I need to figure out which > > thread did that. > > > From the esp address it looks like it'sin the USB probe for the MSC device > (full log attached): > /dfc02000\ Start thread func=dfc135f1 > |dfc02000| set_address 0xdfc0a200 > |dfc02000| config_usb: 0xdfc0a0d0 > |dfc02000| device rev=0210 cls=00 sub=00 proto=00 size=64 > |dfc02000| Searching bootorder for: /pci@i0cf8/usb@12,2/storage@5/*@0/*@0,0 > |dfc02000| Searching bootorder for: /pci@i0cf8/usb@12,2/usb-*@5 > |dfc02000| USB MSC vendor='WD' product='Elements 10B8' rev='1012' type=0 > removable=0 > Wcall16 with invalid stack: eax=dfc02e66 edx=0 func=0x000f91f0 esp=dfc02e54 > ret=0x000f0604 What happens if you apply the patch below? (If it looks like it is hanging, give it at least a minute before stopping it.) -Kevin --- a/src/hw/blockcmd.c +++ b/src/hw/blockcmd.c @@ -168,7 +168,7 @@ scsi_is_ready(struct disk_op_s *op) if (sense.asc == 0x04 && sense.ascq == 0x01 && !in_progress) { /* IN PROGRESS OF BECOMING READY */ - printf("Waiting for device to detect medium... "); + //printf("Waiting for device to detect medium... "); /* Allow 30 seconds more */ end = timer_calc(30000); in_progress = 1;

2 1

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS