Re: [Qemu-devel] [qemu RFC] qapi: add "firmware.json"

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>, Thomas Huth <thuth@redhat.com>,
	qemu-devel@nongnu.org, libvir-list@redhat.com,
	Alexander Graf <agraf@suse.de>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	David Gibson <dgibson@redhat.com>, Eric Blake <eblake@redhat.com>,
	Gary Ching-Pang Lin <glin@suse.com>,
	Kashyap Chamarthy <kchamart@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	Michal Privoznik <mprivozn@redhat.com>,
	Peter Krempa <pkrempa@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	David Gibson <david@gibson.dropbear.id.au>,
	Laurent Vivier <lvivier@redhat.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Subject: Re: [Qemu-devel] [qemu RFC] qapi: add "firmware.json"
Date: Tue, 10 Apr 2018 10:55:00 +0100	[thread overview]
Message-ID: <20180410095500.GK5155@redhat.com> (raw)
In-Reply-To: <20180410095131.ynii6t3om753vl2t@sirius.home.kraxel.org>

On Tue, Apr 10, 2018 at 11:51:31AM +0200, Gerd Hoffmann wrote:
> > > Hmm, I'm wondering whenever it is useful to model things this way.  It's
> > > not like you can actually configure things for -bios seabios.rom or
> > > -kernel uboot.elf.  Only pflash allows to actually configure things, and
> > > there are not that many useful combinations.  The code needs
> > > Read+Execute.  Allowing Write could be useful in theory, to allow the
> > > guest doing firmware updates.  But I think nobody actually does that, so
> > > in practice it is fixed.  The varstore can have different permissions,
> > > but it's only two useful combinations.  Either allow access
> > > unconditionally, or allow access in secure contect (aka smm) only.
> > 
> > (I hope I understand your point right:)
> > 
> > I'm also fine if we simply define a fixed (but extensible) set of
> > mapping methods, basically a new enum type, that simply tells libvirtd
> > what this firmware *is*. IOW, directly reference a mapping method we
> > know libvirt implements, rather than give vague hints.
> > 
> > This could repurpose SystemFirmwareType, but it should become more
> > detailed. I'm thinking like:
> > - ovmf: split files without requiring SMM
> > - ovmf_smm: split files with SMM requirement
> > - seabios: exactly that
> > - ... other things others suggest.
> 
> I wouldn't name them by firmware, that is misleading.  Basically we have
> three cases:
> 
>   (1) single firmware image (seabios, OVMF.fd, ...).
>   (2) split firmware image (OVMF_{CODE,VARS}.fd), where vars can be
>       writable unconditinally.
>   (3) split firmware image, where access to vars should be restricted
>       to smm mode.
> 
> (2) + (3) requires pflash.  (1) works with both pflash and -bios.

A big chunk of the data in the schema looks specific to the pflash
case, but this is not expressed except in the docs. Most of the time
with QAPI when we have data that is only relevant in certain types,
we use a discriminated union to describe it. It feels like a unioon
approach could be better suited to this

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|