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

[Julian Pidancet <julian.pidancet@xxxxxxxxx>]

On Wed, Feb 8, 2012 at 5:26 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 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.
>

You are right, I wrongly assumed that eb-roms.h only contained one ROM.

-- 
Julian

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