SeaBIOS December 2018

seabios@seabios.org

9 participants
15 discussions

[PATCH v2] optionrom: disallow int19 redirect for pnp roms.

by Gerd Hoffmann

Check whenever pnp roms attempt to redirect int19, and in case it does log a message and undo the redirect. A pnp rom should not need this, we have BEVs and BCVs for that. Nevertheless there are roms in the wild which are redirecting int19. At least some BIOS implementations for physical hardware have a config option in the setup to allow/disallow int19 redirections, so just not allowing this seems to be the way to deal with this situation. Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1642135 Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- src/optionroms.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/optionroms.c b/src/optionroms.c index fc992f649f..e90900b790 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -8,6 +8,7 @@ #include "bregs.h" // struct bregs #include "config.h" // CONFIG_* #include "farptr.h" // FLATPTR_TO_SEG +#include "biosvar.h" // GET_IVT #include "hw/pci.h" // pci_config_readl #include "hw/pcidevice.h" // foreachpci #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA @@ -308,6 +309,24 @@ fail: return NULL; } +static int boot_irq_captured(void) +{ + struct segoff_s current, seabios; + + current = GET_IVT(0x19); + seabios = FUNC16(entry_19_official); + return ((current.seg != seabios.seg) || + (current.offset != current.offset)); +} + +static void boot_irq_restore(void) +{ + struct segoff_s seabios; + + seabios = FUNC16(entry_19_official); + SET_IVT(0x19, seabios); +} + // Attempt to map and initialize the option rom on a given PCI device. static void init_pcirom(struct pci_device *pci, int isvga, u64 *sources) @@ -327,8 +346,18 @@ init_pcirom(struct pci_device *pci, int isvga, u64 *sources) if (! rom) // No ROM present. return; + int irq_was_captured = boot_irq_captured(); + struct pnp_data *pnp = get_pnp_rom(rom); setRomSource(sources, rom, RS_PCIROM | (u32)pci); init_optionrom(rom, pci->bdf, isvga); + if (boot_irq_captured() && !irq_was_captured && + !file && !isvga && pnp) { + // This PCI rom is misbehaving - recapture the boot irqs + char *desc = MAKE_FLATPTR(FLATPTR_TO_SEG(rom), pnp->productname); + dprintf(1, "PnP optionrom \"%s\" (bdf %pP) captured int19, restoring\n", + desc, pci); + boot_irq_restore(); + } } -- 2.9.3

2 years, 5 months

[PATCH] optionrom: disallow int19 redirect for pnp roms.

by Gerd Hoffmann

Check whenever pnp roms attempt to redirect int19, and in case it does log a message and undo the redirect. A pnp rom should not need this, we have BEVs and BCVs for that. Nevertheless there are roms in the wild which are redirecting int19. At least some BIOS implementations for physical hardware have a config option in the setup to allow/disallow int19 redirections, so just not allowing this seems to be the way to deal with this situation. Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1642135 Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- src/optionroms.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/optionroms.c b/src/optionroms.c index fc992f649f..4ec5504ca9 100644 --- a/src/optionroms.c +++ b/src/optionroms.c @@ -8,6 +8,7 @@ #include "bregs.h" // struct bregs #include "config.h" // CONFIG_* #include "farptr.h" // FLATPTR_TO_SEG +#include "biosvar.h" // GET_IVT #include "hw/pci.h" // pci_config_readl #include "hw/pcidevice.h" // foreachpci #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA @@ -136,9 +137,24 @@ init_optionrom(struct rom_header *rom, u16 bdf, int isvga) tpm_option_rom(newrom, rom->size * 512); - if (isvga || get_pnp_rom(newrom)) + struct pnp_data *pnp = get_pnp_rom(newrom); + if (isvga || pnp) { + struct segoff_s old19, new19; // Only init vga and PnP roms here. + old19 = GET_IVT(0x19); callrom(newrom, bdf); + new19 = GET_IVT(0x19); + if (old19.seg != new19.seg || + old19.offset != new19.offset) { + dprintf(1, "WARNING! rom tried to hijack int19 " + "(vec %04x:%04x, pnp %s, bev %s, bvc %s)\n", + new19.seg, new19.offset, + pnp ? "yes" : "no", + pnp && pnp->bev ? "yes" : "no", + pnp && pnp->bcv ? "yes" : "no"); + SET_IVT(0x19, old19); + } + } return rom_confirm(newrom->size * 512); } -- 2.9.3

