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
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
On Sat, Nov 24, 2018 at 11:46:53PM +0300, Mike Banon wrote:
> Dear Kevin,
>
> Please could you tell me, why the memory allocation functions like
> malloc_tmphigh() are failing at do_boot() stage? (boot.c) . Because of
> this "VARVERIFY32INIT" problem described in my letter above -
> https://mail.coreboot.org/pipermail/seabios/2018-November/012575.html
> - I rewrote all my code to allocate the RAM for one floppy only (space
> equal to max floppy size, which could be used by any selected floppy)
> . But when I'm trying to boot that selected floppy - it is failing,
> because these floppies are LZMA compressed inside CBFS and to
> decompress them another malloc_tmphigh() is needed (from
> cbfs_copyfile() / coreboot.c) / Although this code compiles fine (no
> VARVERIFY32INIT problem) - this malloc_tmphigh() function is always
> failing and SeaBIOS is freezing.
>
> Spent all my weekend debugging this problem and really stuck, any help
> will be highly appreciated
Hi,
Only the "post" stage is able to allocate ram. Once the "boot" phase
starts executing, areas of memory are locked down, and it's possible
for 3rd party software to become resident in various areas of ram.
SeaBIOS thus can't touch those areas of ram or modify the e820 map.
The VARVERIFY32INIT flag is there to catch this type of common error.
There's some info on this in the docs at:
https://www.seabios.org/Memory_Model#Memory_available_during_initialization
and:
https://www.seabios.org/Execution_and_code_flow
So, any type of allocation would have to occur before the boot stage.
It should be possible (though not necessarily easy) to perform the
allocation in the map_floppy_drive() phase of the code.
-Kevin
On Fri, Nov 30, 2018 at 03:27:21PM +0100, Stefano Garzarella wrote:
> On Fri, Nov 30, 2018 at 2:06 PM Gerd Hoffmann <kraxel(a)redhat.com> wrote:
> > > > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > > > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > > > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
> > >
> > > I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> > > helps with debug reports. I suppose an additional binary could be
> > > made for those looking for the fastest possible speed. (The sole cost
> > > of the debugging is the additional hardware accesses that results from
> > > those debug messages.)
> >
> > The qemu debugcon (CONFIG_DEBUG_IO) is detecable at runtime, it returns
> > 0xe9 on port reads. So we should be able to skip that too. IIRC it
> > isn't *that* straightforward as seabios is initially mapped read/only so
> > a simple probe-on-first-putchar, then cache the result in a variable
> > doesn't work. We could probe after make_bios_writable though which
> > should still avoid printing most of the messages.
>
> Great! I just tried the patch below and it works as you said. If I
> don't have debugcon in QEMU the outs are avoided and the speed is not
> bad (11.4 ms).
> If I have debugcon (iobase=0x402), I can see the output.
>
> Do you think can be an acceptable trade-off?
Makes sense to me.
> index 0770c47..31c080e 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -121,6 +121,10 @@ qemu_preinit(void)
> kvm_detect();
> }
>
> + // Detect qemu debugcon
> + if (inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
> + DebugOutputPort = 0;
As a minor quibble, this needs to be "if (CONFIG_DEBUG_IO && ...)" to
ensure it gets compiled out if debug io is not in use. Also, I think
it would be preferable to introduce a serial_debug_postram_preinit()
method in src/hw/serialio.c that does this check.
> --- a/src/hw/serialio.c
> +++ b/src/hw/serialio.c
> @@ -107,7 +107,7 @@ u16 DebugOutputPort VARFSEG = 0x402;
> void
> qemu_debug_putc(char c)
> {
> - if (CONFIG_DEBUG_IO && runningOnQEMU())
> + if (CONFIG_DEBUG_IO && runningOnQEMU() && GET_GLOBAL(DebugOutputPort))
> // Send character to debug port.
> outb(c, GET_GLOBAL(DebugOutputPort));
> }
As a minor nit, it would be preferable to invoke
GET_GLOBAL(DebugOutputPort) only once in the success case.
-Kevin
The virt machine in the NEMU project has the ambition to be a platform
that has no emulated legacy hardware. This patch series enables support
for that machine type in Seabios.
The impact that this has for a Seabios port is that the CMOS is not
available to query details of CPUs or memory configuration; instead this
patch series modifies the code that queries those details to prefer
those from QEMU FW CFG over CMOS.
I've tested this patch series with pc, Q35 and virt as part of our
automated testing for NEMU.
Rob
From: Arbel Moshe <arbel.moshe(a)oracle.com>
This reverts commit
3c3a3fa6522f ("mptable: Don't describe pci-to-pci bridges.”)
The reverted commit removed the description of non-root PCI busses
from the MPTable in claim they are not necessary.
However, it seems that some guests rely on this information in order to
correclty configure IOAPIC redirection-table entries for interrupts
originated from PCI devices on non-root PCI busses.
One such guest is "Extreme Networks OS".
We observed that if this guest is setup with an E1000 NIC behind a PCI
bridge, the OS wouldn't configure the IOAPIC redirection-table entry for
the IRQ that the E1000 PIN is connected to. Therefore, interrupts from
the NIC were not delivered to OS which caused guest to not have network
connectivity.
Fixes: 3c3a3fa6522f ("mptable: Don't describe pci-to-pci bridges.”)
Reviewed-by: Liran Alon <liran.alon(a)oracle.com>
Reviewed-by: Mark Kanda <mark.kanda(a)oracle.com>
Signed-off-by: Arbel Moshe <arbel.moshe(a)oracle.com>
---
src/fw/mptable.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/fw/mptable.c b/src/fw/mptable.c
index 47385cc5d32d..27686369e98c 100644
--- a/src/fw/mptable.c
+++ b/src/fw/mptable.c
@@ -72,25 +72,30 @@ mptable_setup(void)
}
int entrycount = cpu - cpus;
- // PCI bus
+ // PCI buses
struct mpt_bus *buses = (void*)cpu, *bus = buses;
- if (!hlist_empty(&PCIDevices)) {
+ int lastbus = -1;
+ struct pci_device *pci;
+ foreachpci(pci) {
+ int curbus = pci_bdf_to_bus(pci->bdf);
+ if (curbus == lastbus)
+ continue;
+ lastbus = curbus;
memset(bus, 0, sizeof(*bus));
bus->type = MPT_TYPE_BUS;
- bus->busid = 0;
+ bus->busid = curbus;
memcpy(bus->bustype, "PCI ", sizeof(bus->bustype));
bus++;
- entrycount++;
}
/* isa bus */
- int isabusid = bus - buses;
+ int isabusid;
memset(bus, 0, sizeof(*bus));
bus->type = MPT_TYPE_BUS;
- bus->busid = isabusid;
+ isabusid = bus->busid = lastbus + 1;
memcpy(bus->bustype, "ISA ", sizeof(bus->bustype));
bus++;
- entrycount++;
+ entrycount += bus - buses;
/* ioapic */
u8 ioapic_id = BUILD_IOAPIC_ID;
@@ -108,11 +113,8 @@ mptable_setup(void)
int dev = -1;
unsigned short pinmask = 0;
- struct pci_device *pci;
foreachpci(pci) {
u16 bdf = pci->bdf;
- if (pci_bdf_to_bus(bdf) != 0)
- break;
int pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
int irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE);
if (pin == 0)
--
2.16.1
From: Nikita Leshchenko <nikita.leshchenko(a)oracle.com>
When mpt-scsi receives a SCSI message, it wraps it in a MPT request
message and writes it's address to an IO port to be added to the
request queue.
This MPT request is allocated on the stack. Previous to this commit,
the request is aligned to 4 bytes. However, VirtualBox LSI53c1030
device emulation aligns the request address to 8 bytes.
Therefore, this commit change alignment of request to 8 bytes.
VirtualBox source code which handles this is at
Devices/Storage/DevLsiLogicSCSI.cpp. lsilogicRegisterWrite()
LSILOGIC_REG_REQUEST_QUEUE handler adds the request to the
queue (pRequestQueueBase). lsilogicR3Worker() reads request from
pRequestQueueBase and aligns it to 8 bytes
(u32RequestMessageFrameDesc & ~0x07).
Reviewed-by: Liran Alon <liran.alon(a)oracle.com>
Reviewed-by: Mark Kanda <mark.kanda(a)oracle.com>
Signed-off-by: Nikita Leshchenko <nikita.leshchenko(a)oracle.com>
---
src/hw/mpt-scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hw/mpt-scsi.c b/src/hw/mpt-scsi.c
index 1faede6aec6f..9966ca28054f 100644
--- a/src/hw/mpt-scsi.c
+++ b/src/hw/mpt-scsi.c
@@ -124,7 +124,7 @@ mpt_scsi_cmd(u32 iobase, struct disk_op_s *op,
struct scsi_req {
MptSCSIIORequest_t scsi_io;
MptSGEntrySimple32_t sge;
- } __attribute__((packed, aligned(4))) req = {
+ } __attribute__((packed, aligned(8))) req = {
.scsi_io = {
.TargetID = target,
.Bus = 0,
--
2.16.1
On Thu, Nov 29, 2018 at 03:16:39PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 28, 2018 at 3:38 PM Kevin O'Connor <kevin(a)koconnor.net> wrote:
> > It would be good to understand what the source of the delay is. It
> > might be possible to avoid the delay without adding a new config
> > setting. For example, the patch above will prevent the vga rom from
> > being run, but it should be possible to accomplish the same with
> > "-device VGA,romfile=". The other change this patch makes is to skip
> > device_hardware_setup() - is it known which items in that function add
> > to the delay?
>
> Hi Kevin,
> thanks for your advice.
>
> I tried to understand what are the sources of the delay, and I found
> that most of the time is spent because of QEMU default devices (eg
> e1000 ROM PCI, mouse, keyboard, etc)
> At this point, (without the patch above) I disabled the default QEMU
> devices (-nodefaults) and I reached 12.5 ms of SeaBIOS boot time
> (compared to 22 ms, where 4 ms is taken by the loading of e1000 ROM).
>
> Other 2 ms is taken by enable_vga_console(), where simply comment out
> the printf("SeaBIOS (version %s)\n", VERSION) I reached 10.7 ms of
> boot time.
> Do you think is acceptable to use dprintf(1, ...) instead of printf()?
> Or maybe check if we found the VGA.
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).
> Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
helps with debug reports. I suppose an additional binary could be
made for those looking for the fastest possible speed. (The sole cost
of the debugging is the additional hardware accesses that results from
those debug messages.)
> > Also, what is the target boot time that is considered acceptable?
>
> I'm using qboot as a reference (6/7 ms). It is very minimal, so 10 ms
> I think is considered acceptable for SeaBIOS
Thanks,
-Kevin
--- 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;
On Wed, Nov 28, 2018 at 12:30:42PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 28, 2018 at 3:12 AM Kevin O'Connor <kevin(a)koconnor.net> wrote:
> > On Mon, Nov 26, 2018 at 12:39:12PM +0100, Stefano Garzarella wrote:
> > > Speed up the boot phase when qemu uses "linuxboot" optionrom
> > > (qemu -kernel) and the boot-menu is not required.
> > > Under these conditions we can skip the setup of devices and VGA,
> > > because they will be initialized (if they are required) during
> > > the Linux boot phase.
> > >
> > > Following the time measured between SeaBIOS entry point and
> > > "linuxboot" entry point:
> > >
> > > * Before this patch
> > > qemu -kernel | qemu -vga none -kernel
> > > --------------+-----------------------
> > > 53.5 msec | 23.34 msec
> > >
> > > * After this patch
> > > qemu -kernel | qemu -vga none -kernel
> > > --------------+-----------------------
> > > 12.82 msec | 10.89 msec
> > >
> > > Note: For the measuring, we used the default configuration disabling
> > > debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
> > > "tpm: Check for TPM related ACPI tables before attempting hw"
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare(a)redhat.com>
> > > ---
> > > src/bootsplash.c | 3 +++
> > > src/fw/paravirt.c | 10 ++++++++++
> > > src/fw/paravirt.h | 4 ++++
> > > src/optionroms.c | 3 ++-
> > > src/post.c | 3 +++
> > > 5 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/bootsplash.c b/src/bootsplash.c
> > > index 165c98d..0eda7f2 100644
> > > --- a/src/bootsplash.c
> > > +++ b/src/bootsplash.c
> > > @@ -8,6 +8,7 @@
> > > #include "bregs.h" // struct bregs
> > > #include "config.h" // CONFIG_*
> > > #include "farptr.h" // FLATPTR_TO_SEG
> > > +#include "fw/paravirt.h" // runningOnQEMUFastBoot
> > > #include "malloc.h" // free
> > > #include "output.h" // dprintf
> > > #include "romfile.h" // romfile_loadfile
> > > @@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
> > > void
> > > enable_vga_console(void)
> > > {
> > > + if(runningOnQEMUFastBoot())
> > > + return;
> > > dprintf(1, "Turning on vga text mode console\n");
> > > struct bregs br;
> > >
> > > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > > index 0770c47..9e6e618 100644
> > > --- a/src/fw/paravirt.c
> > > +++ b/src/fw/paravirt.c
> > > @@ -621,4 +621,14 @@ void qemu_cfg_init(void)
> > > if (nogfx && !romfile_find("etc/sercon-port")
> > > && !romfile_find("vgaroms/sgabios.bin"))
> > > const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
> > > +
> > > + /*
> > > + * Enable QEMU fast boot if there is "linuxboot" optionrom and
> > > + * the boot menu is not required.
> > > + */
> > > + if ((romfile_find("genroms/linuxboot_dma.bin")
> > > + || romfile_find("genroms/linuxboot.bin"))
> > > + && !romfile_loadint("etc/show-boot-menu", 1)) {
> > > + PlatformRunningOn |= PF_QEMU_FB;
> > > + }
> >
> > I don't think we should hardcode special meanings to the names of
> > bootable files. If QEMU wants SeaBIOS to not perform some type of
> > hardware init, then I think QEMU should explicitly request that from
> > SeaBIOS (eg, a "etc/dont-run-hardware-init").
>
> I agree, it is cleaner. That was one of my doubt but, with an explicit
> request, it works only with a new version of QEMU.
> Do you think is acceptable?
It would be good to understand what the source of the delay is. It
might be possible to avoid the delay without adding a new config
setting. For example, the patch above will prevent the vga rom from
being run, but it should be possible to accomplish the same with
"-device VGA,romfile=". The other change this patch makes is to skip
device_hardware_setup() - is it known which items in that function add
to the delay?
Also, what is the target boot time that is considered acceptable?
Thanks,
-Kevin
