[RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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

[PATCH 1/2] Kconfig: Add option to enable support for running as a Xen HVM guests BIOS

[Ian Campbell]

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

[PATCH 2/2] Basic support for booting directly as Xen HVM firmware.

[Ian Campbell]

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

Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [PATCH 2/2] Basic support for booting directly as Xen HVM firmware.

[Kevin O'Connor]

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

Re: [SeaBIOS] [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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

Re: [PATCH 2/2] Basic support for booting directly as Xen HVM firmware.

[Ian Campbell]

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.

Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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

Re: Re: [SeaBIOS] [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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.

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 5189 bytes --]

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.

[-- Attachment #2: xen-hack.patch.bz2 --]
[-- Type: application/x-bzip, Size: 90631 bytes --]

[-- Attachment #3: Type: text/plain, Size: 137 bytes --]

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Gerd Hoffmann]

   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

Re: Re: [SeaBIOS] [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [SeaBIOS] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

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.

[-- Attachment #2: 0001-Add-basic-support-for-use-as-Xen-HVM-BIOS.patch --]
[-- Type: text/x-patch, Size: 14436 bytes --]

>From c2f5909e835903f7678f77b0a1626b9a974ef3aa Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Thu, 26 May 2011 16:10:25 +0100
Subject: [PATCH] Add basic support for use as Xen HVM BIOS.

SeaBIOS is called by Xen's hvmloader which does the basic platform
setup and provides various BIOS tables. Therefore avoid re-doing that
setup and copy out the tables as necessary.

Establish the basic infrastructure to make hypercalls.

XXX TODO put hypercall page at a safe location.
XXX TODO consider re-using coreboot table datastructure instead of ad-hoc.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Makefile       |    2 +-
 src/Kconfig    |    6 ++
 src/coreboot.c |    4 +-
 src/mtrr.c     |    3 +-
 src/pciinit.c  |    5 +-
 src/post.c     |   11 +++
 src/shadow.c   |    5 +-
 src/xen.c      |  231 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/xen.h      |   56 ++++++++++++++
 9 files changed, 315 insertions(+), 8 deletions(-)
 create mode 100644 src/xen.c
 create mode 100644 src/xen.h

diff --git a/Makefile b/Makefile
index d17f85a..5a9a7a1 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 123db01..c2fe086 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/coreboot.c b/src/coreboot.c
index f627531..ac480ad 100644
--- a/src/coreboot.c
+++ b/src/coreboot.c
@@ -216,7 +216,7 @@ copy_pir(void *pos)
     PirOffset = (u32)newpos - BUILD_BIOS_ADDR;
 }
 
-static void
+/*static*/ void
 copy_mptable(void *pos)
 {
     struct mptable_floating_s *p = pos;
@@ -240,7 +240,7 @@ copy_mptable(void *pos)
     memcpy((void*)newpos + length, (void*)p->physaddr, mpclength);
 }
 
-static void
+/*static*/ void
 copy_acpi_rsdp(void *pos)
 {
     if (RsdpAddr)
diff --git a/src/mtrr.c b/src/mtrr.c
index 0502c18..0548043 100644
--- a/src/mtrr.c
+++ b/src/mtrr.c
@@ -6,6 +6,7 @@
 
 #include "util.h" // dprintf
 #include "biosvar.h" // GET_EBDA
+#include "xen.h" // usingXen
 
 #define MSR_MTRRcap                    0x000000fe
 #define MSR_MTRRfix64K_00000           0x00000250
@@ -32,7 +33,7 @@
 
 void mtrr_setup(void)
 {
-    if (!CONFIG_MTRR_INIT || CONFIG_COREBOOT)
+    if (!CONFIG_MTRR_INIT || CONFIG_COREBOOT || usingXen())
         return;
 
     u32 eax, ebx, ecx, edx, cpuid_features;
diff --git a/src/pciinit.c b/src/pciinit.c
index ee2e72d..0bd9b72 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -11,6 +11,7 @@
 #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
 #include "pci_regs.h" // PCI_COMMAND
 #include "dev-i440fx.h"
+#include "xen.h" // usingXen
 
 #define PCI_ROM_SLOT 6
 #define PCI_NUM_REGIONS 7
@@ -396,8 +397,8 @@ pci_bios_init_bus(void)
 void
 pci_setup(void)
 {
-    if (CONFIG_COREBOOT)
-        // Already done by coreboot.
+    if (CONFIG_COREBOOT || usingXen())
+        // Already done by coreboot or Xen.
         return;
 
     dprintf(3, "pci setup\n");
diff --git a/src/post.c b/src/post.c
index 7d2b5f2..07167c8 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,8 @@ ram_probe(void)
     dprintf(3, "Find memory size\n");
     if (CONFIG_COREBOOT) {
         coreboot_setup();
+    } else if (usingXen()) {
+	xen_setup();
     } else {
         // On emulators, get memory size from nvram.
         u32 rs = ((inb_cmos(CMOS_MEM_EXTMEM2_LOW) << 16)
@@ -158,6 +161,10 @@ init_bios_tables(void)
         coreboot_copy_biostable();
         return;
     }
+    if (usingXen()) {
+	xen_copy_biostable();
+	return;
+    }
 
     create_pirtable();
 
@@ -329,6 +336,7 @@ post(void)
 {
     // Detect ram and setup internal malloc.
     qemu_cfg_port_probe();
+    xen_probe_hvm_info();
     ram_probe();
     malloc_setup();
 
@@ -380,6 +388,9 @@ _start(void)
         // This is a soft reboot - invoke a hard reboot.
         tryReboot();
 
+    // Check if we are running under Xen.
+    xen_probe();
+
     // 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..0161b65
--- /dev/null
+++ b/src/xen.c
@@ -0,0 +1,231 @@
+// 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 "memmap.h" // add_e820
+
+#define INFO_PHYSICAL_ADDRESS 0x00001000
+#define HYPERCALL_PHYSICAL_ADDRESS    0x00080000 /* MADE UP */
+
+u32 xen_cpuid_base = 0;
+
+static void init_hypercalls(void)
+{
+    u32 eax, ebx, ecx, edx;
+    unsigned long i;
+
+    /* Fill in hypercall transfer pages. */
+    cpuid(xen_cpuid_base + 2, &eax, &ebx, &ecx, &edx);
+    for ( i = 0; i < eax; i++ )
+        wrmsr(ebx, HYPERCALL_PHYSICAL_ADDRESS + (i << 12) + i);
+}
+
+#define __STR(...) #__VA_ARGS__
+#define STR(...) __STR(__VA_ARGS__)
+
+/*
+ * NB. Hypercall address needs to be relative to a linkage symbol for
+ * some version of ld to relocate the relative calls properly.
+ */
+//#define hypercall_pa "_start - " STR(HVMLOADER_PHYSICAL_ADDRESS)	\
+//                           " + " STR(HYPERCALL_PHYSICAL_ADDRESS)
+#define hypercall_pa STR(HYPERCALL_PHYSICAL_ADDRESS)
+
+#define _hypercall0(type, name)						\
+({									\
+	long __res;							\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res)						\
+		:							\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+#define _hypercall1(type, name, a1)					\
+({									\
+	long __res, __ign1;						\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res), "=b" (__ign1)				\
+		: "1" ((long)(a1))					\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+#define _hypercall2(type, name, a1, a2)					\
+({									\
+	long __res, __ign1, __ign2;					\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res), "=b" (__ign1), "=c" (__ign2)		\
+		: "1" ((long)(a1)), "2" ((long)(a2))			\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+#define _hypercall3(type, name, a1, a2, a3)				\
+({									\
+	long __res, __ign1, __ign2, __ign3;				\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res), "=b" (__ign1), "=c" (__ign2),		\
+		"=d" (__ign3)						\
+		: "1" ((long)(a1)), "2" ((long)(a2)),			\
+		"3" ((long)(a3))					\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+#define _hypercall4(type, name, a1, a2, a3, a4)				\
+({									\
+	long __res, __ign1, __ign2, __ign3, __ign4;			\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res), "=b" (__ign1), "=c" (__ign2),		\
+		"=d" (__ign3), "=S" (__ign4)				\
+		: "1" ((long)(a1)), "2" ((long)(a2)),			\
+		"3" ((long)(a3)), "4" ((long)(a4))			\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+#define _hypercall5(type, name, a1, a2, a3, a4, a5)			\
+({									\
+	long __res, __ign1, __ign2, __ign3, __ign4, __ign5;		\
+	asm volatile (							\
+		"call "hypercall_pa" + " STR(__HYPERVISOR_##name * 32)	\
+		: "=a" (__res), "=b" (__ign1), "=c" (__ign2),		\
+		"=d" (__ign3), "=S" (__ign4), "=D" (__ign5)		\
+		: "1" ((long)(a1)), "2" ((long)(a2)),			\
+		"3" ((long)(a3)), "4" ((long)(a4)),			\
+		"5" ((long)(a5))					\
+		: "memory" );						\
+	(type)__res;							\
+})
+
+static inline int
+hypercall_xen_version(
+	int cmd, void *arg)
+{
+	return _hypercall2(int, xen_version, cmd, arg);
+}
+
+void xen_probe(void)
+{
+    u32 base, eax, ebx, ecx, edx;
+    char signature[13];
+    xen_extraversion_t extraversion;
+
+    if (!CONFIG_XEN)
+	return;
+
+    for (base = 0x40000000; base < 0x40010000; base += 0x100) {
+        cpuid(base, &eax, &ebx, &ecx, &edx);
+        memcpy(signature + 0, &ebx, 4);
+        memcpy(signature + 4, &ecx, 4);
+        memcpy(signature + 8, &edx, 4);
+        signature[12] = 0;
+
+        dprintf(1, "Found hypervisor signature \"%s\" at %x\n",
+                signature, base);
+        if (strcmp(signature, "XenVMMXenVMM") == 0) {
+            if ((eax - base) < 2)
+                panic("Insufficient Xen cpuid leaves. eax=%x at base %x\n",
+                      eax, base);
+            xen_cpuid_base = base;
+            break;
+        }
+    }
+
+    if (!xen_cpuid_base)
+	return;
+
+    init_hypercalls();
+
+    /* Print version information. */
+    cpuid(xen_cpuid_base + 1, &eax, &ebx, &ecx, &edx);
+    hypercall_xen_version(XENVER_extraversion, extraversion);
+    dprintf(1, "Detected Xen v%u.%u%s\n", eax >> 16, eax & 0xffff, extraversion);
+}
+
+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;
+};
+
+static void validate_info(struct xen_seabios_info *t)
+{
+    if ( memcmp(t->signature, "XenHVMSeaBIOS", 14) )
+        panic("Bad Xen info signature\n");
+
+    if ( t->length < sizeof(struct xen_seabios_info) )
+        panic("Bad Xen info length\n");
+
+    if (checksum(t, t->length) != 0)
+        panic("Bad Xen info checksum\n");
+}
+
+void xen_probe_hvm_info(void)
+{
+    if (!usingXen())
+        return;
+
+    dprintf(1, "Probing for Xen info\n");
+
+    struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
+
+    validate_info(info);
+
+    dprintf(1, "BIOS INFO: SIGNATURE %s\n", info->signature);
+}
+
+void xen_copy_biostable(void)
+{
+    struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
+
+    dprintf(1, "xen: copy BIOS tables...\n");
+    extern void copy_acpi_rsdp(void *pos);
+    dprintf(1, "xen: ... ACPI RSDP at %x\n", info->acpi_rsdp);
+    copy_acpi_rsdp((void *)info->acpi_rsdp);
+
+    extern void copy_mptable(void *pos);
+    dprintf(1, "xen: ... MPTABLE at %x\n", info->mptable);
+    copy_mptable((void *)info->mptable);
+}
+
+void xen_setup(void)
+{
+    u64 maxram = 0, maxram_over4G = 0;
+    int i;
+    struct xen_seabios_info *info = (void *)INFO_PHYSICAL_ADDRESS;
+
+    dprintf(1, "xen: copy e820...\n");
+
+    for (i = 0; i < info->e820_nr; i++) {
+	struct e820entry *e = &info->e820[i];
+	if (e->type == E820_ACPI || e->type == E820_RAM) {
+            u64 end = e->start + e->size;
+            if (end > 0x100000000ull) {
+                end -= 0x100000000ull;
+                if (end > maxram_over4G)
+                    maxram_over4G = end;
+            } else if (end > maxram)
+                maxram = end;
+	}
+	add_e820(e->start, e->size, e->type);
+    }
+
+    RamSize = maxram;
+    RamSizeOver4G = maxram_over4G;
+}
diff --git a/src/xen.h b/src/xen.h
new file mode 100644
index 0000000..ac89ff9
--- /dev/null
+++ b/src/xen.h
@@ -0,0 +1,56 @@
+#ifndef __XEN_H
+#define __XEN_H
+
+#include "util.h"
+
+void xen_probe(void);
+void xen_probe_hvm_info(void);
+
+extern u32 xen_cpuid_base;
+
+static inline int usingXen(void) {
+    if (!CONFIG_XEN)
+	return 0;
+    return (xen_cpuid_base != 0);
+}
+
+void xen_copy_biostable(void);
+
+void xen_setup(void);
+
+/******************************************************************************
+ *
+ * The following interface definitions are taken from Xen and have the
+ * following license:
+ *
+ * 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.
+ */
+
+/* xen.h */
+
+#define __HYPERVISOR_xen_version          17
+
+/* version.h */
+
+/* arg == xen_extraversion_t. */
+#define XENVER_extraversion 1
+typedef char xen_extraversion_t[16];
+#define XEN_EXTRAVERSION_LEN (sizeof(xen_extraversion_t))
+
+#endif
-- 
1.7.2.5


[-- Attachment #3: xen-hvmloader-per-bios-roms.patch --]
[-- Type: text/x-patch, Size: 5772 bytes --]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306422697 -3600
# Node ID 9b32ecfdffae7b92f7e4d7698416f05bc5778515
# Parent  71a3a138138bb1499a4e11eaea35b66e5cb1924f
hvmloader: allow per-BIOS decision on loading option ROMS

SeaBIOS has functionality to load ROMs from the PCI device directly,
it makes sense to use this when it is available.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 71a3a138138b -r 9b32ecfdffae tools/firmware/hvmloader/config-seabios.h
--- a/tools/firmware/hvmloader/config-seabios.h	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/config-seabios.h	Thu May 26 16:11:37 2011 +0100
@@ -1,9 +1,6 @@
 #ifndef __HVMLOADER_CONFIG_SEABIOS_H__
 #define __HVMLOADER_CONFIG_SEABIOS_H__
 
-#define OPTIONROM_PHYSICAL_ADDRESS  0x000C8000
-#define OPTIONROM_PHYSICAL_END      0x000E0000
-
 #define SEABIOS_PHYSICAL_ADDRESS    0x000E0000
 
 #endif /* __HVMLOADER_CONFIG_SEABIOS_H__ */
diff -r 71a3a138138b -r 9b32ecfdffae tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/config.h	Thu May 26 16:11:37 2011 +0100
@@ -19,7 +19,10 @@ struct bios_config {
     /* SMBIOS */
     unsigned int smbios_start, smbios_end;
 
-    /* Option ROMs */
+    /* ROMS */
+    int load_vgabios;
+    int load_etherboot;
+    int load_option_roms;
     unsigned int optionrom_start, optionrom_end;
 
     /* ACPI tables */
diff -r 71a3a138138b -r 9b32ecfdffae tools/firmware/hvmloader/hvmloader.c
--- a/tools/firmware/hvmloader/hvmloader.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/hvmloader.c	Thu May 26 16:11:37 2011 +0100
@@ -417,39 +417,48 @@ int main(void)
          ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode ) )
         bios->create_mp_tables();
 
-    switch ( virtual_vga )
+    if ( bios->load_vgabios )
     {
-    case VGA_cirrus:
-        printf("Loading Cirrus VGABIOS ...\n");
-        memcpy((void *)VGABIOS_PHYSICAL_ADDRESS,
-               vgabios_cirrusvga, sizeof(vgabios_cirrusvga));
-        vgabios_sz = round_option_rom(sizeof(vgabios_cirrusvga));
-        break;
-    case VGA_std:
-        printf("Loading Standard VGABIOS ...\n");
-        memcpy((void *)VGABIOS_PHYSICAL_ADDRESS,
-               vgabios_stdvga, sizeof(vgabios_stdvga));
-        vgabios_sz = round_option_rom(sizeof(vgabios_stdvga));
-        break;
-    case VGA_pt:
-        printf("Loading VGABIOS of passthroughed gfx ...\n");
-        vgabios_sz =
-            round_option_rom((*(uint8_t *)(VGABIOS_PHYSICAL_ADDRESS+2)) * 512);
-        break;
-    default:
-        printf("No emulated VGA adaptor ...\n");
-        break;
+        switch ( virtual_vga )
+        {
+        case VGA_cirrus:
+            printf("Loading Cirrus VGABIOS ...\n");
+            memcpy((void *)VGABIOS_PHYSICAL_ADDRESS,
+                   vgabios_cirrusvga, sizeof(vgabios_cirrusvga));
+            vgabios_sz = round_option_rom(sizeof(vgabios_cirrusvga));
+            break;
+        case VGA_std:
+            printf("Loading Standard VGABIOS ...\n");
+            memcpy((void *)VGABIOS_PHYSICAL_ADDRESS,
+                   vgabios_stdvga, sizeof(vgabios_stdvga));
+            vgabios_sz = round_option_rom(sizeof(vgabios_stdvga));
+            break;
+        case VGA_pt:
+            printf("Loading VGABIOS of passthroughed gfx ...\n");
+            vgabios_sz =
+                round_option_rom((*(uint8_t *)(VGABIOS_PHYSICAL_ADDRESS+2)) * 512);
+            break;
+        default:
+            printf("No emulated VGA adaptor ...\n");
+            break;
+        }
     }
 
     etherboot_phys_addr = VGABIOS_PHYSICAL_ADDRESS + vgabios_sz;
-    if ( etherboot_phys_addr < bios->optionrom_start )
-        etherboot_phys_addr = bios->optionrom_start;
-    etherboot_sz = scan_etherboot_nic(bios->optionrom_end,
-                                      etherboot_phys_addr);
+    if ( bios->load_etherboot )
+    {
+        if ( etherboot_phys_addr < bios->optionrom_start )
+            etherboot_phys_addr = bios->optionrom_start;
+        etherboot_sz = scan_etherboot_nic(bios->optionrom_end,
+                                          etherboot_phys_addr);
+    }
 
     option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
-    option_rom_sz = pci_load_option_roms(bios->optionrom_end,
-                                         option_rom_phys_addr);
+    if ( bios->load_option_roms )
+    {
+        option_rom_sz = pci_load_option_roms(bios->optionrom_end,
+                                             option_rom_phys_addr);
+    }
 
     if ( hvm_info->acpi_enabled )
     {
diff -r 71a3a138138b -r 9b32ecfdffae tools/firmware/hvmloader/rombios.c
--- a/tools/firmware/hvmloader/rombios.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/rombios.c	Thu May 26 16:11:37 2011 +0100
@@ -372,6 +372,10 @@ struct bios_config rombios_config =  {
     .smbios_start = SMBIOS_PHYSICAL_ADDRESS,
     .smbios_end = SMBIOS_PHYSICAL_END,
 
+    .load_vgabios = 1,
+    .load_etherboot = 1,
+    .load_option_roms = 1,
+
     .optionrom_start = OPTIONROM_PHYSICAL_ADDRESS,
     .optionrom_end = OPTIONROM_PHYSICAL_END,
 
diff -r 71a3a138138b -r 9b32ecfdffae tools/firmware/hvmloader/seabios.c
--- a/tools/firmware/hvmloader/seabios.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/seabios.c	Thu May 26 16:11:37 2011 +0100
@@ -46,8 +46,12 @@ struct bios_config seabios_config = {
     .smbios_start = 0,
     .smbios_end = 0,
 
-    .optionrom_start = OPTIONROM_PHYSICAL_ADDRESS,
-    .optionrom_end = OPTIONROM_PHYSICAL_END,
+    .load_vgabios = 0,
+    .load_etherboot = 0,
+    .load_option_roms = 0,
+
+    .optionrom_start = 0,
+    .optionrom_end = 0,
 
     .acpi_start = 0,
 

[-- Attachment #4: xen-further-seabios-support.patch --]
[-- Type: text/x-patch, Size: 13101 bytes --]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306422697 -3600
# Node ID ff9f86ef7be234c46afc1072f5191428bf66a9cd
# Parent  67d262e901a8fea9ac38fb106ccd7e2573e9506e
hvmloader: further support for SeaBIOS

Build the various BIOS tables and arrange for them to be passed to
SeaBIOS. We define an ad-hoc structure at a known physical address for
this purpose.

Also add a simple "scratch allocator" returning memory which is passed
to SeaBIOS but can be reused once the contents is consumed. SeaBIOS
copies various supplied tables to its own buffers during init.

XXX TODO refactor pci_setup and apic_setup into common location.
XXX TODO consider re-using coreboot table datastructure instead of ad-hoc.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/config-seabios.h
--- a/tools/firmware/hvmloader/config-seabios.h	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/config-seabios.h	Thu May 26 16:11:37 2011 +0100
@@ -1,6 +1,8 @@
 #ifndef __HVMLOADER_CONFIG_SEABIOS_H__
 #define __HVMLOADER_CONFIG_SEABIOS_H__
 
+#define BIOS_INFO_PHYSICAL_ADDRESS  0x00001000
+
 #define SEABIOS_PHYSICAL_ADDRESS    0x000E0000
 
 #endif /* __HVMLOADER_CONFIG_SEABIOS_H__ */
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/config.h	Thu May 26 16:11:37 2011 +0100
@@ -25,9 +25,6 @@ struct bios_config {
     int load_option_roms;
     unsigned int optionrom_start, optionrom_end;
 
-    /* ACPI tables */
-    unsigned int acpi_start;
-
     void (*apic_setup)(void);
     void (*pci_setup)(void);
     void (*smp_setup)(void);
@@ -38,7 +35,7 @@ struct bios_config {
     void (*vm86_setup)(void);
     void (*e820_setup)(void);
 
-    void (*acpi_build_tables)(unsigned int physical);
+    void (*acpi_build_tables)(void);
     void (*create_mp_tables)(void);
 };
 
@@ -73,6 +70,8 @@ extern unsigned long pci_mem_start, pci_
 #define VGABIOS_PHYSICAL_ADDRESS      0x000C0000
 #define HVMLOADER_PHYSICAL_ADDRESS    0x00100000
 
+extern unsigned long scratch_start;
+
 #endif /* __HVMLOADER_CONFIG_H__ */
 
 /*
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/hvmloader.c
--- a/tools/firmware/hvmloader/hvmloader.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/hvmloader.c	Thu May 26 16:11:37 2011 +0100
@@ -112,6 +112,8 @@ asm (
 unsigned long pci_mem_start = PCI_MEM_START;
 unsigned long pci_mem_end = PCI_MEM_END;
 
+unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS;
+
 enum virtual_vga virtual_vga = VGA_none;
 
 static void init_hypercalls(void)
@@ -401,8 +403,7 @@ int main(void)
 
     if (bios->smbios_start) {
         printf("Writing SMBIOS tables ...\n");
-        smbios_sz = hvm_write_smbios_tables(SCRATCH_PHYSICAL_ADDRESS,
-                                            bios->smbios_start,
+        smbios_sz = hvm_write_smbios_tables(bios->smbios_start,
                                             bios->smbios_end);
     }
 
@@ -470,7 +471,7 @@ int main(void)
 
         if ( bios->acpi_build_tables ) {
             printf("Loading ACPI ...\n");
-            bios->acpi_build_tables(bios->acpi_start);
+            bios->acpi_build_tables();
         }
         hypercall_hvm_op(HVMOP_set_param, &p);
     }
@@ -481,6 +482,9 @@ int main(void)
     cmos_write_memory_size();
 
     printf("BIOS map:\n");
+    if ( SCRATCH_PHYSICAL_ADDRESS != scratch_start )
+        printf(" %05x-%05lx: Scratch space\n",
+               SCRATCH_PHYSICAL_ADDRESS, scratch_start);
     if ( vgabios_sz )
         printf(" %05x-%05x: VGA BIOS\n",
                VGABIOS_PHYSICAL_ADDRESS,
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/mp_tables.c
--- a/tools/firmware/hvmloader/mp_tables.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/mp_tables.c	Thu May 26 16:11:37 2011 +0100
@@ -260,11 +260,12 @@ static void fill_mpfps(struct mp_floatin
 }
 
 /* create_mp_tables - creates MP tables for the guest based upon config data */
-void create_mp_tables(void *mp_table_base)
+unsigned long create_mp_tables(void *mp_table_base)
 {
     char *p;
     int vcpu_nr, i, length;
     struct mp_io_intr_entry *mpiie;
+    struct mp_floating_pointer_struct *mpfps;
 
     vcpu_nr = hvm_info->nr_vcpus;
 
@@ -313,9 +314,12 @@ void create_mp_tables(void *mp_table_bas
     /* find the next 16-byte boundary to place the mp floating pointer */
     while ( (unsigned long)p & 0xF )
         p++;
+    mpfps = (struct mp_floating_pointer_struct *)p;
+    p += sizeof(struct mp_floating_pointer_struct);
 
-    fill_mpfps((struct mp_floating_pointer_struct *)p, 
-               (uint32_t)mp_table_base);
+    fill_mpfps(mpfps, (uint32_t)mp_table_base);
 
     fill_mp_config_table((struct mp_config_table *)mp_table_base, length);
+
+    return (unsigned long)mpfps;
 }
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/rombios.c
--- a/tools/firmware/hvmloader/rombios.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/rombios.c	Thu May 26 16:11:37 2011 +0100
@@ -84,7 +84,8 @@ static void rombios_setup_bios_info(uint
     bios_info->bios32_entry = bioshigh;
 }
 
-static void rombios_apic_setup(void)
+/*XXX this should be common*/
+/*static*/ void rombios_apic_setup(void)
 {
     /* Set the IOAPIC ID to the static value used in the MP/ACPI tables. */
     ioapic_write(0x00, IOAPIC_ID);
@@ -99,7 +100,8 @@ static void rombios_apic_setup(void)
     ioapic_write(0x11, SET_APIC_ID(LAPIC_ID(0)));
 }
 
-static void rombios_pci_setup(void)
+/*XXX this should be common*/
+/*static*/ void rombios_pci_setup(void)
 {
     uint32_t base, devfn, bar_reg, bar_data, bar_sz, cmd, mmio_total = 0;
     uint16_t class, vendor_id, device_id;
@@ -113,7 +115,7 @@ static void rombios_pci_setup(void)
     /* Create a list of device BARs in descending order of size. */
     struct bars {
         uint32_t devfn, bar_reg, bar_sz;
-    } *bars = (struct bars *)SCRATCH_PHYSICAL_ADDRESS;
+    } *bars = (struct bars *)scratch_start;
     unsigned int i, nr_bars = 0;
 
     /* Program PCI-ISA bridge with appropriate link routes. */
@@ -343,6 +345,11 @@ static void reset_bios_checksum(void)
     *((uint8_t *)(ROMBIOS_BEGIN + ROMBIOS_MAXOFFSET)) = -checksum;
 }
 
+static void rombios_acpi_build_tables(void)
+{
+    acpi_build_tables(ACPI_PHYSICAL_ADDRESS);
+}
+
 static void rombios_create_mp_tables(void)
 {
     /* Find the 'safe' place in ROMBIOS for the MP tables. */
@@ -379,8 +386,6 @@ struct bios_config rombios_config =  {
     .optionrom_start = OPTIONROM_PHYSICAL_ADDRESS,
     .optionrom_end = OPTIONROM_PHYSICAL_END,
 
-    .acpi_start = ACPI_PHYSICAL_ADDRESS,
-
     .apic_setup = rombios_apic_setup,
     .pci_setup = rombios_pci_setup,
     .smp_setup = smp_initialise,
@@ -391,7 +396,7 @@ struct bios_config rombios_config =  {
     .vm86_setup = rombios_init_vm86_tss,
     .e820_setup = rombios_setup_e820,
 
-    .acpi_build_tables = acpi_build_tables,
+    .acpi_build_tables = rombios_acpi_build_tables,
     .create_mp_tables = rombios_create_mp_tables,
 };
 
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/seabios.c
--- a/tools/firmware/hvmloader/seabios.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/seabios.c	Thu May 26 16:11:37 2011 +0100
@@ -25,12 +25,75 @@
 
 #include "util.h"
 
+#include "acpi/acpi2_0.h"
+
 #define ROM_INCLUDE_SEABIOS
 #include "roms.inc"
 
+struct seabios_info {
+    char signature[14]; /* XenHVMSeaBIOS\0 */
+    uint16_t length;
+    uint32_t acpi_rsdp;
+    uint32_t mptable;
+    uint32_t e820_nr;
+    struct e820entry e820[128];
+    uint8_t checksum;
+};
+
+
+static void seabios_setup_bios_info(uint32_t bioshigh)
+{
+    struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS;
+    uint32_t i;
+    uint8_t checksum;
+
+    BUILD_BUG_ON(sizeof(struct seabios_info) > (0x9FC00 - BIOS_INFO_PHYSICAL_ADDRESS));
+
+    memcpy(info->signature, "XenHVMSeaBIOS", sizeof(info->signature));
+    info->length = sizeof(*info);
+
+    info->checksum = 0;
+
+    checksum = 0;
+    for (i = 0; i < info->length; ++i)
+        checksum += ((uint8_t *)(info))[i];
+
+    info->checksum = -checksum;
+}
+
 static void seabios_pci_setup(void)
 {
+    extern void rombios_pci_setup(void);
     virtual_vga = VGA_cirrus;
+    rombios_pci_setup();
+}
+
+static void seabios_apic_setup(void)
+{
+    extern void rombios_apic_setup(void);
+    rombios_apic_setup();
+}
+
+static void seabios_acpi_build_tables(void)
+{
+    struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS;
+    void *rsdp = scratch_alloc(sizeof(struct acpi_20_rsdp), 0);
+    acpi_build_tables((unsigned long)rsdp);
+    info->acpi_rsdp = (unsigned long)rsdp;
+}
+
+static void seabios_create_mp_tables(void)
+{
+    struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS;
+    void *table = scratch_alloc(32*1024, 0);
+    info->mptable = create_mp_tables(table);
+}
+
+static void seabios_setup_e820(void)
+{
+    struct seabios_info *info = (void *)BIOS_INFO_PHYSICAL_ADDRESS;
+    info->e820_nr = build_e820_table(info->e820);
+    dump_e820_table(info->e820, info->e820_nr);
 }
 
 //BUILD_BUG_ON(sizeof(seabios) > (0x00100000U - SEABIOS_PHYSICAL_ADDRESS));
@@ -53,19 +116,17 @@ struct bios_config seabios_config = {
     .optionrom_start = 0,
     .optionrom_end = 0,
 
-    .acpi_start = 0,
+    .bios_info_setup = seabios_setup_bios_info,
 
-    .bios_info_setup = NULL,
-
-    .apic_setup = NULL,
+    .apic_setup = seabios_apic_setup,
     .pci_setup = seabios_pci_setup,
-    .smp_setup = NULL,
+    .smp_setup = smp_initialise,
 
     .vm86_setup = NULL,
-    .e820_setup = NULL,
+    .e820_setup = seabios_setup_e820,
 
-    .acpi_build_tables = NULL,
-    .create_mp_tables = NULL,
+    .acpi_build_tables = seabios_acpi_build_tables,
+    .create_mp_tables = seabios_create_mp_tables,
 };
 
 /*
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/smbios.c
--- a/tools/firmware/hvmloader/smbios.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/smbios.c	Thu May 26 16:11:37 2011 +0100
@@ -162,7 +162,7 @@ get_memsize(void)
 }
 
 int
-hvm_write_smbios_tables(unsigned long scratch, unsigned long smbios_start, unsigned long smbios_end)
+hvm_write_smbios_tables(unsigned long smbios_start, unsigned long smbios_end)
 {
     xen_domain_handle_t uuid;
     uint16_t xen_major_version, xen_minor_version;
@@ -221,15 +221,15 @@ hvm_write_smbios_tables(unsigned long sc
 
     xen_version_str[sizeof(xen_version_str)-1] = '\0';
 
-    /* SCRATCH_PHYSICAL_ADDRESS is a safe large memory area for scratch. */
-    len = write_smbios_tables((void *)scratch, smbios_start,
+    /* scratch_start is a safe large memory area for scratch. */
+    len = write_smbios_tables((void *)scratch_start, smbios_start,
                               hvm_info->nr_vcpus, get_memsize(),
                               uuid, xen_version_str,
                               xen_major_version, xen_minor_version);
     if ( smbios_start + len > smbios_end )
         goto error_out;
     /* Okay, not too large: copy out of scratch to final location. */
-    memcpy((void *)smbios_start, (void *)scratch, len);
+    memcpy((void *)smbios_start, (void *)scratch_start, len);
 
     return len;
 
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/util.c
--- a/tools/firmware/hvmloader/util.c	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/util.c	Thu May 26 16:11:37 2011 +0100
@@ -362,6 +362,24 @@ void *mem_alloc(uint32_t size, uint32_t 
     return (void *)(unsigned long)s;
 }
 
+void *scratch_alloc(uint32_t size, uint32_t align)
+{
+    uint32_t s, e;
+
+    /* Align to at least one kilobyte. */
+    if ( align < 1024 )
+        align = 1024;
+
+    s = (scratch_start + align - 1) & ~(align - 1);
+    e = s + size - 1;
+
+    BUG_ON(e < s);
+
+    scratch_start = e;
+
+    return (void *)(unsigned long)s;
+}
+
 uint32_t ioapic_read(uint32_t reg)
 {
     *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
diff -r 67d262e901a8 -r ff9f86ef7be2 tools/firmware/hvmloader/util.h
--- a/tools/firmware/hvmloader/util.h	Thu May 26 16:11:37 2011 +0100
+++ b/tools/firmware/hvmloader/util.h	Thu May 26 16:11:37 2011 +0100
@@ -168,6 +168,9 @@ int vprintf(const char *fmt, va_list ap)
 void *mem_alloc(uint32_t size, uint32_t align);
 #define virt_to_phys(v) ((unsigned long)(v))
 
+/* Allocate memory in a scratch region */
+void *scratch_alloc(uint32_t size, uint32_t align);
+
 /* Connect our xenbus client to the backend.  
  * Call once, before any other xenbus actions. */
 void xenbus_setup(void);
@@ -185,9 +188,8 @@ uint32_t rombios_highbios_setup(void);
 
 /* Miscellaneous. */
 void cacheattr_init(void);
-void create_mp_tables(void *table);
-int hvm_write_smbios_tables(unsigned long scratch,
-			    unsigned long smbios_start,
+unsigned long create_mp_tables(void *table);
+int hvm_write_smbios_tables(unsigned long smbios_start,
 			    unsigned long smbios_end);
 void smp_initialise(void);
 

[-- Attachment #5: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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.

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Kevin O'Connor]

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

Re: [Xen-devel] Re: [RFC] [PATCH 0/2] Basic SeaBIOS support for Xen HVM

[Ian Campbell]

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.