From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MtijB-0002If-Df for qemu-devel@nongnu.org; Fri, 02 Oct 2009 10:03:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mtij6-0002IT-Pk for qemu-devel@nongnu.org; Fri, 02 Oct 2009 10:03:21 -0400 Received: from [199.232.76.173] (port=46770 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mtij6-0002IQ-Lz for qemu-devel@nongnu.org; Fri, 02 Oct 2009 10:03:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25626) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mtij5-0002ry-Vz for qemu-devel@nongnu.org; Fri, 02 Oct 2009 10:03:16 -0400 Date: Fri, 2 Oct 2009 16:03:11 +0200 From: Gleb Natapov Message-ID: <20091002140311.GC17326@redhat.com> References: <20090914125141.GB30746@redhat.com> <20090915000824.GA16210@morn.localdomain> <20091001163555.GV9832@redhat.com> <20091002005127.GA4835@morn.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091002005127.GA4835@morn.localdomain> Subject: [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor Cc: qemu-devel@nongnu.org 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.