[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions
From: |
Gleb Natapov |
Subject: |
[Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file. |
Date: |
Fri, 2 Oct 2009 16:03:11 +0200 |
On Thu, Oct 01, 2009 at 08:51:27PM -0400, Kevin O'Connor wrote:
> On Thu, Oct 01, 2009 at 06:35:55PM +0200, Gleb Natapov wrote:
> > > As an aside, it would be good to have a conversation on general BIOS
> > > configuration options. These types of settings are going to be useful
> > > on real hardware also - it would be nice to come up with a scheme that
> > > would work on qemu and coreboot. Maybe something like
> > > get_config_u32("ShowBootMenu") - where on qemu it would get the info
> > > from the qemu port but on coreboot it would pull the setting from the
> > > coreboot flash filesystem.
> > >
> > I started to implement this approach, but found a serious disadvantage:
> > What if config option is not known to qemu or coreboot? What is it
> > present only in qemu and meaningful default behaviour is required
> > for coreboot? Like ShowBootMenu for instance. We want to return qemu
> > setting or coreboot setting or 1 if neither is present.
>
> I think passing in a default parameter like:
> get_config_u32("ShowBootMenu", 1) would work (return the config value
> or "1" if not found).
>
Brrr, ugly.
> [...]
> > --- /dev/null
> > +++ b/src/cbcfg.c
> > @@ -0,0 +1,13 @@
> > +#include "config.h"
> > +#include "util.h"
> > +#include "cfg.h"
> > +
> > +void cfg_get_uuid(u8 *uuid)
> > +{
> > + memset(uuid, 0, 16);
> > +}
> > +
> > +int cfg_show_boot_menu(void)
> > +{
> > + return 1;
> > +}
>
> What this would end up looking like is:
>
> int cfg_show_boot_menu(void)
> {
> return get_cbfs_config_u32("ShowBootMenu", 1);
> }
>
Something like this will be better:
int cfg_show_boot_menu(void)
{
if (cbf_config_exists("ShowBootMenu"))
return get_cbfs_config_u32("ShowBootMenu");
else
return 1;
}
The default value logic may be more complex than this. For instance:
int cfg_show_boot_menu(void)
{
if (cbf_config_exists("ShowBootMenu"))
return get_cbfs_config_u32("ShowBootMenu");
else if (cbf_config_exists("DefaultBootDevice"))
return 0;
else
return 1;
}
The other way to achieve this flexibility is to have interface like
bool get_config_u32(const char *name, u32 *val). This will return true
if name was found and val is meaningful. Caller will implement fallback
to default.
> Having to write these wrapper functions is tedious, which is why it
> would be nice if I could get a name/value pair directly from qemu.
>
And proposed get_config_u32() will look like this:
u32 get_config_u32(const char *name, u32 default)
{
if (COREBOOT)
return get_cbfs_config_u32("ShowBootMenu", 1);
else
return get_qemu_config_u32("ShowBootMenu", 1);
}
just another kind of wrapper. And get_qemu_config_u32 will have to map
string to config id since we are trying to adapt qemu to coreboot way.
> If qemu can provide a "stream" with a text config file in it, that's
> okay too. Basically, that's what I'm thinking of doing on coreboot.
> Something like:
>
> ===============================
> ShowBootMenu=1
> BootMenuDelayMS=5000
> ATA1-0-translation=2
> DefaultBootDevice=2
> ===============================
>
This kind of interface doesn't make any sense to qemu. Qemu doesn't have
shared memory with a guest to store config as a text file like coreboot does.
That is why qemu chose to provide name/value interface. Now you are saying since
we have this text file in coreboot that will have to be parsed to get
name/value interface from it lets do the same for qemu. But this is just
because you want to do abstraction on a wrong level. Qemu already
provides you with name/value so why do you want to transform it to plane
text and then pars to name/value back. Doesn't make any sense.
What makes sense it functional interface:
Does Boot menu should be shown?
What drive to boot from by default?
Load additional ACPI/SMBIOS tables.
And coreboot/qemu will implement them in a platform specific ways.
--
Gleb.