[RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM
As you may know we (the Xen project) are hoping to transition to SeaBIOS at the same time as we transition to the recently upstreamed qemu support for Xen. The following patches add very basic support for running SeaBIOS as the BIOS for a Xen HVM (fully virtualised) guest. These patches really do very little but they are sufficient to allow me to boot an HVM Linux guest to a prompt. SeaBIOS boots indirectly as an hvmloader payload with the first patch and directly if using the second (this second case requires a short patch series on the Xen side, see [0]). This is presented more as a proof of concept since it's not obvious yet which approach will be best from either Xen or SeaBIOS's point of view. I'm generally leaning towards the second option since much of the setup currently done in hvmloader appears to be more down to the limitations of ROMBIOS than any particular design decision. For example SeaBIOS is perfectly happy loading option ROMS from the appropriate BAR so that aspect of hvmloader goes away entirely, similarly with building ACPI, SMBIOS, MP tables etc. I'd be glad to hear any opinions from either side on the matter. One problem I have with the first patch is that I'm lacking the vocabulary to describe the configuration which is currently represented by COREBOOT=n. I wanted to switch the coreboot option to a Kconfig choice (so I can add XEN as another option) so I needed a name for the other option. I went with GENERIC but I've no idea if that is accurate. Is "EMULATOR" more accurate? Suggestions welcome. FWIW I expect (however naive it might be to make such predictions) the majority of the work to integrate SeaBIOS into Xen (other than test, test and more test with different OSes) will be to understand he differences in the various BIOS tables and figure out what is "just because", what represents real bug fixes on one side or the other, what is actually down to implementation details in Xen and what is down to differences between the old qemu-xen and upstream qemu. Having worked through all that I hope that actually finding a clean way to integrate the differences with SeaBIOS (or possibly qemu) will be comparatively trivial... Ian. [0] http://lists.xensource.com/archives/html/xen-devel/2011-05/msg00770.html
Do this by subsuming the existing COREBOOT boolean into a choice option of COREBOOT, XEN and GENERIC. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/Kconfig | 17 ++++++++++++++++- src/shadow.c | 4 ++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Kconfig b/src/Kconfig index 3133d88..fd7de41 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -4,12 +4,27 @@ mainmenu "SeaBIOS Configuration" menu "General Features" + choice + prompt "Select target environment" + default GENERIC + config COREBOOT bool "Build for coreboot" - default n help Configure as a coreboot payload. + config XEN + bool "Build for Xen HVM" + help + Configure to be used by xen hvmloader, for a HVM guest. + + config GENERIC + bool "Build for generic target environment" + help + Configure to be used in a generic target environment. + + endchoice + config THREADS bool "Parallelize hardware init" default y diff --git a/src/shadow.c b/src/shadow.c index ed530e0..a778701 100644 --- a/src/shadow.c +++ b/src/shadow.c @@ -102,7 +102,7 @@ static const struct pci_device_id dram_controller_make_writable_tbl[] = { void make_bios_writable(void) { - if (CONFIG_COREBOOT) + if (CONFIG_COREBOOT || CONFIG_XEN) return; dprintf(3, "enabling shadow ram\n"); @@ -127,7 +127,7 @@ static const struct pci_device_id dram_controller_make_readonly_tbl[] = { void make_bios_readonly(void) { - if (CONFIG_COREBOOT) + if (CONFIG_COREBOOT || CONFIG_XEN) return; dprintf(3, "locking shadow ram\n"); -- 1.7.2.5
Ian Campbell wrote:
One problem I have with the first patch is that I'm lacking the vocabulary to describe the configuration which is currently represented by COREBOOT=n. ..
+++ b/src/Kconfig @@ -4,12 +4,27 @@ mainmenu "SeaBIOS Configuration"
menu "General Features"
+ choice + prompt "Select target environment" + default GENERIC + config COREBOOT bool "Build for coreboot" - default n help Configure as a coreboot payload.
+ config XEN + bool "Build for Xen HVM" + help + Configure to be used by xen hvmloader, for a HVM guest. + + config GENERIC + bool "Build for generic target environment" + help + Configure to be used in a generic target environment. + + endchoice
Shouldn't GENERIC be QEMU? AFAIK that's the alternative to using SeaBIOS with coreboot? //Peter
This allows a guest to be booted using SeaBIOS directly as its firmware rather than being loaded indirectly via hvmloader. This is presented as a proof of concept since it's not obvious yet which the best approach will be from either Xen or SeaBIOS's point of view. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- Makefile | 2 +- src/post.c | 5 ++ src/xen.c | 93 ++++++++++++++++++++++++++++++++++++++++++ src/xen.h | 26 ++++++++++++ src/xen/hvm/hvm_info_table.h | 75 +++++++++++++++++++++++++++++++++ 5 files changed, 200 insertions(+), 1 deletions(-) create mode 100644 src/xen.c create mode 100644 src/xen.h create mode 100644 src/xen/hvm/hvm_info_table.h diff --git a/Makefile b/Makefile index 9affb39..6f12d5a 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ SRC16=$(SRCBOTH) system.c disk.c font.c 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 dev-i440fx.c \ - pci_region.c + pci_region.c xen.c SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c cc-option = $(shell if test -z "`$(1) $(2) -S -o /dev/null -xc \ diff --git a/src/post.c b/src/post.c index 7d2b5f2..a2bd2d7 100644 --- a/src/post.c +++ b/src/post.c @@ -23,6 +23,7 @@ #include "usb.h" // usb_setup #include "smbios.h" // smbios_init #include "paravirt.h" // qemu_cfg_port_probe +#include "xen.h" // xen_probe_hvm_info #include "ps2port.h" // ps2port_setup #include "virtio-blk.h" // virtio_blk_setup @@ -123,6 +124,9 @@ ram_probe(void) /* reserve 256KB BIOS area at the end of 4 GB */ add_e820(0xfffc0000, 256*1024, E820_RESERVED); + + if (xen_reserved_size) + add_e820(0x100000000ull-xen_reserved_size, xen_reserved_size, E820_RESERVED); } // Don't declare any memory between 0xa0000 and 0x100000 @@ -329,6 +333,7 @@ post(void) { // Detect ram and setup internal malloc. qemu_cfg_port_probe(); + xen_probe_hvm_info(); ram_probe(); malloc_setup(); diff --git a/src/xen.c b/src/xen.c new file mode 100644 index 0000000..29e7e68 --- /dev/null +++ b/src/xen.c @@ -0,0 +1,93 @@ +// Xen HVM support +// +// Copyright (C) 2011 Citrix Systems. +// +// This file may be distributed under the terms of the GNU LGPLv3 license. + +#include "config.h" +#include "xen.h" +#include "cmos.h" + +u32 xen_reserved_size = 0; +u32 xen_nr_vcpus = 0; + +struct hvm_info_table *xen_hvm_info; + +static void validate_hvm_info(struct hvm_info_table *t) +{ + u8 *ptr = (u8 *)t; + u8 sum = 0; + int i; + + if ( memcmp(t->signature, "HVM INFO", 8) ) + panic("Bad hvm info signature\n"); + + if ( t->length < sizeof(struct hvm_info_table) ) + panic("Bad hvm info length\n"); + + for ( i = 0; i < t->length; i++ ) + sum += ptr[i]; + + if ( sum != 0 ) + panic("Bad hvm info checksum\n"); +} + +/* Replace possibly erroneous memory-size CMOS fields with correct values. */ +static void cmos_write_memory_size(u32 low_mem_pgend, u32 high_mem_pgend) +{ + u32 base_mem = 640, ext_mem, alt_mem; + u64 high = high_mem_pgend ? ((u64)high_mem_pgend << 12) - 0x100000000ull : 0ull; + + alt_mem = ext_mem = low_mem_pgend << 12; + ext_mem = (ext_mem > 0x0100000) ? (ext_mem - 0x0100000) >> 10 : 0; + if ( ext_mem > 0xffff ) + ext_mem = 0xffff; + alt_mem = (alt_mem > 0x1000000) ? (alt_mem - 0x1000000) >> 16 : 0; + + /* All BIOSes: conventional memory (CMOS *always* reports 640kB). */ + outb_cmos((u8)(base_mem >> 0), 0x15); + outb_cmos((u8)(base_mem >> 8), 0x16); + + /* All BIOSes: extended memory (1kB chunks above 1MB). */ + outb_cmos((u8)( ext_mem >> 0), CMOS_MEM_EXTMEM_LOW); + outb_cmos((u8)( ext_mem >> 8), CMOS_MEM_EXTMEM_HIGH); + + /* Some BIOSes: alternative extended memory (64kB chunks above 16MB). */ + outb_cmos((u8)( alt_mem >> 0), CMOS_MEM_EXTMEM2_LOW); + outb_cmos((u8)( alt_mem >> 8), CMOS_MEM_EXTMEM2_HIGH); + + /* RAM above 4GB */ + outb_cmos((u8) ((high >> 16) & 0xff), CMOS_MEM_HIGHMEM_LOW); + outb_cmos((u8) ((high >> 24) & 0xff), CMOS_MEM_HIGHMEM_MID); + outb_cmos((u8) ((high >> 32) & 0xff), CMOS_MEM_HIGHMEM_HIGH); +} + +void xen_probe_hvm_info(void) +{ + struct hvm_info_table *t; + + t = (struct hvm_info_table *)HVM_INFO_PADDR; + + validate_hvm_info(t); + + xen_hvm_info = t; + + dprintf(1, "Found HVM info table at %p\n", t); + dprintf(1, "Signature %c%c%c%c%c%c%c%c\n", + t->signature[0], t->signature[1], + t->signature[2], t->signature[3], + t->signature[4], t->signature[5], + t->signature[6], t->signature[7]); + dprintf(1, "Length %d, checksum %x\n", t->length, t->checksum); + dprintf(1, "ACPI:%d APIC:%d VCPUS:%d\n", + t->acpi_enabled, t->apic_mode, t->nr_vcpus); + dprintf(1, "low_mem_pgend: %x\n", t->low_mem_pgend); + dprintf(1, "high_mem_pgend: %x\n", t->high_mem_pgend); + dprintf(1, "reserved_mem_pgstart: %x\n", t->reserved_mem_pgstart); + + cmos_write_memory_size(t->low_mem_pgend, t->high_mem_pgend); + + xen_reserved_size = (u32)(0x100000000ull - ((u64)t->reserved_mem_pgstart<<12)); + + xen_nr_vcpus = t->nr_vcpus; +} diff --git a/src/xen.h b/src/xen.h new file mode 100644 index 0000000..eef9a0e --- /dev/null +++ b/src/xen.h @@ -0,0 +1,26 @@ +#ifndef __XEN_H +#define __XEN_H + +#ifdef CONFIG_XEN + +#include "util.h" + +#include "xen/hvm/hvm_info_table.h" + +void xen_probe_hvm_info(void); + +extern u32 xen_reserved_size; +extern u32 xen_nr_vcpus; + +extern struct hvm_info_table *xen_hvm_info; + +#else + +static voic xen_probe_hvm_info(void) {} + +#define xen_reserved_size 0 +#define xen_nr_vcpus 0 + +#endif + +#endif diff --git a/src/xen/hvm/hvm_info_table.h b/src/xen/hvm/hvm_info_table.h new file mode 100644 index 0000000..e665883 --- /dev/null +++ b/src/xen/hvm/hvm_info_table.h @@ -0,0 +1,75 @@ +/****************************************************************************** + * hvm/hvm_info_table.h + * + * HVM parameter and information table, written into guest memory map. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#ifndef __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ +#define __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ + +#define HVM_INFO_PFN 0x09F +#define HVM_INFO_OFFSET 0x800 +#define HVM_INFO_PADDR ((HVM_INFO_PFN << 12) + HVM_INFO_OFFSET) + +/* Maximum we can support with current vLAPIC ID mapping. */ +#define HVM_MAX_VCPUS 128 + +struct hvm_info_table { + char signature[8]; /* "HVM INFO" */ + u32 length; + u8 checksum; + + /* Should firmware build ACPI tables? */ + u8 acpi_enabled; + + /* Should firmware build APIC descriptors (APIC MADT / MP BIOS)? */ + u8 apic_mode; + + /* How many CPUs does this domain have? */ + u32 nr_vcpus; + + /* + * MEMORY MAP provided by HVM domain builder. + * Notes: + * 1. page_to_phys(x) = x << 12 + * 2. If a field is zero, the corresponding range does not exist. + */ + /* + * 0x0 to page_to_phys(low_mem_pgend)-1: + * RAM below 4GB (except for VGA hole 0xA0000-0xBFFFF) + */ + u32 low_mem_pgend; + /* + * page_to_phys(reserved_mem_pgstart) to 0xFFFFFFFF: + * Reserved for special memory mappings + */ + u32 reserved_mem_pgstart; + /* + * 0x100000000 to page_to_phys(high_mem_pgend)-1: + * RAM above 4GB + */ + u32 high_mem_pgend; + + /* Bitmap of which CPUs are online at boot time. */ + u8 vcpu_online[(HVM_MAX_VCPUS + 7)/8]; +}; + +#endif /* __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ */ -- 1.7.2.5
On Fri, May 13, 2011 at 04:59:24PM +0100, Ian Campbell wrote:
This allows a guest to be booted using SeaBIOS directly as its firmware rather than being loaded indirectly via hvmloader. This is presented as a proof of concept since it's not obvious yet which the best approach will be from either Xen or SeaBIOS's point of view.
Thanks. See my comments below. [...]
--- /dev/null +++ b/src/xen.c @@ -0,0 +1,93 @@ [...] +/* Replace possibly erroneous memory-size CMOS fields with correct values. */ +static void cmos_write_memory_size(u32 low_mem_pgend, u32 high_mem_pgend) +{ [...] + /* All BIOSes: conventional memory (CMOS *always* reports 640kB). */ + outb_cmos((u8)(base_mem >> 0), 0x15); + outb_cmos((u8)(base_mem >> 8), 0x16); + + /* All BIOSes: extended memory (1kB chunks above 1MB). */ + outb_cmos((u8)( ext_mem >> 0), CMOS_MEM_EXTMEM_LOW); + outb_cmos((u8)( ext_mem >> 8), CMOS_MEM_EXTMEM_HIGH); [...]
These cmos variables (all of them) are just a communication mechanism between Bochs and rombios - there's no need to set them. With SeaBIOS, just set the global variables RamSize and RamSizeOver4G and make sure the appropriate add_e820 calls are made. [...]
--- /dev/null +++ b/src/xen.h @@ -0,0 +1,26 @@ +#ifndef __XEN_H +#define __XEN_H + +#ifdef CONFIG_XEN
Just a style note - I'd prefer to avoid #ifdefs in the code. Just declare the variables/code unconditionally and add an "if (!CONFIG_XEN) return;" to the top of any externally called functions. SeaBIOS uses gcc's -fwhole-program and linker scripts to weed out unneeded code and variables. [...]
--- /dev/null +++ b/src/xen/hvm/hvm_info_table.h @@ -0,0 +1,75 @@ +/****************************************************************************** + * hvm/hvm_info_table.h + * + * HVM parameter and information table, written into guest memory map. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to [...]
That's a big copyright statement for one little struct. -Kevin
On Sat, 2011-05-14 at 15:02 +0100, Kevin O'Connor wrote:
On Fri, May 13, 2011 at 04:59:24PM +0100, Ian Campbell wrote:
This allows a guest to be booted using SeaBIOS directly as its firmware rather than being loaded indirectly via hvmloader. This is presented as a proof of concept since it's not obvious yet which the best approach will be from either Xen or SeaBIOS's point of view.
Thanks. See my comments below.
[...]
--- /dev/null +++ b/src/xen.c @@ -0,0 +1,93 @@ [...] +/* Replace possibly erroneous memory-size CMOS fields with correct values. */ +static void cmos_write_memory_size(u32 low_mem_pgend, u32 high_mem_pgend) +{ [...] + /* All BIOSes: conventional memory (CMOS *always* reports 640kB). */ + outb_cmos((u8)(base_mem >> 0), 0x15); + outb_cmos((u8)(base_mem >> 8), 0x16); + + /* All BIOSes: extended memory (1kB chunks above 1MB). */ + outb_cmos((u8)( ext_mem >> 0), CMOS_MEM_EXTMEM_LOW); + outb_cmos((u8)( ext_mem >> 8), CMOS_MEM_EXTMEM_HIGH); [...]
These cmos variables (all of them) are just a communication mechanism between Bochs and rombios - there's no need to set them.
I wasn't sure if these CMOS addresses were well defined enough that DOS applications etc might look at them. Sounds like you are saying they are not so I'll remove this bit.
With SeaBIOS, just set the global variables RamSize and RamSizeOver4G and make sure the appropriate add_e820 calls are made.
So would it be best to add a new case to ram_probe()?
[...]
--- /dev/null +++ b/src/xen.h @@ -0,0 +1,26 @@ +#ifndef __XEN_H +#define __XEN_H + +#ifdef CONFIG_XEN
Just a style note - I'd prefer to avoid #ifdefs in the code. Just declare the variables/code unconditionally and add an "if (!CONFIG_XEN) return;" to the top of any externally called functions. SeaBIOS uses gcc's -fwhole-program and linker scripts to weed out unneeded code and variables.
OK. I'll do that.
[...]
--- /dev/null +++ b/src/xen/hvm/hvm_info_table.h @@ -0,0 +1,75 @@ +/****************************************************************************** + * hvm/hvm_info_table.h + * + * HVM parameter and information table, written into guest memory map. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to [...]
That's a big copyright statement for one little struct.
This file was taken verbatim (except s/uint([0-9]*_t/u\1/g) from the public headers provided by Xen, which have this permissive license to allow them to be incorporated into other projects as necessary. I think (although I should check first) that as long as we only ever expect changes to flow from the Xen headers into SeaBIOS it would be permissible under the terms of this license to subsume this file under SeaBIOS's GPLv3 license, if you would prefer to do it that way. There's likely to be a smallish number of other interface headers required in this way (e.g. xenstore and pv block interfaces for pv driver support, a small number of hypercall interfaces etc). Is the general approach of importing these headers as required ok or shall I look into a mechanism for supplying SeaBIOS with an out-of-tree set of headers to use? Ian.
On Fri, May 13, 2011 at 04:59:03PM +0100, Ian Campbell wrote:
As you may know we (the Xen project) are hoping to transition to SeaBIOS at the same time as we transition to the recently upstreamed qemu support for Xen.
The following patches add very basic support for running SeaBIOS as the BIOS for a Xen HVM (fully virtualised) guest.
Just so I understand, is this needed for Xen pre-qemu integration, post-qemu integration, or both?
These patches really do very little but they are sufficient to allow me to boot an HVM Linux guest to a prompt. SeaBIOS boots indirectly as an hvmloader payload with the first patch and directly if using the second (this second case requires a short patch series on the Xen side, see [0]).
So, patch 1 in this thread is needed for indirect, and patch 1+2 in this thread is needed for direct?
This is presented more as a proof of concept since it's not obvious yet which approach will be best from either Xen or SeaBIOS's point of view.
I'm generally leaning towards the second option since much of the setup currently done in hvmloader appears to be more down to the limitations of ROMBIOS than any particular design decision. For example SeaBIOS is perfectly happy loading option ROMS from the appropriate BAR so that aspect of hvmloader goes away entirely, similarly with building ACPI, SMBIOS, MP tables etc. I'd be glad to hear any opinions from either side on the matter.
I'm okay with the second option. This is reminiscent of the seabios+coreboot integration - there used to be an assembler loader (called ADLO) that would do all sorts of weird things to get rombios to load. One of my goals with SeaBIOS was to make that unnecessary.
One problem I have with the first patch is that I'm lacking the vocabulary to describe the configuration which is currently represented by COREBOOT=n. I wanted to switch the coreboot option to a Kconfig choice (so I can add XEN as another option) so I needed a name for the other option. I went with GENERIC but I've no idea if that is accurate. Is "EMULATOR" more accurate? Suggestions welcome.
The preferred approach would be to autodetect Xen at runtime and use that to control the code flow. A CONFIG_XEN option would then only be used to reduce the overall code size for those that do not need Xen support and want a smaller binary. Admittedly, CONFIG_COREBOOT doesn't do this today, but that's only because I haven't gotten around to fixing it. (Ideally, there'd be something like a CONFIG_EMULATORS (for qemu/kvm/bochs) and a CONFIG_COREBOOT, and everything would be auto-detected if both were enabled at the same time.
FWIW I expect (however naive it might be to make such predictions) the majority of the work to integrate SeaBIOS into Xen (other than test, test and more test with different OSes) will be to understand he differences in the various BIOS tables and figure out what is "just because", what represents real bug fixes on one side or the other, what is actually down to implementation details in Xen and what is down to differences between the old qemu-xen and upstream qemu. Having worked through all that I hope that actually finding a clean way to integrate the differences with SeaBIOS (or possibly qemu) will be comparatively trivial...
Sounds good. -Kevin
On Sat, 2011-05-14 at 14:36 +0100, Kevin O'Connor wrote:
On Fri, May 13, 2011 at 04:59:03PM +0100, Ian Campbell wrote:
As you may know we (the Xen project) are hoping to transition to SeaBIOS at the same time as we transition to the recently upstreamed qemu support for Xen.
The following patches add very basic support for running SeaBIOS as the BIOS for a Xen HVM (fully virtualised) guest.
Just so I understand, is this needed for Xen pre-qemu integration, post-qemu integration, or both?
We'd like to have all the pieces in place for the Xen 4.2 release, which isn't really planned out yet, but I think the late end of this year would be a reasonable bet. I think we can be flexible around the order integration happens in, we simply won't flip the default until everything is in place. Is that what you meant by post/pre-qemu integration?
These patches really do very little but they are sufficient to allow me
to boot an HVM Linux guest to a prompt. SeaBIOS boots indirectly as an hvmloader payload with the first patch and directly if using the second (this second case requires a short patch series on the Xen side, see [0]).
So, patch 1 in this thread is needed for indirect, and patch 1+2 in this thread is needed for direct?
Correct.
This is presented more as a proof of concept since it's not obvious yet which approach will be best from either Xen or SeaBIOS's point of view.
I'm generally leaning towards the second option since much of the setup currently done in hvmloader appears to be more down to the limitations of ROMBIOS than any particular design decision. For example SeaBIOS is perfectly happy loading option ROMS from the appropriate BAR so that aspect of hvmloader goes away entirely, similarly with building ACPI, SMBIOS, MP tables etc. I'd be glad to hear any opinions from either side on the matter.
I'm okay with the second option. This is reminiscent of the seabios+coreboot integration - there used to be an assembler loader (called ADLO) that would do all sorts of weird things to get rombios to load. One of my goals with SeaBIOS was to make that unnecessary.
One problem I have with the first patch is that I'm lacking the vocabulary to describe the configuration which is currently represented by COREBOOT=n. I wanted to switch the coreboot option to a Kconfig choice (so I can add XEN as another option) so I needed a name for the other option. I went with GENERIC but I've no idea if that is accurate. Is "EMULATOR" more accurate? Suggestions welcome.
The preferred approach would be to autodetect Xen at runtime and use that to control the code flow. A CONFIG_XEN option would then only be used to reduce the overall code size for those that do not need Xen support and want a smaller binary.
Sure. I'll change things to do it that way then. As well as CONFIG_XEN should I be adding a xen_present variable which is set dynamically and used as appropriate? If so then is a #ifdef to define it to 0 in the !CONFIG_XEN case (to allow the dead code to be eliminated) necessary? Or is gcc's whole program optimisation smart enough to spot when a global variable is never set != 0 and eliminate code as necessary?
Admittedly, CONFIG_COREBOOT doesn't do this today, but that's only because I haven't gotten around to fixing it. (Ideally, there'd be something like a CONFIG_EMULATORS (for qemu/kvm/bochs) and a CONFIG_COREBOOT, and everything would be auto-detected if both were enabled at the same time.
FWIW I expect (however naive it might be to make such predictions) the majority of the work to integrate SeaBIOS into Xen (other than test, test and more test with different OSes) will be to understand he differences in the various BIOS tables and figure out what is "just because", what represents real bug fixes on one side or the other, what is actually down to implementation details in Xen and what is down to differences between the old qemu-xen and upstream qemu. Having worked through all that I hope that actually finding a clean way to integrate the differences with SeaBIOS (or possibly qemu) will be comparatively trivial...
Sounds good.
-Kevin
On Mon, May 16, 2011 at 09:44:28AM +0100, Ian Campbell wrote:
On Sat, 2011-05-14 at 14:36 +0100, Kevin O'Connor wrote:
On Fri, May 13, 2011 at 04:59:03PM +0100, Ian Campbell wrote:
As you may know we (the Xen project) are hoping to transition to SeaBIOS at the same time as we transition to the recently upstreamed qemu support for Xen. [...] Just so I understand, is this needed for Xen pre-qemu integration, post-qemu integration, or both?
We'd like to have all the pieces in place for the Xen 4.2 release, which isn't really planned out yet, but I think the late end of this year would be a reasonable bet. I think we can be flexible around the order integration happens in, we simply won't flip the default until everything is in place.
Is that what you meant by post/pre-qemu integration?
I was referring to your statement above about "transition to SeaBIOS at the same time as we transition to the recently upstreamed qemu support for Xen". I wasn't sure if your patches are tied to that "upstreamed qemu" work or not.
One problem I have with the first patch is that I'm lacking the vocabulary to describe the configuration which is currently represented by COREBOOT=n. I wanted to switch the coreboot option to a Kconfig choice (so I can add XEN as another option) so I needed a name for the other option. I went with GENERIC but I've no idea if that is accurate. Is "EMULATOR" more accurate? Suggestions welcome.
The preferred approach would be to autodetect Xen at runtime and use that to control the code flow. A CONFIG_XEN option would then only be used to reduce the overall code size for those that do not need Xen support and want a smaller binary.
Sure. I'll change things to do it that way then.
As well as CONFIG_XEN should I be adding a xen_present variable which is set dynamically and used as appropriate? If so then is a #ifdef to define it to 0 in the !CONFIG_XEN case (to allow the dead code to be eliminated) necessary?
Thanks. I'd suggest something like a "int usingXen(void)" inline which reads "xen_present". The first couple of lines of usingXen() could do a "if (!CONFIG_XEN) return 0". Though - if the code is as simple as the patches you've already sent - then you can leave it on permanently for emulators (ie, do "if (CONFIG_COREBOOT)") - I'm not worried about a few extra bytes for Xen in the emulator case.
Or is gcc's whole program optimisation smart enough to spot when a global variable is never set != 0 and eliminate code as necessary?
I don't think it's that smart. -Kevin
On Mon, 2011-05-16 at 19:39 -0400, Kevin O'Connor wrote:
On Mon, May 16, 2011 at 09:44:28AM +0100, Ian Campbell wrote:
Sure. I'll change things to do it that way then.
As well as CONFIG_XEN should I be adding a xen_present variable which is set dynamically and used as appropriate? If so then is a #ifdef to define it to 0 in the !CONFIG_XEN case (to allow the dead code to be eliminated) necessary?
Thanks.
I'd suggest something like a "int usingXen(void)" inline which reads "xen_present". The first couple of lines of usingXen() could do a "if (!CONFIG_XEN) return 0".
I've done this, although rather than check a xen_present variable I just check whether or not the xen_hvm_info ptr is NULL. I also needed to move the call to xen_probe_hvm_info sooner.
Though - if the code is as simple as the patches you've already sent - then you can leave it on permanently for emulators (ie, do "if (CONFIG_COREBOOT)") - I'm not worried about a few extra bytes for Xen in the emulator case.
I think once we get a xenbus client and some PV drivers etc it will be an amount worth saving in the !Xen case. The following implements all your feedback (I hope). I have squashed it down into a single commit which supports direct boot. Ian.
From 25fdf95c608c73a00a72f0aa1bc72cbd9c03d4ce Mon Sep 17 00:00:00 2001 From: Ian Campbell <ian.campbell@citrix.com> Date: Tue, 17 May 2011 16:43:34 +0100 Subject: [PATCH] Kconfig: Add basic support for booting directly as Xen HVM firmware
This allows a guest to be booted using SeaBIOS directly as its firmware rather than being loaded indirectly via hvmloader. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- Makefile | 2 +- src/Kconfig | 6 +++ src/post.c | 19 ++++++++++ src/shadow.c | 5 ++- src/xen.c | 57 ++++++++++++++++++++++++++++++++ src/xen.h | 18 ++++++++++ src/xen/hvm/hvm_info_table.h | 75 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 src/xen.c create mode 100644 src/xen.h create mode 100644 src/xen/hvm/hvm_info_table.h diff --git a/Makefile b/Makefile index 9affb39..6f12d5a 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ SRC16=$(SRCBOTH) system.c disk.c font.c 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 dev-i440fx.c \ - pci_region.c + pci_region.c xen.c SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c cc-option = $(shell if test -z "`$(1) $(2) -S -o /dev/null -xc \ diff --git a/src/Kconfig b/src/Kconfig index 3133d88..69b63b3 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -10,6 +10,12 @@ menu "General Features" help Configure as a coreboot payload. + config XEN + bool "Build for Xen HVM" + default n + help + Configure to be used by xen hvmloader, for a HVM guest. + config THREADS bool "Parallelize hardware init" default y diff --git a/src/post.c b/src/post.c index 7d2b5f2..6703f5e 100644 --- a/src/post.c +++ b/src/post.c @@ -23,6 +23,7 @@ #include "usb.h" // usb_setup #include "smbios.h" // smbios_init #include "paravirt.h" // qemu_cfg_port_probe +#include "xen.h" // xen_probe_hvm_info #include "ps2port.h" // ps2port_setup #include "virtio-blk.h" // virtio_blk_setup @@ -101,6 +102,22 @@ ram_probe(void) dprintf(3, "Find memory size\n"); if (CONFIG_COREBOOT) { coreboot_setup(); + } else if (usingXen()) { + RamSize = xen_hvm_info->low_mem_pgend << 12; + + add_e820(0, RamSize, E820_RAM); + if (xen_hvm_info->high_mem_pgend) { + RamSizeOver4G = ((u64)xen_hvm_info->high_mem_pgend - 0x100000ull) << 12ull; + add_e820(0x100000000ull, RamSizeOver4G, E820_RAM); + } else { + RamSizeOver4G = 0; + } + + if (xen_hvm_info->reserved_mem_pgstart) { + u32 reserved_base = xen_hvm_info->reserved_mem_pgstart<<12; + u64 reserved_size = 0x100000000ull - reserved_base; + add_e820(reserved_base, reserved_size, E820_RESERVED); + } } else { // On emulators, get memory size from nvram. u32 rs = ((inb_cmos(CMOS_MEM_EXTMEM2_LOW) << 16) @@ -380,6 +397,8 @@ _start(void) // This is a soft reboot - invoke a hard reboot. tryReboot(); + xen_probe_hvm_info(); + // Allow writes to modify bios area (0xf0000) make_bios_writable(); HaveRunPost = 1; diff --git a/src/shadow.c b/src/shadow.c index ed530e0..cb39ddf 100644 --- a/src/shadow.c +++ b/src/shadow.c @@ -10,6 +10,7 @@ #include "config.h" // CONFIG_* #include "pci_ids.h" // PCI_VENDOR_ID_INTEL #include "dev-i440fx.h" +#include "xen.h" // usingXen // On the emulators, the bios at 0xf0000 is also at 0xffff0000 #define BIOS_SRC_OFFSET 0xfff00000 @@ -102,7 +103,7 @@ static const struct pci_device_id dram_controller_make_writable_tbl[] = { void make_bios_writable(void) { - if (CONFIG_COREBOOT) + if (CONFIG_COREBOOT || usingXen()) return; dprintf(3, "enabling shadow ram\n"); @@ -127,7 +128,7 @@ static const struct pci_device_id dram_controller_make_readonly_tbl[] = { void make_bios_readonly(void) { - if (CONFIG_COREBOOT) + if (CONFIG_COREBOOT || usingXen()) return; dprintf(3, "locking shadow ram\n"); diff --git a/src/xen.c b/src/xen.c new file mode 100644 index 0000000..3b3b79a --- /dev/null +++ b/src/xen.c @@ -0,0 +1,57 @@ +// Xen HVM support +// +// Copyright (C) 2011 Citrix Systems. +// +// This file may be distributed under the terms of the GNU LGPLv3 license. + +#include "config.h" +#include "xen.h" +#include "cmos.h" + +struct hvm_info_table *xen_hvm_info = NULL; + +static int validate_hvm_info(struct hvm_info_table *t) +{ + if ( memcmp(t->signature, "HVM INFO", 8) ) + return 1; + + if ( t->length < sizeof(struct hvm_info_table) ) + return 1; + + if (checksum(t, t->length) != 0) + return 1; + + return 0; +} + +void xen_probe_hvm_info(void) +{ + struct hvm_info_table *t; + + if (!CONFIG_XEN) + return; + + dprintf(1, "Probing for Xen HVM info\n"); + + t = (struct hvm_info_table *)HVM_INFO_PADDR; + + if (validate_hvm_info(t)) { + dprintf(1, "Xen HVM info not found\n"); + return; + } + + xen_hvm_info = t; + + dprintf(1, "Found HVM info table at %p\n", t); + dprintf(1, "Signature %c%c%c%c%c%c%c%c\n", + t->signature[0], t->signature[1], + t->signature[2], t->signature[3], + t->signature[4], t->signature[5], + t->signature[6], t->signature[7]); + dprintf(1, "Length %d, checksum %x\n", t->length, t->checksum); + dprintf(1, "ACPI:%d APIC:%d VCPUS:%d\n", + t->acpi_enabled, t->apic_mode, t->nr_vcpus); + dprintf(1, "low_mem_pgend: %x\n", t->low_mem_pgend); + dprintf(1, "high_mem_pgend: %x\n", t->high_mem_pgend); + dprintf(1, "reserved_mem_pgstart: %x\n", t->reserved_mem_pgstart); +} diff --git a/src/xen.h b/src/xen.h new file mode 100644 index 0000000..723e0c5 --- /dev/null +++ b/src/xen.h @@ -0,0 +1,18 @@ +#ifndef __XEN_H +#define __XEN_H + +#include "util.h" + +#include "xen/hvm/hvm_info_table.h" + +void xen_probe_hvm_info(void); + +extern struct hvm_info_table *xen_hvm_info; + +static inline int usingXen(void) { + if (!CONFIG_XEN) + return 0; + return (xen_hvm_info != NULL); +} + +#endif diff --git a/src/xen/hvm/hvm_info_table.h b/src/xen/hvm/hvm_info_table.h new file mode 100644 index 0000000..e665883 --- /dev/null +++ b/src/xen/hvm/hvm_info_table.h @@ -0,0 +1,75 @@ +/****************************************************************************** + * hvm/hvm_info_table.h + * + * HVM parameter and information table, written into guest memory map. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#ifndef __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ +#define __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ + +#define HVM_INFO_PFN 0x09F +#define HVM_INFO_OFFSET 0x800 +#define HVM_INFO_PADDR ((HVM_INFO_PFN << 12) + HVM_INFO_OFFSET) + +/* Maximum we can support with current vLAPIC ID mapping. */ +#define HVM_MAX_VCPUS 128 + +struct hvm_info_table { + char signature[8]; /* "HVM INFO" */ + u32 length; + u8 checksum; + + /* Should firmware build ACPI tables? */ + u8 acpi_enabled; + + /* Should firmware build APIC descriptors (APIC MADT / MP BIOS)? */ + u8 apic_mode; + + /* How many CPUs does this domain have? */ + u32 nr_vcpus; + + /* + * MEMORY MAP provided by HVM domain builder. + * Notes: + * 1. page_to_phys(x) = x << 12 + * 2. If a field is zero, the corresponding range does not exist. + */ + /* + * 0x0 to page_to_phys(low_mem_pgend)-1: + * RAM below 4GB (except for VGA hole 0xA0000-0xBFFFF) + */ + u32 low_mem_pgend; + /* + * page_to_phys(reserved_mem_pgstart) to 0xFFFFFFFF: + * Reserved for special memory mappings + */ + u32 reserved_mem_pgstart; + /* + * 0x100000000 to page_to_phys(high_mem_pgend)-1: + * RAM above 4GB + */ + u32 high_mem_pgend; + + /* Bitmap of which CPUs are online at boot time. */ + u8 vcpu_online[(HVM_MAX_VCPUS + 7)/8]; +}; + +#endif /* __XEN_PUBLIC_HVM_HVM_INFO_TABLE_H__ */ -- 1.7.2.5
On Tue, 2011-05-17 at 16:59 +0100, Ian Campbell wrote:
The following implements all your feedback (I hope). I have squashed it down into a single commit which supports direct boot.
I've gotten all the PCI setup and ACPI stuff etc etc working but, frankly, the patch to SeaBIOS is getting to be pretty enormous and intrusive. The more I look at it the more I am coming to the conclusion that it would be better to have hvmloader setup all this platform level stuff and pass details onto SeaBIOS as necessary, following the model used with coreboot->SeaBIOS rather than the emulator's way of doing things. hvmloader basically already fulfils the same role for Xen HVM guests as coreboot does for physical hardware so I think that makes a certain amount of sense. I'm going to try reworking things in that direction and see how they look. Cheers, Ian.
On Mon, May 23, 2011 at 11:44:38AM +0100, Ian Campbell wrote:
On Tue, 2011-05-17 at 16:59 +0100, Ian Campbell wrote:
The following implements all your feedback (I hope). I have squashed it down into a single commit which supports direct boot.
BTW, in general, the above patch looks okay to me.
I've gotten all the PCI setup and ACPI stuff etc etc working but, frankly, the patch to SeaBIOS is getting to be pretty enormous and intrusive.
Is that due to incompleteness / innacuracies in the current SeaBIOS code, or due to requirements specific to Xen? If you have test code, I'd be curious to see a patch on the mailing list - it may help later to understand the use cases for SeaBIOS.
The more I look at it the more I am coming to the conclusion that it would be better to have hvmloader setup all this platform level stuff and pass details onto SeaBIOS as necessary, following the model used with coreboot->SeaBIOS rather than the emulator's way of doing things. hvmloader basically already fulfils the same role for Xen HVM guests as coreboot does for physical hardware so I think that makes a certain amount of sense.
I'm okay with that approach as well - there are pros and cons to each method. (At various points, it's been discussed whether SeaBIOS should generate ACPI tables for coreboot, and it has also been discussed if QEmu should just pass in ACPI tables to SeaBIOS..) -Kevin
On Tue, 2011-05-24 at 01:17 +0100, Kevin O'Connor wrote:
On Mon, May 23, 2011 at 11:44:38AM +0100, Ian Campbell wrote:
On Tue, 2011-05-17 at 16:59 +0100, Ian Campbell wrote:
The following implements all your feedback (I hope). I have squashed it down into a single commit which supports direct boot.
BTW, in general, the above patch looks okay to me.
I've gotten all the PCI setup and ACPI stuff etc etc working but, frankly, the patch to SeaBIOS is getting to be pretty enormous and intrusive.
Is that due to incompleteness / innacuracies in the current SeaBIOS code, or due to requirements specific to Xen?
A mixture of both but mainly requirements specific to Xen, I think. PCI setup is one of the main things, the interrupt routing in particular is different which has knock on effects on chipset setup (e.g. legacy PCI ISA IRQ routing) and the BIOS tables (e.g. ACPI _PRT entries). This strikes me as being highly "mainboard" specific, IOW the stuff I would expect to find in coreboot rather than SeaBIOS. Another difference is that the existing hvmloader tables are ACPI 2.0 while SeaBIOS only creates 1.0 tables, I'm not sure that strictly speaking counts as a incompleteness in SeaBIOS since it's not obvious that Xen actually needs/uses any of the functionality of 2.0. Also it might be something SeaBIOS would like to grow in any case.
If you have test code, I'd be curious to see a patch on the mailing list - it may help later to understand the use cases for SeaBIOS.
It's in a pretty embarrassing state at the moment, but OK. Please note that I was nowhere near suggesting this patch was in any way suitable to be applied or even near to becoming an RFC! I initially took a rather gung-ho approach in the interests of matching as closely as possible how hvmloader+rombios sets things up, rather than evaluating the actual useful meaning of the differences. Having made things match my intention was to go back and evaluate each change and attempt to get rid of it unless there was an extremely pressing reason not to, but I didn't reach that phase yet. So I think this is an upper bound (possibly by a large-ish order of magnitude) on the level of change. I hardcoded a few things here and there too (e.g. numbers of CPUs), in the interests of getting something working before going back and cleaning it up. The diff (vs. the patches I already posted) is attached, it's diffstat is: src/Kconfig | 34 + src/acpi-dsdt.dsl | 9482 +++++++++++++++++++++++++++++++++++++++++++++----- src/acpi-dsdt.hex | 9331 +++++++++++++++++++++++++++++++++++++++++++------- src/acpi.c | 152 +- src/acpi.h | 93 + src/config.h | 6 +- src/dev-i440fx.c | 16 +- src/pciinit.c | 6 +- src/pmm.c | 5 +- src/post.c | 5 + src/util.h | 5 + src/xen.c | 128 +- src/xen.h | 3 + src/xen/hvm/hvm_op.h | 245 ++ src/xen/hvm/params.h | 145 + src/xen/version.h | 94 + src/xen/xen.h | 739 ++++ src/xen_hypercall.h | 187 + 18 files changed, 18591 insertions(+), 2085 deletions(-) The DSDT change rather dominates, dropping it gives a more sensible: 11 files changed, 414 insertions(+), 39 deletions(-) I just ripped the DSDT out of hvmloader, I'm sure with analysis the changes could be reduced to something much more sane, although I have a feeling it would still end up as a subtly different variant of the table (_PRT and such).
The more I look at it the more I am coming to the conclusion that it would be better to have hvmloader setup all this platform level stuff and pass details onto SeaBIOS as necessary, following the model used with coreboot->SeaBIOS rather than the emulator's way of doing things. hvmloader basically already fulfils the same role for Xen HVM guests as coreboot does for physical hardware so I think that makes a certain amount of sense.
I'm okay with that approach as well
The SeaBIOS patch for this approach seems likely to be an awful lot smaller. Basically most "if (COREBOOT)" become effectively "if (COREBOOT||XEN)" + a Xen equivalent to coreboot_copy_biostable and coreboot_fill_map.
- there are pros and cons to each method.
Apart from avoiding bloating SeaBIOS with a load of Xen specific stuff (which would be duplicated in hvmloader and also needed by the tianocore firmware variant etc) I think the big thing pulling me towards this approach is the idea that the BIOS tables describe the hardware, and since the "hardware" in this case is defined by Xen the tables ought to be supplied by Xen. I don't know if that's a distinction which people who really know about things at this level actually make though.
(At various points, it's been discussed whether SeaBIOS should generate ACPI tables for coreboot,
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
and it has also been discussed if QEmu should just pass in ACPI tables to SeaBIOS..)
That sounds plausible to me, at least based on my current understanding of what I think is going on ;-) Ian.
Hi,
PCI setup is one of the main things, the interrupt routing in particular is different which has knock on effects on chipset setup (e.g. legacy PCI ISA IRQ routing) and the BIOS tables (e.g. ACPI _PRT entries). This strikes me as being highly "mainboard" specific, IOW the stuff I would expect to find in coreboot rather than SeaBIOS.
There is q35 emulation support in for qemu in the pipeline, which has simliar requirements, especially it needs a different dsdt table too. So a solution is needed here anyway. One option is to just pass the table from qemu, another one is to have multiple tables compiled in and have seabios pick one at runtime depending on the hardware it detects.
Another difference is that the existing hvmloader tables are ACPI 2.0 while SeaBIOS only creates 1.0 tables, I'm not sure that strictly speaking counts as a incompleteness in SeaBIOS since it's not obvious that Xen actually needs/uses any of the functionality of 2.0. Also it might be something SeaBIOS would like to grow in any case.
Indeed, I don't see that as a xen specific thing. cheers, Gerd
On Tue, May 24, 2011 at 12:02:07PM +0100, Ian Campbell wrote:
On Tue, 2011-05-24 at 01:17 +0100, Kevin O'Connor wrote:
If you have test code, I'd be curious to see a patch on the mailing list - it may help later to understand the use cases for SeaBIOS. The diff (vs. the patches I already posted) is attached, it's diffstat is:
Thanks. I understand it's just test code.
(At various points, it's been discussed whether SeaBIOS should generate ACPI tables for coreboot,
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
The idea - when it was last raised - was to provide raw info in a coreboot specific manor (via the "coreboot tables"), and then have SeaBIOS populate ACPI/SMBIOS/MPTable/etc. from that info. It was never pursued. -Kevin
On Tue, 2011-05-24 at 22:44 -0400, Kevin O'Connor wrote:
On Tue, May 24, 2011 at 12:02:07PM +0100, Ian Campbell wrote:
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
The idea - when it was last raised - was to provide raw info in a coreboot specific manor (via the "coreboot tables"), and then have SeaBIOS populate ACPI/SMBIOS/MPTable/etc. from that info. It was never pursued.
Speaking of coreboot tables... I also need to pass some start of day info into seabios (ACPI tables, e820 etc). Currently I just used a little ad-hoc data structure at a known physical address but I wonder if perhaps I should/could reuse the coreboot table datastructures? They are existing and well defined and I suppose they are pretty static, but I don't want to add any additional compatibility burden if you guys would rather avoid it. FWIW my current patch for hvmloader->seabios (rather than direct SeaBIOS boot) is attached. Things I'd like to address before submitting it properly are putting the hypercall pages somewhere saner (and reserved etc) and using coreboot tables if desired (or at the very least do something better than commenting out the "static"...). I also attached the patches for hvmloader. Ian.
On Thu, May 26, 2011 at 04:13:37PM +0100, Ian Campbell wrote:
On Tue, 2011-05-24 at 22:44 -0400, Kevin O'Connor wrote:
On Tue, May 24, 2011 at 12:02:07PM +0100, Ian Campbell wrote:
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
The idea - when it was last raised - was to provide raw info in a coreboot specific manor (via the "coreboot tables"), and then have SeaBIOS populate ACPI/SMBIOS/MPTable/etc. from that info. It was never pursued.
Speaking of coreboot tables... I also need to pass some start of day info into seabios (ACPI tables, e820 etc). Currently I just used a little ad-hoc data structure at a known physical address but I wonder if perhaps I should/could reuse the coreboot table datastructures? They are existing and well defined and I suppose they are pretty static, but I don't want to add any additional compatibility burden if you guys would rather avoid it.
Will Xen support the fw_cfg interface? If so, passing this info should be possible through that interface. Another thing to consider would be if coreboot+SeaBIOS in place of hvmloader would be a fit. (I don't have a good feel for what hvmloader does to judge that.) Using the coreboot tables in Xen seems a bit odd, but I can't say it would cause a problem. -Kevin
On Fri, 2011-05-27 at 02:20 +0100, Kevin O'Connor wrote:
On Thu, May 26, 2011 at 04:13:37PM +0100, Ian Campbell wrote:
On Tue, 2011-05-24 at 22:44 -0400, Kevin O'Connor wrote:
On Tue, May 24, 2011 at 12:02:07PM +0100, Ian Campbell wrote:
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
The idea - when it was last raised - was to provide raw info in a coreboot specific manor (via the "coreboot tables"), and then have SeaBIOS populate ACPI/SMBIOS/MPTable/etc. from that info. It was never pursued.
Speaking of coreboot tables... I also need to pass some start of day info into seabios (ACPI tables, e820 etc). Currently I just used a little ad-hoc data structure at a known physical address but I wonder if perhaps I should/could reuse the coreboot table datastructures? They are existing and well defined and I suppose they are pretty static, but I don't want to add any additional compatibility burden if you guys would rather avoid it.
Will Xen support the fw_cfg interface?
I don't think so, at least not in general. (fw_cfg is the qemu thing on ports 0x510/511, right?) I don't think the qemu instance in a Xen domain has all the info it would need in order to provide the tables.
Another thing to consider would be if coreboot+SeaBIOS in place of hvmloader would be a fit. (I don't have a good feel for what hvmloader does to judge that.)
I think hvmloader contains a subset of coreboot's functionality. I've wondered about possibly using coreboot in the future, but I think that would be a separate project.
Using the coreboot tables in Xen seems a bit odd, but I can't say it would cause a problem.
The existing ad-hoc structure I've defined is: struct xen_seabios_info { char signature[14]; /* XenHVMSeaBIOS\0 */ u16 length; u32 acpi_rsdp; u32 mptable; u32 e820_nr; struct e820entry e820[128]; u8 checksum; }; so I was mainly thinking of e.g. CB_TAG_MEMORY along with CB_MEM_TABLE. I think I'll stick with defining a structure myself, these things are all discoverable via signatures so we can always transition in the future. Ian.
On Fri, May 27, 2011 at 10:27:14AM +0100, Ian Campbell wrote:
On Fri, 2011-05-27 at 02:20 +0100, Kevin O'Connor wrote:
Will Xen support the fw_cfg interface? I don't think so, at least not in general. (fw_cfg is the qemu thing on ports 0x510/511, right?)
Yes - 0x510/0x511. [...]
The existing ad-hoc structure I've defined is: struct xen_seabios_info { char signature[14]; /* XenHVMSeaBIOS\0 */ u16 length; u32 acpi_rsdp; u32 mptable; u32 e820_nr; struct e820entry e820[128]; u8 checksum; }; so I was mainly thinking of e.g. CB_TAG_MEMORY along with CB_MEM_TABLE.
I think I'll stick with defining a structure myself, these things are all discoverable via signatures so we can always transition in the future.
At some point, Xen will likely need "romfile" support (see src/paravirt.h) in order to support pulling in non-device option roms and things like "bootorder" support. The "romfile" stuff just uses a list of ("name", size, data) 3-tuples to pass in arbitrary data to SeaBIOS. You may wish to consider supporting an interface like that from the start. -Kevin
On Mon, 2011-05-30 at 15:43 +0100, Kevin O'Connor wrote:
On Fri, May 27, 2011 at 10:27:14AM +0100, Ian Campbell wrote:
On Fri, 2011-05-27 at 02:20 +0100, Kevin O'Connor wrote:
Will Xen support the fw_cfg interface? I don't think so, at least not in general. (fw_cfg is the qemu thing on ports 0x510/511, right?)
Yes - 0x510/0x511.
[...]
The existing ad-hoc structure I've defined is: struct xen_seabios_info { char signature[14]; /* XenHVMSeaBIOS\0 */ u16 length; u32 acpi_rsdp; u32 mptable; u32 e820_nr; struct e820entry e820[128]; u8 checksum; }; so I was mainly thinking of e.g. CB_TAG_MEMORY along with CB_MEM_TABLE.
I think I'll stick with defining a structure myself, these things are all discoverable via signatures so we can always transition in the future.
At some point, Xen will likely need "romfile" support (see src/paravirt.h) in order to support pulling in non-device option roms
What sorts of things live in those?
and things like "bootorder" support. The "romfile" stuff just uses a list of ("name", size, data) 3-tuples to pass in arbitrary data to SeaBIOS. You may wish to consider supporting an interface like that from the start.
The datastructure I'm proposing above would only be used to propagate information hvmloader itself has created (BIOS tables) through to SeaBios. hvmloader doesn't have any insight into things like bootorder etc so it wouldn't be able to create tables of that type. The other stuff sounds like the sort of thing Xen would normally communicate from the toolstack to the guest using xenstore, unless the payloads are huge? The fw_cfg remains an option for this sort of data too, if that turns out to make sense. I'll be posting the next version of the patch shortly. Thanks, Ian.
participants (6)
-
Gerd Hoffmann -
Ian Campbell -
Ian Campbell -
Ian Campbell
-
Kevin O'Connor -
Peter Stuge