[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: |
Tue, 15 Sep 2009 08:43:39 +0300 |
On Mon, Sep 14, 2009 at 08:08:24PM -0400, Kevin O'Connor wrote:
> On Mon, Sep 14, 2009 at 03:51:41PM +0300, Gleb Natapov wrote:
> > Move qemu config code from smbios.c to its own files. Add support for
> > -boot menu=on|off qemu option.
>
> Hi Gleb,
>
> A couple of comments:
>
> > // Allow user to modify BCV/IPL order.
> > - interactive_bootmenu();
> > + if (qemu_cfg_show_boot_menu())
> > + interactive_bootmenu();
>
> Can you move this test into interactive_bootmenu()? (For non-qemu
> users the flow control looks odd otherwise.)
>
OK. The flow control will not go away though just move to other place so
non-qemu user will see odd flow control anyway :)
> > --- /dev/null
> > +++ b/src/pv.c
>
> What is "pv"? How about "qemu-cfg.c"?
>
pv == ParaVirtualization. I want to put non qemu specific thing there
to. For instance I put kvm_para_available() function there that checks
cpuid signature. I want to use it to replace if(CONFIG_KVM) in
ram_probe().
> > +void qemu_cfg_port_probe(void)
> > +{
> > + char *sig = "QEMU";
> > + int i;
> > +
> > + qemu_cfg_present = 1;
> > +
> > + qemu_cfg_select(QEMU_CFG_SIGNATURE);
> > +
> > + for (i = 0; i < 4; i++)
> > + if (inb(QEMU_CFG_DATA_PORT) != sig[i]) {
> > + qemu_cfg_present = 0;
> > + break;
> > + }
> > + dprintf(4, "qemu_cfg_present=%d\n", qemu_cfg_present);
> > +}
>
> This needs to have a "if (! CONFIG_COREBOOT) return;" - as the current
> check for qemu is not safe on real hardware.
>
Will add.
> Otherwise it looks good to me.
>
> 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.
>
Lets have conversation now. Sounds useful to me. Do you want to use
strings as option names though? They add to BIOS image size and it is
limited, no?
--
Gleb.
- [Qemu-devel] [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/14
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Kevin O'Connor, 2009/09/14
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file.,
Gleb Natapov <=
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Kevin O'Connor, 2009/09/15
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/17
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Kevin O'Connor, 2009/09/17
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/18
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Kevin O'Connor, 2009/09/19
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/19
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Kevin O'Connor, 2009/09/29
- [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/30
- Re: [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Blue Swirl, 2009/09/30
- Re: [Qemu-devel] Re: [PATCH][SEABIOS] Move qemu config port access functions into separate file., Gleb Natapov, 2009/09/30