From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=51855 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PHjb5-00039Y-EM
	for qemu-devel@nongnu.org; Sun, 14 Nov 2010 15:54:52 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1PHjb4-0005OB-1L
	for qemu-devel@nongnu.org; Sun, 14 Nov 2010 15:54:47 -0500
Received: from mx1.redhat.com ([209.132.183.28]:3318)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1PHjb3-0005O7-LP
	for qemu-devel@nongnu.org; Sun, 14 Nov 2010 15:54:45 -0500
Date: Sun, 14 Nov 2010 22:54:32 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20101114205432.GB14828@redhat.com>
References: <1289749181-12070-1-git-send-email-gleb@redhat.com>
	<1289749181-12070-16-git-send-email-gleb@redhat.com>
	<AANLkTi=1JpWB7GR3s2soWzgMsN3QixyfKF9uA6kCE5K3@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <AANLkTi=1JpWB7GR3s2soWzgMsN3QixyfKF9uA6kCE5K3@mail.gmail.com>
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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: kvm@vger.kernel.org, Gleb Natapov <gleb@redhat.com>, armbru@redhat.com, qemu-devel@nongnu.org, alex.williamson@redhat.com, kevin@koconnor.net

On Sun, Nov 14, 2010 at 08:49:59PM +0000, Blue Swirl wrote:
> On Sun, Nov 14, 2010 at 3:39 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> > =A0hw/fw_cfg.c | =A0 14 ++++++++++++++
> > =A0hw/fw_cfg.h | =A0 =A04 +++-
> > =A0sysemu.h =A0 =A0| =A0 =A01 +
> > =A0vl.c =A0 =A0 =A0 =A0| =A0 51 +++++++++++++++++++++++++++++++++++++=
++++++++++++++
> > =A04 files changed, 69 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> > index 7b9434f..f6a67db 100644
> > --- a/hw/fw_cfg.c
> > +++ b/hw/fw_cfg.c
> > @@ -53,6 +53,7 @@ struct FWCfgState {
> > =A0 =A0 FWCfgFiles *files;
> > =A0 =A0 uint16_t cur_entry;
> > =A0 =A0 uint32_t cur_offset;
> > + =A0 =A0Notifier machine_ready;
> > =A0};
> >
> > =A0static void fw_cfg_write(FWCfgState *s, uint8_t value)
> > @@ -315,6 +316,15 @@ int fw_cfg_add_file(FWCfgState *s, =A0const char=
 *filename, uint8_t *data,
> > =A0 =A0 return 1;
> > =A0}
> >
> > +static void fw_cfg_machine_ready(struct Notifier* n)
> > +{
> > + =A0 =A0uint32_t len;
> > + =A0 =A0char *bootindex =3D get_boot_devices_list(&len);
> > +
> > + =A0 =A0fw_cfg_add_bytes(container_of(n, FWCfgState, machine_ready),
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FW_CFG_BOOTINDEX, (uint8_t*=
)bootindex, len);
>=20
> I started to implement this to OpenBIOS but I noticed a small issue.
> First the first byte must be read to determine length. Then the read
> routine will be called again to read the correct amount of bytes. This
> would work, but since there is no shortage of IDs, I'd prefer a system
> where one ID is used to query the length and another ID is used to
> read the data, without the length byte. This is similar how command
> line, initrd etc. are handled.
>=20
> This would have the advantage that since fw_cfg uses little endian
> format, the length value would easily scale to for example 64 bits to
> support terabytes of boot device lists. ;-)

Yea. Let's just print # of devices as a property, in ASCII.
No endian-ness, no nothing.
Also - can we just NULL-terminate each ID?