SeaBIOS January 2022

seabios@seabios.org

16 participants
18 discussions

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag
by Igor Mammedov 21 Jan '22

21 Jan '22

On Wed, 19 Jan 2022 15:48:20 +0000 Peter Maydell <peter.maydell(a)linaro.org> wrote: > On Wed, 19 Jan 2022 at 14:44, Godmar Back <gback(a)cs.vt.edu> wrote: > > after upgrading to 6.2.0, I observed that code such as MIT's xv6 (see > > [1]) is no longer able to detect multiple CPUs. Their code works in > > 6.1.1, however. > > Hi; this isn't a great place for reporting QEMU bugs, because > it's more of a user-to-user discussion list. Not all QEMU > developers read it. I've cc'd the ACPI maintainers, who > hopefully may have an idea about what's happening here. > You could also file a bug at > https://gitlab.com/qemu-project/qemu/-/issues > > > I built 6.1.1 from source and 6.2.0 from source and I have also tested > > with CentOS stream's 6.1.1 qemu-kvm and was able to pinpoint this > > change to these 2 versions of qemu. I am using qemu-system-i386 > > specifically. > > > > I tried to go through the ChangeLog to see if the `-smp` option was > > deprecated or changed. I found this note [2] about invalid topologies > > having been removed in 5.2. Here's what I found after long > > experimentation: > > > > The legacy MP tables appear to work only if you specify the longform > > `-smp cpus=4,cores=1,threads=1,sockets=4` in 6.2.0. If you specify > > `-smp 4` or `-smp cpus=4` it will not work in 6.2.0 (it worked in > > 6.1.1). I am guessing that perhaps the MP tables add entries for each > > socket, but that perhaps the behavior of the shortcuts `-smp n` and > > `-smp cpus=n` was changed to influence the number of cores rather than > > sockets. > > > > In other words, `-smp cpus=n` now means `-smp > > cpus=n,cores=n,threads=1,sockets=1` whereas in 6.1.1 and before it > > meant `-smp cpus=n,cores=1,threads=1,sockets=n`. I note that > > specifying `-smp cpus=4,cores=4,threads=1,sockets=1` in 6.1.1 also > > does not create 4 entries in the legacy MP tables. Thanks for reporting issue in such a detailed way. QEMU doesn't generate legacy MP tables and as reported the above issue is still present in earlier versions when cores are used. Well seabios has a comment: /* Only populate the MPS tables with the first logical CPU in each package */ So I'd guess it has never worked for anything but sockets. With QEMU starting to prefer cores over sockets by default I'd suggest to either * explicitly provide desired topology (i.e. sockets) * use older machine type which still preffers sockets by default (ex: up to 6.1 machine types) If anybody cares about legacy tables + cores/threads usecase, I suggest to investigate what can be done on SeaBIOS side which generates MP tables (assuming if anything could be done at all). CCing SeaBIOS mail-list. > > Can someone confirm/deny this? If so, it's a breaking change that > > perhaps could be mentioned in the Changelog to save others the time > > when they upgrade. Affected educational OS include MIT's xv6 and > > Stanford's pintos OS. Legacy MP table is not actively maintained part of the code, hence it's configuration which is not tested. However if someone is interested in maintaining this, one should contribute at least a testcase that will warn developers early if usecase is broken. We can't promise not breaking it ever but at least we would be able to document any breaking changes in release notes. > > Thanks for all the work you do on qemu! > > > > - Godmar > > > > [1] https://github.com/mit-pdos/xv6-public/blob/eeb7b415dbcb12cc362d0783e41c3d1… > > [2] https://qemu-project.gitlab.io/qemu/about/removed-features.html#smp-invalid… > > > > (I'm typing this email in gmail using the plaintext setting, hopefully > > it'll preserve line breaks.) > > thanks > -- PMM >

3 3

[PATCH] sercon: add missing GET_LOW() to access rx_bytes
by Volker Rümelin 20 Jan '22

20 Jan '22

