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

[Daniel P . Berrangé]

On Mon, Apr 09, 2018 at 07:57:54PM +0200, Laszlo Ersek wrote:
> On 04/09/18 10:49, Daniel P. Berrangé wrote:
> > On Sat, Apr 07, 2018 at 02:01:17AM +0200, Laszlo Ersek wrote:
> >> Add a schema that describes the properties of virtual machine firmware.
> >>
> >> Each firmware executable installed on a host system should come with a
> >> JSON file that conforms to this schema, and informs the management
> >> applications about the firmware's properties.
> >>
> >> In addition, a configuration directory with symlinks to the JSON files
> >> should exist, with the symlinks carefully named to reflect a priority
> >> order. Management applications can then search this directory in priority
> >> order for the first firmware executable that satisfies their search
> >> criteria. The found JSON file provides the management layer with domain
> >> configuration bits that are required to run the firmware binary.
> >>
> >> Cc: "Daniel P. Berrange" <address@hidden>
> >> Cc: Alexander Graf <address@hidden>
> >> Cc: Ard Biesheuvel <address@hidden>
> >> Cc: David Gibson <address@hidden>
> >> Cc: Eric Blake <address@hidden>
> >> Cc: Gary Ching-Pang Lin <address@hidden>
> >> Cc: Gerd Hoffmann <address@hidden>
> >> Cc: Kashyap Chamarthy <address@hidden>
> >> Cc: Markus Armbruster <address@hidden>
> >> Cc: Michael Roth <address@hidden>
> >> Cc: Michal Privoznik <address@hidden>
> >> Cc: Peter Krempa <address@hidden>
> >> Cc: Peter Maydell <address@hidden>
> >> Cc: Thomas Huth <address@hidden>
> >> Signed-off-by: Laszlo Ersek <address@hidden>
> >> ---
> >>
> >> Notes:
> >>     Folks on the CC list, please try to see if the suggested schema is
> >>     flexible enough to describe the virtual firmware(s) that you are
> >>     familiar with. Thanks!
> >>
> >>  Makefile              |   9 ++
> >>  Makefile.objs         |   4 +
> >>  qapi/firmware.json    | 343 
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  qapi/qapi-schema.json |   1 +
> >>  qmp.c                 |   5 +
> >>  .gitignore            |   4 +
> >>  6 files changed, 366 insertions(+)
> >>  create mode 100644 qapi/firmware.json
> >>
> > 
> >> diff --git a/qapi/firmware.json b/qapi/firmware.json
> >> new file mode 100644
> >> index 000000000000..f267240f44dd
> >> --- /dev/null
> >> +++ b/qapi/firmware.json
> >> @@ -0,0 +1,343 @@
> >> +# -*- Mode: Python -*-
> >> +
> >> +##
> >> +# = Firmware
> >> +##
> >> +
> >> +##
> >> +# @FirmwareDevice:
> >> +#
> >> +# Defines the device types that a firmware file can be mapped into.
> >> +#
> >> +# @memory: The firmware file is to be mapped into memory.
> >> +#
> >> +# @kernel: The firmware file is to be loaded like a Linux kernel. This is
> >> +#          similar to @memory but may imply additional processing that is
> >> +#          specific to the target architecture.
> >> +#
> >> +# @flash: The firmware file is to be mapped into a pflash chip.
> >> +#
> >> +# Since: 2.13
> >> +##
> >> +{ 'enum' : 'FirmwareDevice',
> >> +  'data' : [ 'memory', 'kernel', 'flash' ] }
> >> +
> >> +##
> >> +# @FirmwareAccess:
> >> +#
> >> +# Defines the possible permissions for a given access mode to a device 
> >> that
> >> +# maps a firmware file.
> >> +#
> >> +# @denied: The access is denied.
> >> +#
> >> +# @permitted: The access is permitted.
> >> +#
> >> +# @restricted-to-secure-context: The access is permitted for guest code 
> >> that
> >> +#                                runs in a secure context; otherwise the 
> >> access
> >> +#                                is denied. The definition of "secure 
> >> context"
> >> +#                                is specific to the target architecture.
> >> +#
> >> +# Since: 2.13
> >> +##
> >> +{ 'enum' : 'FirmwareAccess',
> >> +  'data' : [ 'denied', 'permitted', 'restricted-to-secure-context' ] }
> > 
> > I'm not really understanding the purpose of this - what does it map to
> > on the command line ?
> 
> That's difficult to answer generally, because -bios and -kernel have
> different meanings per board type. So I didn't aim at command line
> switches here; instead I tried to capture where and how the firmware
> wants to "end up" in the virtual hardware. How that maps to a particular
> board is a separate question.

I tend to think that defining a mapping to command line arguments is a key
feature that this should cover. Even if there variations across boards, QEMU
still has a small finite set of approaches to configure firmware, so it does
not feel unreasonable to define what they are and how they map to thes firmware
files.

Your FirmwareDevice enum above with "memory", "kernel" and "flash" has
pretty much suggested the -bios, -kernel or -drive if=flash args anway

> So, the schema intends to describe the mapping that the firmware expects
> from the board. How that is implemented on the QEMU command line is left
> as an exercise to ... libvirtd. :)

I think this is pretty unhelpful. Essentially that is saying here is a big
pile of information about firmware, but we're not going to tell you how to
use it correctly, everyone must figure it out themselves.


> >> +##
> >> +# @FirmwareFile:
> >> +#
> >> +# Gathers the common traits of system firmware executables and NVRAM 
> >> templates.
> >> +#
> >> +# @pathname: absolute pathname of the firmware file on the host filesystem
> >> +#
> >> +# @description: human-readable description of the firmware file
> >> +#
> >> +# @tags: a list of machine-readable strings providing additional 
> >> information
> > 
> > This makes it look like this information is something applications should be
> > using when setting up firmwares, which is definitely not what we want. Lets
> > rename this
> > 
> >   "@build_options: arbitrary list of firmware specific build options, for
> >                    informative purposes only. Applications should not 
> > attempt
> >                    to interpret / assign meaning to these options"
> 
> Hmmm, I agree with you half-way here. I'm not saying that applications
> *should* consult the tags, but they might want let the user express a
> search condition for the tags. Near the end of the RFC, there's an
> example JSON where the sole nvramslot advertizes two variable store
> templates (both of which are compatible with the firmware and the
> nvramslot from a technical POV). However, one varstore template is
> logically empty, and the other varstore template has the MS certificates
> pre-enrolled and Secure Boot enabled. If the new domain is created with
> an OS installer that is not signed at all, the choice of varstore
> template can make a big difference. And, the way I could distinguish
> these two templates from each other (in a machine readable format) is
> the "tags" list -- pls. search the RFC for the string '"mscerts"'.

I don't think we should be using "tags" for the "mscerts" information.
There should be some kind of explicit way to denote that the vars have
been pre-enrolled or not.


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 :|