2 years, 5 months

Re: [SeaBIOS] [coreboot] Upgrade the 12 years old LZMA libraries - should we do it?

by Carl-Daniel Hailfinger

Hi Ivan, On 03.04.2018 20:03, Ivan Ivanov wrote: > I have noticed that both coreboot and seabios are using the very old > versions of LZMA SDK. True. I introduced the lzma code in coreboot (back when it was called LinuxBIOS) when we were working on OLPC XO-1 support. > If we will upgrade our LZMA libraries from the > outdated-by-12-years 4.42 to the current version 18.04 , speed and > compression ratio should improve and maybe a few bugs will be fixed. Do you have any numbers for this? An improved compression ratio and improved speed would be nice indeed, but how does the size of the decompression code change? If the decompression code grows more than the size reduction from better compression, it would be a net loss. A significantly reduced decompression speed would also be a problem. Decompression speed would have to be measured both for stream decompression (i.e. the decompressor gets the compressed data in single-byte or multibyte chunks) as well as full-size decompression (i.e. the decompressor can access all compressed data at once). We also have to make sure that stream decompression still works after the change. > Do you think it should be done, or you are OK with using such an > outdated version? A size benefit for the resulting image is a good reason to switch. Regards, Carl-Daniel

2 years, 8 months

USB-to-SATA bridge conflicts with USB boot flash drive

by Аладышев Константин