The variable rx_bytes is marked VARLOW. Add a missing GET_LOW() to access rx_bytes. Reported-by: Gabe Black <gabe.black(a)gmail.com> Signed-off-by: Volker Rümelin <vr_qemu(a)t-online.de> --- src/sercon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sercon.c b/src/sercon.c index 66a1f24..a33dd08 100644 --- a/src/sercon.c +++ b/src/sercon.c @@ -641,7 +641,7 @@ sercon_check_event(void) while (inb(addr + SEROFF_LSR) & 0x01) { byte = inb(addr + SEROFF_DATA); if (GET_LOW(rx_bytes) < sizeof(rx_buf)) { - SET_LOW(rx_buf[rx_bytes], byte); + SET_LOW(rx_buf[GET_LOW(rx_bytes)], byte); SET_LOW(rx_bytes, GET_LOW(rx_bytes) + 1); count++; } -- 2.31.1

2 2

[PATCH] nvme: Move PRP data to ZoneHigh
by Alexander Graf 19 Jan '22

19 Jan '22

Commit 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists") introduced multi-page requests using the NVMe PRP mechanism. To store the list and "first page to write to" hints, it added fields to the NVMe namespace struct. Unfortunately, that struct resides in fseg which is read-only at runtime. While KVM ignores the read-only part and allows writes, real hardware and TCG adhere to the semantics and ignore writes to the fseg region. The net effect of that is that reads and writes were always happening on address 0, unless they went through the bounce buffer logic. This patch splits moves all PRP maintenance data from the actual namespace allocation and allocates them in ZoneHigh instead. That way, the PRP list is fully read-write at runtime. Fixes: 01f2736cc905d ("nvme: Pass large I/O requests as PRP lists") Reported-by: Matt DeVillier <matt.devillier(a)gmail.com> Signed-off-by: Alexander Graf <graf(a)amazon.com> --- src/hw/nvme-int.h | 12 ++++++++---- src/hw/nvme.c | 24 ++++++++++++++++-------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/hw/nvme-int.h b/src/hw/nvme-int.h index a4c1555..ceb2c79 100644 --- a/src/hw/nvme-int.h +++ b/src/hw/nvme-int.h @@ -108,6 +108,13 @@ struct nvme_ctrl { struct nvme_cq io_cq; }; +/* Page List Maintenance Data */ +struct nvme_prp_info { + u32 prpl_len; + void *prp1; + u64 prpl[NVME_MAX_PRPL_ENTRIES]; +}; + struct nvme_namespace { struct drive_s drive; struct nvme_ctrl *ctrl; @@ -123,10 +130,7 @@ struct nvme_namespace { /* Page aligned buffer of size NVME_PAGE_SIZE. */ char *dma_buffer; - /* Page List */ - u32 prpl_len; - void *prp1; - u64 prpl[NVME_MAX_PRPL_ENTRIES]; + struct nvme_prp_info *prp; }; /* Data structures for NVMe admin identify commands */ diff --git a/src/hw/nvme.c b/src/hw/nvme.c index f035fa2..963c31e 100644 --- a/src/hw/nvme.c +++ b/src/hw/nvme.c @@ -267,6 +267,14 @@ nvme_probe_ns(struct nvme_ctrl *ctrl, u32 ns_idx, u8 mdts) ns->ns_id = ns_id; ns->lba_count = id->nsze; + ns->prp = malloc_high(sizeof(*ns->prp)); + if (!ns->prp) { + warn_noalloc(); + free(ns); + goto free_buffer; + } + memset(ns->prp, 0, sizeof(*ns->prp)); + struct nvme_lba_format *fmt = &id->lbaf[current_lba_format]; ns->block_size = 1U << fmt->lbads; @@ -431,10 +439,10 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count, if ((ns->block_size * count) > (NVME_PAGE_SIZE * 2)) { /* We need to describe more than 2 pages, rely on PRP List */ - prp2 = ns->prpl; + prp2 = ns->prp->prpl; } else if ((ns->block_size * count) > NVME_PAGE_SIZE) { /* Directly embed the 2nd page if we only need 2 pages */ - prp2 = (void *)(long)ns->prpl[0]; + prp2 = (void *)(long)ns->prp->prpl[0]; } else { /* One page is enough, don't expose anything else */ prp2 = NULL; @@ -465,15 +473,15 @@ nvme_io_readwrite(struct nvme_namespace *ns, u64 lba, char *buf, u16 count, static void nvme_reset_prpl(struct nvme_namespace *ns) { - ns->prpl_len = 0; + ns->prp->prpl_len = 0; } static int nvme_add_prpl(struct nvme_namespace *ns, u64 base) { - if (ns->prpl_len >= NVME_MAX_PRPL_ENTRIES) + if (ns->prp->prpl_len >= NVME_MAX_PRPL_ENTRIES) return -1; - ns->prpl[ns->prpl_len++] = base; + ns->prp->prpl[ns->prp->prpl_len++] = base; return 0; } @@ -492,7 +500,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count) size = count * ns->block_size; /* Special case for transfers that fit into PRP1, but are unaligned */ if (((size + (base & ~NVME_PAGE_MASK)) <= NVME_PAGE_SIZE)) { - ns->prp1 = op_buf; + ns->prp->prp1 = op_buf; return count; } @@ -507,7 +515,7 @@ static int nvme_build_prpl(struct nvme_namespace *ns, void *op_buf, u16 count) for (; size > 0; base += NVME_PAGE_SIZE, size -= NVME_PAGE_SIZE) { if (first_page) { /* First page is special */ - ns->prp1 = (void*)base; + ns->prp->prp1 = (void*)base; first_page = 0; continue; } @@ -726,7 +734,7 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, struct disk_op_s *op, int write) blocks = nvme_build_prpl(ns, op_buf, blocks_remaining); if (blocks) { - res = nvme_io_readwrite(ns, op->lba + i, ns->prp1, blocks, write); + res = nvme_io_readwrite(ns, op->lba + i, ns->prp->prp1, blocks, write); dprintf(5, "ns %u %s lba %llu+%u: %d\n", ns->ns_id, write ? "write" : "read", op->lba, blocks, res); -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879

2 1

Git Repo https circularly redirecting to itself?
by Pvt Snafu 19 Jan '22

19 Jan '22

Anyone else having a problem accessing https://git.seabios.org/seabios.git/ tonight? For me, it's just doing 301 redirects to itself until my client gives up. Same for a web browser. cURL output: > < HTTP/2 301 > < server: nginx > < date: Mon, 03 Jan 2022 04:24:47 GMT > < content-type: text/html; charset=utf-8 > < content-length: 48 > < content-security-policy: default-src 'none' > < location: /seabios.git/ There's more, but this seems to be the relevant bit.

2 1

Error when stack "hopping back" to check IRQs
by Gabe Black 19 Jan '22

19 Jan '22

Hi! I'm a major contributor to the gem5 open source computer architecture simulator, and I'm trying to get SeaBIOS and FreeDOS to run on it. We've had at least some level of x86 support on our simulator for a number of years now, but we've primarily focused on 64 bit mode. I've found a lot of bugs in our simulator as I've been going along, but despite my best efforts I haven't been able to find a way to blame my code for the bug I'm currently stuck on. I'm using a very stripped down configuration of SeaBIOS, and am using the serial console to interact with it since I haven't written a simulated VGA interface yet. It reads FreeDOS from their published QEMU disk image, and it starts up and prints a menu where it wants me to select from 4 different boot modes. Roughly when I send a character over the serial connection, the simulator crashes because the software running on it tried to access an address that has nothing behind it. I've been tracing this problem down, and I see an int 0x16 with code 0x1 happening, which is trying to check the keyboard status, I believe. That goes along, and eventually calls check_irqs, which calls need_hop_back which returns 1, and then calls "stack_hop_back" to call back into itself but on the "original callers stack" I 75% know what that's talking about, but I'm not 100%. Anyway, once we're on the other stack, we call into the clock_poll_irq, that calls clock_update, that calls the (inlined I think) sercon_check_event, and when that tries to SET_LOW(rx_buf[rx_bytes], byte) the bad access happens. At least I'm pretty confident that's where it happens, it could also be in one of the other lines right around there. The problem seems to be that the variable it's trying to access is supposed to be in the "e" segment, ie with selector 0xe000 and base address 0xe0000. The code that does this is here: fbd43: 8e c3 mov %bx,%es fbd45: 26 8a 16 9d f7 mov %es:-0x863,%dl fbd4a: 80 fa 0f cmp $0xf,%dl fbd4d: 77 e6 ja fbd35 <clock_update+0x6b> fbd4f: 66 0f b6 0e 9d f7 movzbl -0x863,%ecx <==== where it asplodes fbd55: 26 67 88 81 a0 f7 00 mov %al,%es:0xf7a0(%ecx) Note the comparison against 0xf, which I think is where it checks against the size of rx_buf. You can see here that this access is (I think) using the %ds register by default. It has an operand size prefix, and a 2 byte displacement of 0xf79d. Adding this to 0xe0000 gives 0xef79d, which from what I've seen is a pretty valid looking address, not far below where I have the BIOS ROM mapped in. Unfortunately when this has problems, %ds is actually 0x9d80, which gives a base of 0x9d800, which gives a linear address of 0xacf9d. This is in the middle of the (not yet implemented) VGA framebuffer which is why it dies. I then traced down why %ds has this value, and it's from the "hop back" step, specifically here: asm volatile( // Backup stack_pos and current %ss/%esp "movl %6, %4\n" "movw %%ss, %w3\n" "movl %%esp, %6\n" // Restore original callers' %ss/%esp "movl -4(%4), %5\n" "movl %5, %%ss\n" <======== Where %ss is set "movw %%ds:-8(%4), %%sp\n" "movl %5, %%ds\n" <======== Where %ds is set // Call func Note that in this code, *both* %ss and %ds are being set, and being set to the same thing. This value *was* successfully pulled off the saved data from when the int was originally called as far as I can tell, but this value of %ds does *not* seem to be correct, since the first time it's used it causes the bad access. Could you please help me figure out what's going wrong here? Is this supposed to work out somehow, and my simulator is just wrong (my bet, but what's it doing wrong?), or is this a bug in SeaBIOS? Am I using SeaBIOS in some way it's known not to work? Please let me know if you need any other info, I'll be more than happy to get this sorted out! Gabe

2 3

[PATCH v3 0/4] nvme: Add PRP List support
by Alexander Graf 18 Jan '22

18 Jan '22

While looking at VM bootup times, we stumbled over the fact that the NVMe code only does I/O operations of up to 4kb at a given point in time. This is usually ok, but if you have an OS that loads a lot of data on boot in combination to network backed storage, it shows in bootup times. There is no need to restrict ourselves to 4kb though. The INT13 call we receive gives us much larger chunks which we can just map into a native bigger NVMe I/O call if the request buffer is page aligned. This patch implements all logic required to do the above and gives a substantial performance boost on boot. v1 -> v2: - fix dprintf - Fix bounds check on PRP list add logic - Reduce PRP list to 15 entries embedded in the ns struct. This reduces BIOS reserved memory footprint by 4kb. v2 -> v3: - new patch: nvme: Split requests by maximum allowed size - Adjust the maximum request size to sector units. The hardware field describes it in pages. Alexander Graf (4): nvme: Record maximum allowed request size nvme: Allow to set PRP2 nvme: Pass large I/O requests as PRP lists nvme: Split requests by maximum allowed size src/hw/nvme-int.h | 16 ++++++- src/hw/nvme.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 121 insertions(+), 17 deletions(-) -- 2.16.4 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879

6 15

new november release?
by Gerd Hoffmann 14 Jan '22

14 Jan '22

Hi, It's been a while since the last release, we have some nice but unreleased bits in the master branch (tpm and nvme improvements for example), time for a new release I think. How about freeze in a week, release by roughly mid-november? take care, Gerd

3 13

coreboot and seabios
by rene de zwart 10 Jan '22

10 Jan '22

I have been trying to make a coreboot rom and have a few questions c.q. confirmations. As far as I understand the process . Coreboot has to initialize the memory to 32bit flatmode and call in layout.S:entry_elf(). see https://www.seabios.org/Execution_and_code_flow . seabios starts and does it's magic . seabios starts the linux kernel According to the linux bootprotocol (Documentation/x86/boot.rst) The linux kernel runs my computer perfect so many answers could be answered by the kernel source. as the same held for seabios and intel documentation . Linux drivers/i2c/busses: .. i2c-i801.c .. i2c-smbus.c . Linux arch/x86/boot: .. main.c .. pm.c . Seabios .. docs/memory_model.md . Intel® 965 Express Chipset Family .. 82p965.pdf Is this the correct file? questions

2 2

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS January 2022