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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

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.


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