December 2015 - SeaBIOS - mail.coreboot.org

Re: [SeaBIOS] 5 basic features provided by CSM, which SeaBIOS does provide?
by Kevin O'Connor Dec. 21, 2015

Dec. 21, 2015

On Fri, Dec 18, 2015 at 08:43:37AM +0000, Stojsavljevic, Zoran wrote: > Hello both lists, Jiming, > > There are 5 basic features, provided by CSM SeaBIOS payload, and I > am looking to list of these features. > > Could you, please, provide to me list of these features, and some > description, and/or pointer to these descriptions? I don't fully understand your question. One can build SeaBIOS as a Compatibility Support Module (CSM) for UEFI, and this has been tested with OVMF on virtual machines. It's theoretically also possible to use it as a CSM on real hardware with a UEFI payload on coreboot, but to the best of my knowledge that has never been tested. When SeaBIOS is used as a CSM module, it provides legacy 16bit BIOS support on UEFI machines to bootloaders and option roms that require that support. The only link I have for SeaBIOS CSM is: http://www.seabios.org/Build_overview#Build_as_a_UEFI_Compatibility_Support… Cheers, -Kevin

2 1

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

Dec. 21, 2015

On Sun, Dec 20, 2015 at 09:49:54AM +0000, Gonglei (Arei) wrote: > > From: Kevin O'Connor [mailto:kevin@koconnor.net] > > Sent: Saturday, December 19, 2015 11:12 PM > > On Sat, Dec 19, 2015 at 12:03:15PM +0000, Gonglei (Arei) wrote: > > > Maybe the root cause is not NMI but INTR, so yield() can open hardware > > interrupt, > > > And then execute interrupt handler, but the interrupt handler make the > > SeaBIOS > > > stack broken, so that the BSP can't execute the instruction and occur > > exception, > > > VM_EXIT to Kmod, which is an infinite loop. But I don't have any proofs except > > > the surface phenomenon. > > > > I can't see any reason why allowing interrupts at this location would > > be a problem. > > > Does it have any relationship with *extra stack* of SeaBIOS? None that I can see. Also, the kvm trace seems to show the code trying to execute at rip=0x03 - that will crash long before the extra stack is used. > > > Kevin, can we drop yield() in smp_setup() ? > > > > It's possible to eliminate this instance of yield, but I think it > > would just push the crash to the next time interrupts are enabled. > > > Perhaps. I'm not sure. > > > > Is it really useful and allowable for SeaBIOS? Maybe for other components? > > > I'm not sure. Because we found that when SeaBIOS is booting, if we inject a > > > NMI by QMP, the guest will *stuck*. And the kvm tracing log is the same with > > > the current problem. > > > > If you apply the patches you had to prevent that NMI crash problem, > > does it also prevent the above crash? > > > Yes, but we cannot prevent the NMI injection (though I'll submit some patches to > forbid users' NMI injection after NMI_EN disabled by RTC bit7 of port 0x70). > -Kevin

2 1

Re: [SeaBIOS] SeaBIOS recognising USB 3.0 on boot works - partly
by Kevin O'Connor Dec. 20, 2015

Dec. 20, 2015

On Fri, Dec 18, 2015 at 11:51:16AM +0000, edward wandasiewicz wrote: > I thought, lets try swapping over the devices, but keeping the same > Type-C cables on the left and right hand side and see what happens. > > I get scenario 5. > > 5. NO - Type C & Type C - one Philiips & no Mushkin > > cbmem.no.TypeC.TypeC.one.Philips.no.Mushkin Unfortunately, that log gets truncated before the interesting part. Can you reproduce with debug log 3? (Using 46-g0cc3233 is fine.) -Kevin

3 8

[PATCH 1/2] xhci: Improve port status change debugging
by Kevin O'Connor Dec. 20, 2015

Dec. 20, 2015

