Re: [Xen-devel] [PATCH RFC] hvmloader: Make ROM dependencies optional

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-02-08 at 14:53 +0000, Julian Pidancet wrote:
> On Wed, Feb 8, 2012 at 1:28 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Sun, 2012-02-05 at 16:18 +0000, Julian Pidancet wrote:
> >> When booting HVMs with SeaBIOS, the BIOS itself takes care of extracting
> >> option ROMs from the PCI devices. These ROMs are usually provided with by
> >> the device model (qemu).
> >> Thus, hvmloader should not require any longer the rombios, stdvga,
> >> cirrusvga or etherboot ROMs to be present.
> >>
> >> Also, the 32bitbios_support.c file is specific to rombios and should not
> >> be built when building hvmloader with seabios.
> >
> >> [...]
> >> @@ -475,16 +477,20 @@ int main(void)
> >>         switch ( virtual_vga )
> >>          {
> >>          case VGA_cirrus:
> >> +#ifdef ENABLE_CIRRUSVGA
> >
> > Just outside the context here is an "if ( bios->load_roms )" which
> > prevents the ROMs from being loaded if we are using SeaBIOS -- is that
> > not sufficient for your needs?
> >
> > Currently hvmloader builds with support for both ROMBIOS and SeaBIOS and
> > selects the one to use based on a configuration key in xenstore (pushed
> > down by the toolstack depending on which qemu you are running). Have you
> > modified something elsewhere in order to build a SeaBIOS-only hvmloader?
> >
> > Perhaps it would make sense for the ROMs to be keyed off whether or not
> > ROMBIOS is enabled rather than each keying off their own DIR var?
> >
> > You seem to have mixed some other changes, specifically s/:=/?=/ in a
> > few places and also some sort of change wrt how eb-roms.h is generated
> > (including dropping the 8086100e ROM from the image, which must be
> > discussed).
> >
> > These other changes should be submitted and rationalised separately.
> >
> 
> The purpose of this change was initially to allow the user to build
> hvmloader with SeaBIOS only without having to depend on rombios,
> vgabios or ipxe. I've only tested my changes by calling make from the
> hvmloader/ directory directly. The Xen build system could also not
> have to depend on bcc for example.
> 
> In addition to the patch I submitted, I suppose it would make sense to
> introduce a global configuration option for the Makefile in the
> firmware/ directory to decide wether it should recurse in the rombios,
> vgabios, and etherboot directories.
> 
> I changed some variable assignations := by ?= because I wanted the
> user of the build system to be able to specify a custom build
> directory for these ROMs.

Seems fair enough, although I would still like to see separate issues
solved in separate patches.

> Same thing for the etherboot ROM, I made the Makefile not use eb-rom.h
> and replace it with a call to mkhex on the binary file directly, so
> the user can choose which ROM to inject into hvmloader, and I found it
> to be more consistent with the rest of the Makefile.

I don't think you have replaced like with like. Specifically you seem to
have omitted the 8086100e ROM -- look how eb-roms.h is built in
tools/firmware/etherboot using NICS = rtl8139 8086100e from the file
"Config" compared with your code which only uses ETHERBOOT_NIC :=
rtl8139.

Ian.



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