March 2024 - SeaBIOS - mail.coreboot.org

Regression 32bit Kernel + Virtio-scsi
by Max Tottenham June 13, 2024

June 13, 2024

Hi Folks The commits between be848676..a6ed6b701 appear to break QEMU guests using a 32bit kernel when booting from virtio_scsi devices. I end up seeing the following in dmesg: [ 2.016642] virtio_scsi: probe of virtio0 failed with error -2 Notably - I've tried modifying the amount of RAM the geust boots with, based on the comments in the commit messages, PAE *is* also enabled in the guest kernel (the kernel v6.1.76). qemu-system-x86_64 .... -m 2048M - Guest boots fine qemu-system-x86_64 .... -m 4095M - Guest does not boot. In the latter case I still see the 64 bit address space being used in the BIOS debug log in the latter case: 00.006: === PCI bus & bridge init === 00.006: PCI: pci_bios_init_bus_rec bus = 0x0 00.007: === PCI device probing === 00.007: Found 8 PCI devices (max PCI bus is 00) 00.008: PCIe: using q35 mmconfig at 0xb0000000 00.008: === PCI new allocation pass #1 === 00.008: PCI: check devices 00.009: === PCI new allocation pass #2 === 00.009: PCI: IO: c000 - c11f 00.009: PCI: 32: 00000000c0000000 - 00000000fec00000 00.009: PCI: 64: 0000380000000000 - 0000380040000000 00.009: PCI: map device bdf=00:02.0 bar 4, addr 380000000000, size 00004000 [prefmem] 00.010: PCI: map device bdf=00:03.0 bar 4, addr 380000004000, size 00004000 [prefmem] 00.010: PCI: map device bdf=00:04.0 bar 4, addr 380000008000, size 00004000 [prefmem] 00.010: PCI: map device bdf=00:02.0 bar 0, addr 0000c000, size 00000040 [io] 00.011: PCI: map device bdf=00:03.0 bar 0, addr 0000c040, size 00000040 [io] 00.011: PCI: map device bdf=00:04.0 bar 0, addr 0000c080, size 00000040 [io] 00.011: PCI: map device bdf=00:1f.3 bar 4, addr 0000c0c0, size 00000040 [io] 00.011: PCI: map device bdf=00:1f.2 bar 4, addr 0000c100, size 00000020 [io] 00.012: PCI: map device bdf=00:04.0 bar 6, addr feb80000, size 00040000 [mem] 00.012: PCI: map device bdf=00:01.0 bar 6, addr febc0000, size 00010000 [mem] 00.012: PCI: map device bdf=00:01.0 bar 2, addr febd0000, size 00001000 [mem] 00.012: PCI: map device bdf=00:02.0 bar 1, addr febd1000, size 00001000 [mem] 00.013: PCI: map device bdf=00:03.0 bar 1, addr febd2000, size 00001000 [mem] 00.013: PCI: map device bdf=00:04.0 bar 1, addr febd3000, size 00001000 [mem] 00.013: PCI: map device bdf=00:1f.2 bar 5, addr febd4000, size 00001000 [mem] 00.013: PCI: map device bdf=00:01.0 bar 0, addr fd000000, size 01000000 [prefmem] 00.014: PCI: init bdf=00:00.0 id=8086:29c0 00.014: PCI: init bdf=00:01.0 id=1234:1111 00.014: PCI: init bdf=00:02.0 id=1af4:1004 00.015: PCI: init bdf=00:03.0 id=1af4:1004 00.016: PCI: init bdf=00:04.0 id=1af4:1000 I'm happy to help test/provide more logs if they are useful. -- Max Tottenham | Senior Software Engineer /(* Akamai Technologies

3 7

[PATCH 0/2] Minor vgabios comment and variable name changes
by Kevin O'Connor March 20, 2024

March 20, 2024

These two patches are name and comment changes only. The goal is to make the vgahw_xxx() functions a little more clear. The current vga mode description struct (struct vgamode_s) is passed around to avoid extra calls to get_current_mode(). However, this can be confusing as some vgahw_xxx() calls work on an arbitrary mode while others only work on the current video mode. Use separate names (curmode_g vs vmode_g) to make the distinction more clear. -Kevin Kevin O'Connor (2): vgasrc: Use curmode_g instead of vmode_g when mode is the current video mode vgasrc: Rename vgahw_get_linesize() to vgahw_minimum_linelength() vgasrc/bochsvga.c | 40 ++++++++++---------- vgasrc/bochsvga.h | 16 ++++---- vgasrc/cbvga.c | 30 +++++++-------- vgasrc/clext.c | 20 +++++----- vgasrc/stdvga.c | 40 +++++++++++--------- vgasrc/stdvga.h | 17 +++++---- vgasrc/stdvgamodes.c | 12 +++--- vgasrc/swcursor.c | 16 ++++---- vgasrc/vbe.c | 53 +++++++++++++------------- vgasrc/vgabios.c | 6 +-- vgasrc/vgafb.c | 88 ++++++++++++++++++++++---------------------- vgasrc/vgafb.h | 4 +- vgasrc/vgahw.h | 88 ++++++++++++++++++++++---------------------- vgasrc/vgautil.h | 31 ++++++++-------- 14 files changed, 233 insertions(+), 228 deletions(-) -- 2.44.0

1 3

[PATCH v2] vbe: implement function 09h (get/set palette data)
by Daniel Verkamp March 15, 2024

March 15, 2024

Since the VBE mode attributes indicate that all modes are not VGA compatible, applications must use VBE function 09h to manipulate the palette rather than directly accessing the VGA registers. This implementation uses the standard VGA registers for all hardware, which may not be appropriate; I only verified qemu -device VGA. Without this patch, the get/set palette function returns an error code, so programs that use 8-bit indexed color modes fail. For example, Quake (DOS) printed "Error: Unable to load VESA palette" and exited when trying to set a SVGA mode like 640x480, but with the patch it succeeds. This fixes qemu issue #251 and #1862. <https://gitlab.com/qemu-project/qemu/-/issues/251> <https://gitlab.com/qemu-project/qemu/-/issues/1862> Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- V2: use existing stdvga_dac_read/write() functions vgasrc/vbe.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++ vgasrc/vgahw.h | 10 +++++++++ 2 files changed, 71 insertions(+) diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 91abc9ab..7e4b80bf 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -376,6 +376,66 @@ fail: regs->ax = 0x014f; } +static void +vbe_104f09(struct bregs *regs) +{ + struct vgamode_s *vmode_g = get_current_mode(); + if (! vmode_g) + goto fail; + u8 memmodel = GET_GLOBAL(vmode_g->memmodel); + u8 depth = GET_GLOBAL(vmode_g->depth); + if (memmodel == MM_DIRECT || memmodel == MM_YUV || depth > 8) { + regs->ax = 0x034f; + return; + } + if (regs->dh) + goto fail; + u8 start = regs->dl; + int count = regs->cx; + int max_colors = 1 << depth; + if (start + count > max_colors) + goto fail; + u16 seg = regs->es; + u8 *data_far = (void*)(regs->di+0); + u8 rgb[3]; + int ret = 0; + int i; + switch (regs->bl) { + case 0x80: + case 0x00: + for (i = 0; i < count; i++) { + SET_SEG(ES, seg); + rgb[0] = GET_VAR(ES, data_far[i*4 + 2]); + rgb[1] = GET_VAR(ES, data_far[i*4 + 1]); + rgb[2] = GET_VAR(ES, data_far[i*4 + 0]); + ret = vgahw_dac_write(GET_SEG(SS), rgb, start + i, 1); + if (ret < 0) + goto fail; + } + break; + case 0x01: + for (i = 0; i < count; i++) { + ret = vgahw_dac_read(GET_SEG(SS), rgb, start + i, 1); + if (ret < 0) + goto fail; + SET_SEG(ES, seg); + SET_VAR(ES, data_far[i*4 + 0], rgb[2]); + SET_VAR(ES, data_far[i*4 + 1], rgb[1]); + SET_VAR(ES, data_far[i*4 + 2], rgb[0]); + SET_VAR(ES, data_far[i*4 + 3], 0); + } + break; + default: + goto fail; + } + if (ret < 0) + goto fail; + regs->ax = 0x004f; + return; +fail: + regs->ax = 0x014f; +} + static void vbe_104f0a(struct bregs *regs) { @@ -456,6 +516,7 @@ handle_104f(struct bregs *regs) case 0x06: vbe_104f06(regs); break; case 0x07: vbe_104f07(regs); break; case 0x08: vbe_104f08(regs); break; + case 0x09: vbe_104f09(regs); break; case 0x0a: vbe_104f0a(regs); break; case 0x10: vbe_104f10(regs); break; case 0x15: vbe_104f15(regs); break; diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h index 8b64660e..b10075ac 100644 --- a/vgasrc/vgahw.h +++ b/vgasrc/vgahw.h @@ -141,6 +141,16 @@ static inline int vgahw_set_dacformat(struct vgamode_s *vmode_g, int val) { return stdvga_set_dacformat(vmode_g, val); } +static inline int vgahw_dac_write(u16 seg, u8 *data_far, u8 start, int count) { + stdvga_dac_write(seg, data_far, start, count); + return 0; +} + +static inline int vgahw_dac_read(u16 seg, u8 *data_far, u8 start, int count) { + stdvga_dac_read(seg, data_far, start, count); + return 0; +} + static inline int vgahw_save_restore(int cmd, u16 seg, void *data) { if (CONFIG_VGA_CIRRUS) return clext_save_restore(cmd, seg, data); -- 2.43.0

2 3

[PATCH v2] vgasrc: round up save/restore size
by Daniel Verkamp March 15, 2024

March 15, 2024

When calculating the size of the buffer required for the VGA/VBE state, round up rather than truncating when dividing the number of bytes to get the number of 64-byte blocks. Without this modification, the save state function will write past the end of a buffer of the size requested. Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- V2: apply the fix to VGA function 101c as well vgasrc/vbe.c | 2 +- vgasrc/vgabios.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 66afb011..1a0948cf 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -242,7 +242,7 @@ vbe_104f04(struct bregs *regs) if (ret < 0) goto fail; if (cmd == 0) - regs->bx = ret / 64; + regs->bx = DIV_ROUND_UP(ret, 64); regs->ax = 0x004f; return; fail: diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c index 198ee555..73ba1c3d 100644 --- a/vgasrc/vgabios.c +++ b/vgasrc/vgabios.c @@ -1081,7 +1081,7 @@ handle_101c(struct bregs *regs) if (ret < 0) goto fail; if (cmd == 0) - regs->bx = ret / 64; + regs->bx = DIV_ROUND_UP(ret, 64); regs->al = 0x1c; fail: return; -- 2.43.0

3 3

[PATCH] vbe: implement function 09h (get/set palette data)
by Daniel Verkamp March 12, 2024

March 12, 2024

Since the VBE mode attributes indicate that all modes are not VGA compatible, applications must use VBE function 09h to manipulate the palette rather than directly accessing the VGA registers. This implementation uses the standard VGA registers for all hardware, which may not be appropriate; I only verified qemu -device VGA. Without this patch, the get/set palette function returns an error code, so programs that use 8-bit indexed color modes fail. For example, Quake (DOS) printed "Error: Unable to load VESA palette" and exited when trying to set a SVGA mode like 640x480, but with the patch it succeeds. This fixes qemu issue #251 and #1862. <https://gitlab.com/qemu-project/qemu/-/issues/251> <https://gitlab.com/qemu-project/qemu/-/issues/1862> Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- src/std/vbe.h | 7 +++++++ vgasrc/stdvga.h | 3 +++ vgasrc/stdvgaio.c | 30 ++++++++++++++++++++++++++++++ vgasrc/vbe.c | 42 ++++++++++++++++++++++++++++++++++++++++++ vgasrc/vgahw.h | 8 ++++++++ 5 files changed, 90 insertions(+) diff --git a/src/std/vbe.h b/src/std/vbe.h index fe96f5ec..fddaa4fd 100644 --- a/src/std/vbe.h +++ b/src/std/vbe.h @@ -88,6 +88,13 @@ struct vbe_crtc_info { u8 reserved[40]; } PACKED; +struct vbe_palette_entry { + u8 blue; + u8 green; + u8 red; + u8 align; +} PACKED; + /* VBE Return Status Info */ /* AL */ #define VBE_RETURN_STATUS_SUPPORTED 0x4F diff --git a/vgasrc/stdvga.h b/vgasrc/stdvga.h index 4184c600..af10fd78 100644 --- a/vgasrc/stdvga.h +++ b/vgasrc/stdvga.h @@ -74,6 +74,9 @@ int stdvga_get_displaystart(struct vgamode_s *vmode_g); int stdvga_set_displaystart(struct vgamode_s *vmode_g, int val); int stdvga_get_dacformat(struct vgamode_s *vmode_g); int stdvga_set_dacformat(struct vgamode_s *vmode_g, int val); +struct vbe_palette_entry; +int stdvga_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count); +int stdvga_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count); int stdvga_save_restore(int cmd, u16 seg, void *data); void stdvga_enable_video_addressing(u8 disable); int stdvga_setup(void); diff --git a/vgasrc/stdvgaio.c b/vgasrc/stdvgaio.c index 77fcecdf..ec22c4a1 100644 --- a/vgasrc/stdvgaio.c +++ b/vgasrc/stdvgaio.c @@ -6,6 +6,7 @@ #include "farptr.h" // GET_FARVAR #include "stdvga.h" // VGAREG_PEL_MASK +#include "std/vbe.h" // vbe_palette_entry #include "vgautil.h" // stdvga_pelmask_read #include "x86.h" // inb @@ -184,3 +185,32 @@ stdvga_dac_write(u16 seg, u8 *data_far, u8 start, int count) count--; } } + +int +stdvga_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) +{ + outb(start, VGAREG_DAC_WRITE_ADDRESS); + while (count) { + outb(GET_FARVAR(seg, pal_far->red), VGAREG_DAC_DATA); + outb(GET_FARVAR(seg, pal_far->green), VGAREG_DAC_DATA); + outb(GET_FARVAR(seg, pal_far->blue), VGAREG_DAC_DATA); + pal_far++; + count--; + } + return 0; +} + +int +stdvga_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) +{ + outb(start, VGAREG_DAC_READ_ADDRESS); + while (count) { + SET_FARVAR(seg, pal_far->red, inb(VGAREG_DAC_DATA)); + SET_FARVAR(seg, pal_far->green, inb(VGAREG_DAC_DATA)); + SET_FARVAR(seg, pal_far->blue, inb(VGAREG_DAC_DATA)); + SET_FARVAR(seg, pal_far->align, 0); + pal_far++; + count--; + } + return 0; +} diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 91abc9ab..0ac88805 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -376,6 +376,47 @@ fail: regs->ax = 0x014f; } +static void +vbe_104f09(struct bregs *regs) +{ + struct vgamode_s *vmode_g = get_current_mode(); + if (! vmode_g) + goto fail; + u8 memmodel = GET_GLOBAL(vmode_g->memmodel); + u8 depth = GET_GLOBAL(vmode_g->depth); + if (memmodel == MM_DIRECT || memmodel == MM_YUV || depth > 8) { + regs->ax = 0x034f; + return; + } + if (regs->dh) + goto fail; + u8 start = regs->dl; + int count = regs->cx; + int max_colors = 1 << depth; + if (start + count > max_colors) + goto fail; + u16 seg = regs->es; + struct vbe_palette_entry *pal = (void*)(regs->di+0); + int ret; + switch (regs->bl) { + case 0x80: + case 0x00: + ret = vgahw_set_palette_colors(seg, pal, start, count); + break; + case 0x01: + ret = vgahw_get_palette_colors(seg, pal, start, count); + break; + default: + goto fail; + } + if (ret < 0) + goto fail; + regs->ax = 0x004f; + return; +fail: + regs->ax = 0x014f; +} + static void vbe_104f0a(struct bregs *regs) { @@ -456,6 +497,7 @@ handle_104f(struct bregs *regs) case 0x06: vbe_104f06(regs); break; case 0x07: vbe_104f07(regs); break; case 0x08: vbe_104f08(regs); break; + case 0x09: vbe_104f09(regs); break; case 0x0a: vbe_104f0a(regs); break; case 0x10: vbe_104f10(regs); break; case 0x15: vbe_104f15(regs); break; diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h index 8b64660e..684530d6 100644 --- a/vgasrc/vgahw.h +++ b/vgasrc/vgahw.h @@ -141,6 +141,14 @@ static inline int vgahw_set_dacformat(struct vgamode_s *vmode_g, int val) { return stdvga_set_dacformat(vmode_g, val); } +static inline int vgahw_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) { + return stdvga_set_palette_colors(seg, pal_far, start, count); +} + +static inline int vgahw_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) { + return stdvga_get_palette_colors(seg, pal_far, start, count); +} + static inline int vgahw_save_restore(int cmd, u16 seg, void *data) { if (CONFIG_VGA_CIRRUS) return clext_save_restore(cmd, seg, data); -- 2.43.0

2 3

[PATCH] vbe: round up save/restore size
by Daniel Verkamp March 11, 2024

March 11, 2024

When calculating the size of the buffer required for the VBE state, round up rather than truncating when dividing the number of bytes to get the number of 64-byte blocks. Without this modification, the save state function will write past the end of a buffer of the size requested. Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- vgasrc/vbe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 66afb011..1a0948cf 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -242,7 +242,7 @@ vbe_104f04(struct bregs *regs) if (ret < 0) goto fail; if (cmd == 0) - regs->bx = ret / 64; + regs->bx = DIV_ROUND_UP(ret, 64); regs->ax = 0x004f; return; fail: -- 2.43.0

3 2

[PATCH v2] vbe: Add VBE 2.0+ OemData field to struct vbe_info
by Daniel Verkamp March 10, 2024

March 10, 2024

Per the VBE 2.0 specification, the VBE controller information is 512 bytes long when the "VBE2" signature is provided, instead of the original 256 bytes. src/bootsplash.c uses the original pre-VBE-2.0 256-byte structure while also filling in the "VBE2" signature, so a video BIOS that makes use of the VBE2 OemData area could write past the end of the allocated region. The original bootsplash code did not have this bug; it was introduced when the bootsplash VBE structures were merged with the VGA ROM struct definitions. Fixes: 69e941c159ed ("Merge bootsplash and VGA ROM vbe structure definitions") Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- v2 fixes the inverse bug introduced by the original patch - the vgabios would memset too much data if the caller did not request VBE2 data. src/std/vbe.h | 2 ++ vgasrc/vbe.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/std/vbe.h b/src/std/vbe.h index 94b4ad86..fe96f5ec 100644 --- a/src/std/vbe.h +++ b/src/std/vbe.h @@ -18,6 +18,8 @@ struct vbe_info { struct segoff_s oem_product_string; struct segoff_s oem_revision_string; u8 reserved[222]; + /* VBE 2.0 */ + u8 oem_data[256]; } PACKED; struct vbe_mode_info { diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 66afb011..91abc9ab 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -32,16 +32,18 @@ vbe_104f00(struct bregs *regs) { u16 seg = regs->es; struct vbe_info *info = (void*)(regs->di+0); + size_t info_size = offsetof(struct vbe_info, oem_data); if (GET_FARVAR(seg, info->signature) == VBE2_SIGNATURE) { dprintf(4, "Get VBE Controller: VBE2 Signature found\n"); + info_size = sizeof(*info); } else if (GET_FARVAR(seg, info->signature) == VESA_SIGNATURE) { dprintf(4, "Get VBE Controller: VESA Signature found\n"); } else { dprintf(4, "Get VBE Controller: Invalid Signature\n"); } - memset_far(seg, info, 0, sizeof(*info)); + memset_far(seg, info, 0, info_size); SET_FARVAR(seg, info->signature, VESA_SIGNATURE); -- 2.43.0

2 1

[PATCH] vbe: Add VBE 2.0+ OemData field to struct vbe_info
by Daniel Verkamp March 6, 2024

March 6, 2024

Per the VBE 2.0 specification, the VBE controller information is 512 bytes long when the "VBE2" signature is provided, instead of the original 256 bytes. src/bootsplash.c uses the original pre-VBE-2.0 256-byte structure while also filling in the "VBE2" signature, so a video BIOS that makes use of the VBE2 OemData area could write past the end of the allocated region. The original bootsplash code did not have this bug; it was introduced when the bootsplash VBE structures were merged with the VGA ROM struct definitions. Fixes: 69e941c159ed ("Merge bootsplash and VGA ROM vbe structure definitions") Signed-off-by: Daniel Verkamp <daniel(a)drv.nu> --- src/std/vbe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/std/vbe.h b/src/std/vbe.h index 94b4ad86..fe96f5ec 100644 --- a/src/std/vbe.h +++ b/src/std/vbe.h @@ -18,6 +18,8 @@ struct vbe_info { struct segoff_s oem_product_string; struct segoff_s oem_revision_string; u8 reserved[222]; + /* VBE 2.0 */ + u8 oem_data[256]; } PACKED; struct vbe_mode_info { -- 2.43.0

1 0

[PATCH] fix smbios blob length overflow
by Igor Mammedov March 3, 2024

March 3, 2024

When tables are more than 64K, size of copied tables will be truncated due to cast from u32 to u16, and as result only a small portion of the tables will be copied in the end. That leads to corrupted tables (a part from QEMU and remainder is whatever was in memory block allocated for the tables). Fix it by making qtables_len 32bit int. Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> --- src/fw/biostables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 1c09a785..90712d4b 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -499,7 +499,7 @@ smbios_build_tables(struct romfile_s *f_tables, u16 *number_of_structures) { struct smbios_type_0 *t0; - u16 qtables_len, need_t0 = 1; + u32 qtables_len, need_t0 = 1; u8 *qtables, *tables; if (f_tables->size != *length) -- 2.39.3

2 2