Report the port state on a port status change notification. Clear the change bits so further change notifications are delivered. Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> --- src/hw/usb-xhci.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index 654feba..5f44239 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -658,9 +658,15 @@ static void xhci_process_events(struct usb_xhci_s *xhci) } case ER_PORT_STATUS_CHANGE: { - u32 portid = (etrb->ptr_low >> 24) & 0xff; - dprintf(3, "%s: status change port #%d\n", - __func__, portid); + u32 port = ((etrb->ptr_low >> 24) & 0xff) - 1; + // Read status, and clear port status change bits + u32 portsc = readl(&xhci->pr[port].portsc); + u32 pclear = (((portsc & ~(XHCI_PORTSC_PED|XHCI_PORTSC_PR)) + & ~(XHCI_PORTSC_PLS_MASK<<XHCI_PORTSC_PLS_SHIFT)) + | (1<<XHCI_PORTSC_PLS_SHIFT)); + writel(&xhci->pr[port].portsc, pclear); + + xhci_print_port_state(3, __func__, port, portsc); break; } default: -- 2.5.0

1 1

[PATCH V2] fw/pci: do not automatically allocate IO region for PCIe bridges
by Marcel Apfelbaum Dec. 20, 2015

Dec. 20, 2015

PCIe downstream ports (Root Ports and switches Downstream Ports) appear to firmware as PCI-PCI bridges and a 4K IO space is allocated for them even if there is no device behind them requesting IO space, all that for hotplug purpose. However, PCIe devices can work without IO, so there is no need to allocate IO space for hotplug. Signed-off-by: Marcel Apfelbaum <marcel(a)redhat.com> --- v1 -> v2: - Addressed Gerd's comments: - move pci_find_capability out of the loop and call it only once. - more descriptive names for variables Notes: - This patch fixes a 15 PCIe Root ports limitation when used with virtio pci-express devices having no IO space requirements: <qemu cmd line> -M q35 `for i in {1..15}; do echo -device ioh3420,chassis=$i,id=b$i -device virtio-net-pci,bus=b$i,disable-legacy=on; done` - There is a patch on QEMU devel list that tackles the problem from another angle by allowing PCIe downstream ports to not forward IO requests. https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04478.html - The mentioned patch is of course not enough and also requires management software intervention. Thanks, Marcel src/fw/pciinit.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index 7b8aab7..c31c2fa 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -645,9 +645,8 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev, return entry; } -static int pci_bus_hotplug_support(struct pci_bus *bus) +static int pci_bus_hotplug_support(struct pci_bus *bus, u8 pcie_cap) { - u8 pcie_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_EXP, 0); u8 shpc_cap; if (pcie_cap) { @@ -727,7 +726,8 @@ static int pci_bios_check_devices(struct pci_bus *busses) */ parent = &busses[0]; int type; - int hotplug_support = pci_bus_hotplug_support(s); + u8 pcie_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0); + int hotplug_support = pci_bus_hotplug_support(s, pcie_cap); for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; @@ -736,7 +736,8 @@ static int pci_bios_check_devices(struct pci_bus *busses) if (pci_region_align(&s->r[type]) > align) align = pci_region_align(&s->r[type]); u64 sum = pci_region_sum(&s->r[type]); - if (!sum && hotplug_support) + int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); + if (!sum && hotplug_support && !resource_optional) sum = align; /* reserve min size for hot-plug */ u64 size = ALIGN(sum, align); int is64 = pci_bios_bridge_region_is64(&s->r[type], -- 2.1.0

3 2

[PATCH 0/3] tpm: Small fixes and add menu for TPM control
by Stefan Berger Dec. 20, 2015

Dec. 20, 2015

This series provides two small fixes and then adds the menu for TPM control. Regards, Stefan Stefan Berger (3): Remove check for working TPM from TPM interrupt handler Check length parameter of the array tpm: Add a menu for TPM configuration src/boot.c | 9 +- src/std/tcg.h | 41 ++++ src/tcgbios.c | 661 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/tcgbios.h | 2 + src/util.h | 1 + 5 files changed, 708 insertions(+), 6 deletions(-) -- 2.4.3

3 10

[PATCH v2 0/3] tpm: Small fixes and add menu for TPM control
by Stefan Berger Dec. 20, 2015

Dec. 20, 2015

This series provides two small fixes and then adds the menu for TPM control. Regards, Stefan v1->v2: - using 'typedef u8 tpm_ppi_code' for storing the TPM menu's 'message codes' in. Stefan Berger (3): tpm: Remove check for working TPM from TPM interrupt handler tpm: Check length parameter of the array tpm: Add a menu for TPM configuration src/boot.c | 9 +- src/std/tcg.h | 41 ++++ src/tcgbios.c | 654 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/tcgbios.h | 2 + src/util.h | 1 + 5 files changed, 701 insertions(+), 6 deletions(-) -- 2.4.3

2 4

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

Dec. 20, 2015

On Sat, Dec 19, 2015 at 12:03:15PM +0000, Gonglei (Arei) wrote: > Maybe the root cause is not NMI but INTR, so yield() can open hardware interrupt, > And then execute interrupt handler, but the interrupt handler make the SeaBIOS > stack broken, so that the BSP can't execute the instruction and occur exception, > VM_EXIT to Kmod, which is an infinite loop. But I don't have any proofs except > the surface phenomenon. I can't see any reason why allowing interrupts at this location would be a problem. > Kevin, can we drop yield() in smp_setup() ? It's possible to eliminate this instance of yield, but I think it would just push the crash to the next time interrupts are enabled. > Is it really useful and allowable for SeaBIOS? Maybe for other components? > I'm not sure. Because we found that when SeaBIOS is booting, if we inject a > NMI by QMP, the guest will *stuck*. And the kvm tracing log is the same with > the current problem. If you apply the patches you had to prevent that NMI crash problem, does it also prevent the above crash? -Kevin

2 1

Re: [SeaBIOS] SeaBIOS recognising USB 3.0 on boot works - partly
by Kevin O'Connor Dec. 19, 2015

Dec. 19, 2015

On Sat, Dec 19, 2015 at 01:49:04AM +0000, edward wandasiewicz wrote: > Is it a technically a manufacturer messing up the USB spec definitions it > should be following? That's a great question, and I don't know the answer to it. It would be great if there was some guidance in the USB3 spec to help here as that would help point out if this is a controller, device, or software issue. > Of all the USB drives I've come across, this Philips one is the only one > that behaves like this. > > It's like its saying "hey, I'm a USB 2.0 spec device, no wait a moment, I'm > really a USB 3.0 spec device." and we turn around and say "make ya mind > up", by which time it's too late. We've seen this USB2/3 confusion pop up a few times before - for example, see: http://www.seabios.org/pipermail/seabios/2015-November/009907.html http://www.seabios.org/pipermail/seabios/2015-August/009626.html http://www.seabios.org/pipermail/seabios/2014-June/008151.html This is the first time I've seen a report of a device/controller having the confusion long enough for the USB2 instance to survive through full drive detection. (FYI, some of the issues in the emails above I think were resolved with other fixes.) > Sometimes we just can't fix every problem. > > Granted this one is a tough one to solve. I don't think it's that difficult to address your particular hardware - for example, see the patch below on top of the testing branch. It's unclear to me, however, what implications a change like the one below would have on other people's hardware. :-( -Kevin --- a/src/hw/usb.c +++ b/src/hw/usb.c @@ -441,10 +441,14 @@ usb_hub_port_setup(void *data) mutex_unlock(&hub->cntl->resetlock); // Make sure device is still present (catches some weird USB3 devices) - ret = hub->op->detect(hub, port); - if (ret <= 0) { - hub->op->disconnect(hub, port); - goto done; + if (hub->cntl->type == USB_TYPE_XHCI && usbdev->speed == USB_HIGHSPEED) { + while (!timer_check(hub->detectend)) + yield(); + ret = hub->op->detect(hub, port); + if (ret <= 0) { + hub->op->disconnect(hub, port); + goto done; + } } // Configure the device

2 1

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

Dec. 19, 2015

On Fri, Dec 18, 2015 at 03:04:58AM +0000, Gonglei (Arei) wrote: > Hi Kevin & Paolo, > > Luckily, I reproduced this problem last night. And I got the below log when SeaBIOS is stuck. [...] > [2015-12-18 10:38:10] gonglei: finish while [...] > <...>-31509 [035] 154753.180077: kvm_exit: reason EXCEPTION_NMI rip 0x3 info 0 80000306 > <...>-31509 [035] 154753.180077: kvm_emulate_insn: 0:3:f0 53 (real) > <...>-31509 [035] 154753.180077: kvm_inj_exception: #UD (0x0) > <...>-31509 [035] 154753.180077: kvm_entry: vcpu 0 This is an odd finding. It seems to indicate that the code is caught in an infinite irq loop once irqs are enabled. What doesn't make sense is that an NMI shouldn't depend on the cpu irq enable flag. Also, I can't explain why rip would be 0x03, nor why a #UD in an exception handler wouldn't result in a triple fault. Maybe someone with more kvm knowledge could help here. I did notice that you appear to be running with SeaBIOS v1.8.1 - I recommend you upgrade to the latest. There were two important fixes in this area (8b9942fa and 3156b71a). I don't think either of these fixes would explain the log above, but it would be best to eliminate the possibility. -Kevin

2 2