Hello! I use coreboot+SeaBIOS bundle on custom motherboard with Intel Haswell i7 and Lynxpoint-LP chipset and I've come to a strange issue with USB-to-SATA bridge TUSB9261 (http://www.ti.com/product/TUSB9261). In my experiments I plug HDD to one USB port, and USB boot flash drive to another USB port. Everything works fine when HDD is present on USB-to-SATA bridge. SeaBIOS displays both devices in its boot menu. But if HDD is not present and if USB boot flash drive is plugged to a port greater than USB-to-SATA bridge port, SeaBIOS doesn't see flash drive as bootable device. In this case I see in log messages similar to this: """ Searching bootorder for: /pci@i0cf8/usb@1d/hub@1/storage@7/*@0/*@0,0 Searching bootorder for: /pci@i0cf8/usb@1d/hub@1/usb-*@7 ehci_send_pipe qh=0x000eab80 dir=0 data=0x00006b79 size=31 ehci_send_pipe qh=0x000eab00 dir=128 data=0x00006c98 size=36 WARNING - Timeout at ehci_wait_td:516! ehci pipe=0x000eab00 cur=000069c0 tok=80240d80 next=1 td=0x000069c0 status=240d80 USB transmission failed Unable to configure USB MSC drive. phys_free f4950 (detail=0x7f6c13c0) Unable to configure USB MSC device. """ Summarizing all the above with SeaBIOS logs: SeaBIOS displays flash drive correctly as bootable device when: - HDD is present on SATA-to-USB port (https://pastebin.com/SuKBkQrq) - HDD is not present on USB port, but SATA-to-USB port number is greater than USB flash drive number (https://pastebin.com/9f6RTBHB) SeaBIOS doesn't display flash drive as bootable device when: - HDD is not present on USB port and SATA-to-USB port number is less than USB flash drive number (https://pastebin.com/abBPhej0) What can be the source of the problem and what can I do to solve this issue? Best Regards, Aladyshev Konstantin

2 years, 9 months

Re: [SeaBIOS] [PATCH v2] ramdisk: search for all available floppy images instead of one

by Kevin O'Connor

On Thu, Dec 06, 2018 at 07:12:19PM +0300, Mike Banon wrote: > Hi Kevin, thank you very much for the suggestions! This is a second > version of my "more_than_one_floppy" patch : now it will search for > the max size of floppy stored inside CBFS, then allocate this size of > RAM and use it later for any floppy selected by user. sha256 = > 2f8bb18c1606d437de49e585769f100c085234de29d8520e2dc74d25b7f7645a Thanks, see my comments below. [...] > --- > diff --git a/src/block.h b/src/block.h > index f64e880..aaa236f 100644 > --- a/src/block.h > +++ b/src/block.h > @@ -2,7 +2,7 @@ > #define __BLOCK_H > > #include "types.h" // u32 > - > +#include "romfile.h" // struct romfile_s > > /**************************************************************** > * Disk command request > @@ -48,6 +48,7 @@ struct drive_s { > struct drive_s { > u8 type; // Driver type (DTYPE_*) > u8 floppy_type; // Type of floppy (only for floppy drives). > + struct romfile_s *floppy_file; // Floppy file (only for virtual floppies). > struct chs_s lchs; // Logical CHS > u64 sectors; // Total sectors count > u32 cntl_id; // Unique id for a given driver type. Driver specific fields should not be added to drive_s - the code should use the container_of() trick to allocate driver specific fields. (Create and allocate a custom driver struct that contains a 'struct drive_s' and when called with a drive_s use container_of() to get a pointer back to that main struct - for an example, take a look at what src/hw/usb-msc.c does.) > diff --git a/src/boot.c b/src/boot.c > index 9f82f3c..79f1e7d 100644 > --- a/src/boot.c > +++ b/src/boot.c > @@ -584,7 +584,7 @@ bcv_prepboot(void) > break; > case IPL_TYPE_FLOPPY: > map_floppy_drive(pos->drive); > - add_bev(IPL_TYPE_FLOPPY, 0); > + add_bev(IPL_TYPE_FLOPPY, (u32)pos->drive); > break; > case IPL_TYPE_HARDDISK: > map_hd_drive(pos->drive); > @@ -733,6 +733,12 @@ do_boot(int seq_nr) > static void > do_boot(int seq_nr) > { > + > + int ret; > + void *pos; > + struct romfile_s *file; > + struct drive_s *drive; > + > if (! CONFIG_BOOT) > panic("Boot support not compiled in.\n"); > > @@ -744,6 +750,16 @@ do_boot(int seq_nr) > switch (ie->type) { > case IPL_TYPE_FLOPPY: > printf("Booting from Floppy...\n"); > + drive = (struct drive_s *)ie->vector; > + file = drive->floppy_file; > + // File is NULL if a floppy is physical. > + if (file) { > + // Copy virtual floppy image into ram. > + pos = (void *)drive->cntl_id; > + ret = file->copy(file, pos, file->size); > + if (ret < 0) > + break; > + } Similarly, we shouldn't add driver specific code into the boot phase. A better place to do this would be during the BCV mapping phase (eg, bcv_prepboot() ). The idea is to do the work after the boot menu, but before the boot phase. Ideally the code would detect the driver wants a callback and then call driver specific code that resides in src/hw/ramdisk.c. -Kevin

2 years, 10 months

[PATCH] shadow: set code segment to high rom region when enabling RAM

by Evgeny Yakovlev

Currently make_bios_writable_intel will call __make_bios_writeable_intel from high rom memory by manually correcting its offset to make sure that we safely execute it while overriding memory mapping through PAMs However we still may call code from low memory, when __make_bios_writeable_intel itself calls other code without manual pointer adjustments. Right now it calls pci_config_readl and pci_config_writel. Consider this scenario: 0. Linker puts pci_config_writel in F-segment. 1. first pci_config_writel is called to reprogram PAM0-3, which means remap regions 0xF0000-0xFFFFF and 0xD0000 - 0xC7FFF. 2. second pci_config_writel is called to reprogram PAM4-7 but code in F-segment is no longer valid, including pci_config_writel. However we don't crash due to instruction cache being hot between two calls. Adding manual i-cache flush (by reloading the same CS segment for example) between two calls finally crashes the firmware. This change wraps a call to __make_bios_writable_bios by setting code segment at base 0xFFF00000. This way simple nested function calls work as long as no data fetches happen during execution through CS segment (i.e. something like dprintf is still not safe to call). Signed-off-by: Evgeny Yakovlev <wrfsh(a)yandex-team.ru> --- src/config.h | 16 ++++++++++------ src/fw/shadow.c | 37 ++++++++++++++++++++++++++++--------- src/misc.c | 8 ++++++++ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/config.h b/src/config.h index 93c8dbc..87c4118 100644 --- a/src/config.h +++ b/src/config.h @@ -60,13 +60,17 @@ #define SEG_BDA 0x0040 #define SEG_BIOS 0xf000 +// On the emulators, the bios at 0xf0000 is also at 0xffff0000 +#define BIOS_SRC_OFFSET 0xfff00000 + // Segment definitions in protected mode (see rombios32_gdt in misc.c) -#define SEG32_MODE32_CS (1 << 3) -#define SEG32_MODE32_DS (2 << 3) -#define SEG32_MODE16_CS (3 << 3) -#define SEG32_MODE16_DS (4 << 3) -#define SEG32_MODE16BIG_CS (5 << 3) -#define SEG32_MODE16BIG_DS (6 << 3) +#define SEG32_MODE32_CS (1 << 3) +#define SEG32_MODE32_DS (2 << 3) +#define SEG32_MODE16_CS (3 << 3) +#define SEG32_MODE16_DS (4 << 3) +#define SEG32_MODE16BIG_CS (5 << 3) +#define SEG32_MODE16BIG_DS (6 << 3) +#define SEG32_MODE32_HIGH_CS (7 << 3) // Debugging levels. If non-zero and CONFIG_DEBUG_LEVEL is greater // than the specified value, then the corresponding irq handler will diff --git a/src/fw/shadow.c b/src/fw/shadow.c index 4c627a8..80d0889 100644 --- a/src/fw/shadow.c +++ b/src/fw/shadow.c @@ -18,9 +18,6 @@ #include "util.h" // make_bios_writable #include "x86.h" // wbinvd -// On the emulators, the bios at 0xf0000 is also at 0xffff0000 -#define BIOS_SRC_OFFSET 0xfff00000 - union pamdata_u { u8 data8[8]; u32 data32[2]; @@ -56,6 +53,30 @@ __make_bios_writable_intel(u16 bdf, u32 pam0) , SYMBOL(code32flat_end) - SYMBOL(code32flat_start)); } +// A wrapper for executing __make_bios_writable_intel in high memory code segment +static void +__make_bios_writable_intel_highmem(u16 bdf, u32 pam0) +{ + // Save whatever CS segment we've had before entering here + // and switch to highmem code + u16 prev_cs; + __asm__ __volatile__ ("mov %%cs, %0":"=rm"(prev_cs)); + __asm__ __volatile__ ("ljmp %0, $__make_bios_writable_intel_highmem__enter\n" + "__make_bios_writable_intel_highmem__enter:\n" + : + : "n"(SEG32_MODE32_HIGH_CS)); + + __make_bios_writable_intel(bdf, pam0); + + // Far jump to return to previous segment + __asm__ __volatile__ ("push %0\n" + "push $__make_bios_writable_intel_highmem__exit\n" + "retf\n" + "__make_bios_writable_intel_highmem__exit:\n" + : + : "rm"((u32)prev_cs)); +} + static void make_bios_writable_intel(u16 bdf, u32 pam0) { @@ -65,13 +86,11 @@ make_bios_writable_intel(u16 bdf, u32 pam0) // if ram isn't backing the bios segment when shadowing is // disabled, the code itself won't be in memory. So, run the // code from the high-memory flash location. - u32 pos = (u32)__make_bios_writable_intel + BIOS_SRC_OFFSET; - void (*func)(u16 bdf, u32 pam0) = (void*)pos; - func(bdf, pam0); - return; + __make_bios_writable_intel_highmem(bdf, pam0); + } else { + // Ram already present - just enable writes + __make_bios_writable_intel(bdf, pam0); } - // Ram already present - just enable writes - __make_bios_writable_intel(bdf, pam0); } static void diff --git a/src/misc.c b/src/misc.c index b511730..cb931a5 100644 --- a/src/misc.c +++ b/src/misc.c @@ -160,6 +160,14 @@ u64 rombios32_gdt[] VARFSEG __aligned(8) = { GDT_GRANLIMIT(0xffffffff) | GDT_CODE | GDT_BASE(BUILD_BIOS_ADDR), // 16 bit data segment base=0 limit=0xffffffff (SEG32_MODE16BIG_DS) GDT_GRANLIMIT(0xffffffff) | GDT_DATA, + + // + // Following segments are used when enabling shadow memory + // where we need to execute code strictly from base 0xFFF00000 + // + + // 32 bit code segment in high memory + GDT_GRANLIMIT(0xfffff) | GDT_CODE | GDT_B | GDT_BASE(BIOS_SRC_OFFSET), }; // GDT descriptor -- 2.7.4

2 years, 10 months

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

by Kevin O'Connor

On Mon, Dec 10, 2018 at 03:18:46PM +0100, Stefano Garzarella wrote: > On Wed, Dec 5, 2018 at 6:00 PM Kevin O'Connor <kevin(a)koconnor.net> wrote: > > On Fri, Nov 30, 2018 at 11:23:30AM +0100, Stefano Garzarella wrote: > > > On Thu, Nov 29, 2018 at 5:55 PM Kevin O'Connor <kevin(a)koconnor.net> wrote: > > > > --- a/src/output.c > > > > +++ b/src/output.c > > > > @@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc }; > > > > static void > > > > screenc(char c) > > > > { > > > > + if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff) > > > > + // No need to thunk to 16bit mode if vgabios is not present > > > > + return; > > > > struct bregs br; > > > > memset(&br, 0, sizeof(br)); > > > > br.flags = F_IF; > > > > > > Do you plan to commit this patch? > > > > Only if it's useful - does sercon make it not worthwhile? > > Yes, I think it is useful. If I don't use "-nographic" it works as expected. FYI, I committed this change. -Kevin

