[Qemu-devel] Re: [PATCHv4 15/15] Pass boot device list to firmware.

[Blue Swirl]

On Tue, Nov 16, 2010 at 7:22 AM, Gleb Natapov <address@hidden> wrote:
> On Mon, Nov 15, 2010 at 09:52:19PM -0500, Kevin O'Connor wrote:
>> On Mon, Nov 15, 2010 at 03:36:25PM +0200, Gleb Natapov wrote:
>> > On Mon, Nov 15, 2010 at 08:26:35AM -0500, Kevin O'Connor wrote:
>> > > On Mon, Nov 15, 2010 at 09:40:08AM +0200, Gleb Natapov wrote:
>> > > > On Sun, Nov 14, 2010 at 10:40:33PM -0500, Kevin O'Connor wrote:
>> > > > > Why not just return a newline separated list that is null terminated?
>> > > > >
>> > > > Doing it like this will needlessly complicate firmware side. How do you
>> > > > know how much memory to allocate before reading device list?
>> > >
>> > > My preference would be for the size to be exposed via the
>> > > QEMU_CFG_FILE_DIR selector.  (My preference would be for all objects
>> > > in fw_cfg to have entries in QEMU_CFG_FILE_DIR describing their size
>> > > in a reliable manner.)
>> > >
>> > Will interface suggested by Blue will be good for you? The one with two
>> > fw_cfg ids. BOOTINDEX_LEN for len and BOOTINDEX_DATA for device list. I
>>
>> I dislike how different fw_cfg objects pass the length in different
>> ways (eg, QEMU_CFG_E820_TABLE passes length as first 4 bytes).  This
>> is a common problem - I'd prefer if we could adopt one uniform way of
>> passing length.  I think QEMU_CFG_FILE_DIR solves this problem well.
>>
> Looking at available fw cfg option I see that _SIZE _DATA is also a
> common pattern. The problem with QEMU_CFG_FILE_DIR is that we have very
> little available slots right now. If we a going to require everything to
> use it we better grow number of available slots considerably now while
> it is easily done (no option defined above file slots yet).

FW_CFG_FILE_DIR seems to be a bit poorly designed. Maybe we should
deprecate it and design a more scalable model. There are also string
variables passed to BIOS (-prom-env for Sparc/PPC) which could then
use this new model instead of NVRAM.

> I personally do not have preferences one way or the other. Blue are you
> OK with using QEMU_CFG_FILE_DIR?

That would also work.

>> I also have an ulterior motive here.  If the boot order is exposed as
>> a newline separated list via an entry in QEMU_CFG_FILE_DIR, then this
>> becomes free for coreboot users as well.  (On coreboot, the boot order
>> could be placed in a "file" in flash with no change to the seabios
>> code.)
>>
> You can define get_boot_order() function and implement it differently
> for qemu and coreboot. For coreboot it will be one linear. Just call
> cbfs_copyfile("bootorder"). BTW why newline separation is important?

Newline and zero are both OK since neither can appear inside a valid boot path.