From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37536 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIQ7y-0003q9-B2 for qemu-devel@nongnu.org; Tue, 16 Nov 2010 13:19:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PIQ7x-0007Bn-02 for qemu-devel@nongnu.org; Tue, 16 Nov 2010 13:19:34 -0500 Received: from mail-qw0-f45.google.com ([209.85.216.45]:50620) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PIQ7w-0007Bh-R3 for qemu-devel@nongnu.org; Tue, 16 Nov 2010 13:19:32 -0500 Received: by qwi2 with SMTP id 2so215385qwi.4 for ; Tue, 16 Nov 2010 10:19:32 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20101116072245.GP7948@redhat.com> References: <1289749181-12070-1-git-send-email-gleb@redhat.com> <1289749181-12070-16-git-send-email-gleb@redhat.com> <20101115034033.GA1309@morn.localdomain> <20101115074008.GF7948@redhat.com> <20101115132635.GA14119@morn.localdomain> <20101115133625.GN7948@redhat.com> <20101116025219.GA6423@morn.localdomain> <20101116072245.GP7948@redhat.com> From: Blue Swirl Date: Tue, 16 Nov 2010 18:19:10 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCHv4 15/15] Pass boot device list to firmware. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: kvm@vger.kernel.org, mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, alex.williamson@redhat.com, Kevin O'Connor On Tue, Nov 16, 2010 at 7:22 AM, Gleb Natapov 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 termin= ated? >> > > > > >> > > > Doing it like this will needlessly complicate firmware side. How d= o 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. =C2=A0(My preference would be for all ob= jects >> > > 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 tw= o >> > 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). =C2=A0Thi= s >> is a common problem - I'd prefer if we could adopt one uniform way of >> passing length. =C2=A0I 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. =C2=A0If 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. =C2=A0(On coreboot, the boot or= der >> 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 p= ath.