On Wed, Jul 07, 2010 at 12:14:02PM +0900, Isaku Yamahata wrote:
> This patch makes use of pci device initialization helper function
> to convert if/switch clause to table driven.
> So this makes it easier to add q35 initialization code.
>
> Signed-off-by: Isaku Yamahata <yamahata(a)valinux.co.jp>
I committed the first patch. I committed the second patch after
fixing up the compile on it.
You're likely seeing " Working around non-functional -combine" printed
during the build. When in this mode the build textually includes all
the .c files in order to still take advantage of the -fwhole-program
optimization. Unfortunately in this mode, improper includes can slip
through.
You might want to try grabbing a newer version of gcc. I believe both
the latest gcc v4.4 and gcc v4.5 work correctly.
-Kevin
On Thu, Jul 08, 2010 at 08:52:19PM +0800, Liu, Jinsong wrote:
> Kevin O'Connor wrote:
> > I've also noticed this patch increases the seabios build size by 18K.
> > This isn't great because it is 18K that then can't be used by option
> > roms, and it brings seabios very close to its current 128K limit (124K
> > when full debugging enabled).
[...]
> Can the limitation 128K be increased?
Now that qemu doesn't directly place option roms in the c/d segments,
it can probably be increased to 256K. I don't know if qemu changes
would be needed (it doesn't seem like it). Expanding beyond 256K
would be a bit of work.
The limit due to option roms is more restrictive. The rom space (at
least on FC13) for vgabios, e1000 gpxe, and vapic comes to 116.5KiB -
add another 8KiB for additional gpxe roms and we're basically back to
the 128KiB limit.
-Kevin
On Sun, Jun 27, 2010 at 01:28:52PM +0800, Liu, Jinsong wrote:
> > Attached is updated patch:
I've been looking at this patch.
I've had a hard time understanding how the SSDT code works. It looks
like the OS calls the \_GPE._L02 method, which reads ioport
0xaf00-0xaf20, determines which processors have changed state, updates
the madt tables, and then calls Notify(Cxx,).
Why is the madt table read/written?
I've also noticed this patch increases the seabios build size by 18K.
This isn't great because it is 18K that then can't be used by option
roms, and it brings seabios very close to its current 128K limit (124K
when full debugging enabled).
-Kevin
The VRING_AVAIL_F_NO_INTERRUPT flag is a hint that interrupts should be
suppressed. It does not guarantee that interrupts will not be raised.
Therefore, make sure to clear the interrupt after each virtio-blk read.
This avoids a stuck interrupt interfering with the OS loaded later in
the boot process.
Signed-off-by: Stefan Hajnoczi <stefanha(a)linux.vnet.ibm.com>
---
src/virtio-blk.c | 6 ++++++
src/virtio-pci.h | 4 ++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/virtio-blk.c b/src/virtio-blk.c
index 16d9ad8..74af488 100644
--- a/src/virtio-blk.c
+++ b/src/virtio-blk.c
@@ -62,6 +62,12 @@ virtio_blk_read(struct disk_op_s *op)
/* Reclaim virtqueue element */
vring_get_buf(vq, NULL);
+
+ /* Clear interrupt status register. Avoid leaving interrupts stuck if
+ * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised.
+ */
+ vp_get_isr(GET_GLOBAL(vdrive_g->ioaddr));
+
return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK;
}
diff --git a/src/virtio-pci.h b/src/virtio-pci.h
index 9da761d..d21d5a5 100644
--- a/src/virtio-pci.h
+++ b/src/virtio-pci.h
@@ -71,6 +71,10 @@ static inline void vp_set_status(unsigned int ioaddr, u8 status)
outb(status, ioaddr + VIRTIO_PCI_STATUS);
}
+static inline u8 vp_get_isr(unsigned int ioaddr)
+{
+ return inb(ioaddr + VIRTIO_PCI_ISR);
+}
static inline void vp_reset(unsigned int ioaddr)
{
--
1.7.1
Isaku Yamahata wrote:
> This patch makes use of pci device initialization helper function
> to convert if/switch clause to table driven.
> So this makes it easier to add q35 initialization code.
I wish that this support could go into coreboot, but I'm not
sure how that could be made more attractive.. Any ideas?
//Peter
On Mon, Jul 05, 2010 at 11:22:29AM +0900, Isaku Yamahata wrote:
> This patch makes use of pci device initialization helper function
> to convert if/switch clause to table driven.
> So this makes it easier to add q35 initialization code.
Looks okay to me. A couple of minor comments..
> SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \
> acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \
> - lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c
> + lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c i440fx.c
The "i440fx.c" is a bit cryptic for someone unfamiliar with the hardware.
Maybe "dev-i440fx.c" or something similar?
> +++ b/src/i440fx.h
> @@ -0,0 +1,17 @@
> +// initialization function which are specific to i440fx chipset
> +//
> +// Copyright (C) 2008 Kevin O'Connor <kevin(a)koconnor.net>
> +// Copyright (C) 2006 Fabrice Bellard
> +//
> +// Copyright (C) 2010 Isaku Yamahata <yamahata at valinux co jp>
> +// Split out from pciinit.c
> +//
> +// This file may be distributed under the terms of the GNU LGPLv3 license.
> +#ifndef __I440FX_H
> +#define __I440FX_H
> +
> +void piix_isa_bridge_init(u16 bdf, void *arg);
> +void piix_ide_init(u16 bdf, void *arg);
> +void piix4_pm_init(u16 bdf, void *arg);
> +
> +#endif // __I440FX_H
I would prefer not to add long copyright statements on function
prototypes. Lets copyright the code, not the headers.
-Kevin