2 years, 10 months

Re: [SeaBIOS] [PATCH] qemu: avoid debug prints if debugcon is not enabled

by Kevin O'Connor

On Tue, Dec 11, 2018 at 10:10:33AM +0100, Stefano Garzarella wrote: > On Tue, Dec 11, 2018 at 4:08 AM Kevin O'Connor <kevin(a)koconnor.net> wrote: > > > > On Sun, Dec 02, 2018 at 02:10:13PM +0100, Stefano Garzarella wrote: > > > In order to speed up the boot phase, we can check the QEMU > > > debugcon device, and disable the writes if it is not recognized. > > > > > > This patch allow us to save around 10 msec (time measured > > > between SeaBIOS entry point and "linuxboot" entry point) > > > when CONFIG_DEBUG_LEVEL=1 and debugcon is not enabled. > > > > Thanks. In order to properly track reboots, the HaveRunPost variable > > must always be the first global variable to be modified. So, the qemu > > debug init must come later in the boot. How about the patch below > > instead? > > Thanks for the explanation! It looks good! Thanks. I committed this change. -Kevin

2 years, 10 months

[PATCH] qemu: avoid debug prints if debugcon is not enabled

by Stefano Garzarella

In order to speed up the boot phase, we can check the QEMU debugcon device, and disable the writes if it is not recognized. This patch allow us to save around 10 msec (time measured between SeaBIOS entry point and "linuxboot" entry point) when CONFIG_DEBUG_LEVEL=1 and debugcon is not enabled. Signed-off-by: Stefano Garzarella <sgarzare(a)redhat.com> --- src/fw/paravirt.h | 3 +++ src/hw/serialio.c | 16 ++++++++++++++-- src/hw/serialio.h | 1 + src/post.c | 3 +++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h index a14d83e..f7e1d4c 100644 --- a/src/fw/paravirt.h +++ b/src/fw/paravirt.h @@ -49,6 +49,9 @@ static inline int runningOnKVM(void) { // QEMU_CFG_DMA ID bit #define QEMU_CFG_VERSION_DMA 2 +// QEMU debugcon read value +#define QEMU_DEBUGCON_READBACK 0xe9 + int qemu_cfg_enabled(void); int qemu_cfg_dma_enabled(void); void qemu_preinit(void); diff --git a/src/hw/serialio.c b/src/hw/serialio.c index 319a85c..d9acd3c 100644 --- a/src/hw/serialio.c +++ b/src/hw/serialio.c @@ -103,11 +103,23 @@ serial_debug_flush(void) u16 DebugOutputPort VARFSEG = 0x402; +void +qemu_debug_postram_preinit(void) +{ + /* Check if the QEMU debug output port is active */ + if (CONFIG_DEBUG_IO && + inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK) + DebugOutputPort = 0; +} + // Write a character to the special debugging port. void qemu_debug_putc(char c) { - if (CONFIG_DEBUG_IO && runningOnQEMU()) + u16 port; + + if (CONFIG_DEBUG_IO && runningOnQEMU() && + (port = GET_GLOBAL(DebugOutputPort))) // Send character to debug port. - outb(c, GET_GLOBAL(DebugOutputPort)); + outb(c, port); } diff --git a/src/hw/serialio.h b/src/hw/serialio.h index 88296fe..31fbea3 100644 --- a/src/hw/serialio.h +++ b/src/hw/serialio.h @@ -24,6 +24,7 @@ void serial_debug_preinit(void); void serial_debug_putc(char c); void serial_debug_flush(void); extern u16 DebugOutputPort; +void qemu_debug_postram_preinit(void); void qemu_debug_putc(char c); #endif // serialio.h diff --git a/src/post.c b/src/post.c index f93106a..650a314 100644 --- a/src/post.c +++ b/src/post.c @@ -332,6 +332,9 @@ handle_post(void) // Allow writes to modify bios area (0xf0000) make_bios_writable(); + // Setup QEMU debug output port + qemu_debug_postram_preinit(); + // Now that memory is read/writable - start post process. dopost(); } -- 2.19.2

2 years, 10 months

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

by Kevin O'Connor

On Fri, Nov 30, 2018 at 11:23:30AM +0100, Stefano Garzarella wrote: > On Thu, Nov 29, 2018 at 5:55 PM Kevin O'Connor <kevin(a)koconnor.net> wrote: > > Interesting. I tracked down this printf delay - it's due to the > > save/restore of cpu state when thunking to 16bit mode. (For every > > character displayed on the screen the code enters 16bit mode to invoke > > the vgabios and it saves/restores the cr0, gdt, fs, gs, a20, nmi > > states during that process.) It's trivial to eliminate the calls when > > there is no vgabios though (see patch below). > > Thanks, the patch works, but unfortunately, when I use qemu > -nographic, the /etc/sercon-port is set to PORT_SERIAL1 (in > src/fw/paravirt.c:623), bypassing the patch. > Maybe in QEMU is better to set /etc/sercom-port to 0 when there is no > serial port, or when we want a fast boot. You should be able to use "-device VGA,romfile=" instead. > > --- a/src/output.c > > +++ b/src/output.c > > @@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc }; > > static void > > screenc(char c) > > { > > + if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff) > > + // No need to thunk to 16bit mode if vgabios is not present > > + return; > > struct bregs br; > > memset(&br, 0, sizeof(br)); > > br.flags = F_IF; > > Do you plan to commit this patch? Only if it's useful - does sercon make it not worthwhile? -Kevin

2 years, 10 months

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS December 2018