Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 12:20, Daniel P. Berrangé wrote:
> On Sat, Apr 07, 2018 at 02:01:17AM +0200, Laszlo Ersek wrote:

>> +{ 'struct' : 'SystemFirmware',
>> +  'data'   : { 'executable' : 'FirmwareFile',
>> +   'type'   : 'SystemFirmwareType',
>> +   'targets': [ 'str' ],
>> +   'sysfw-map'  : 'FirmwareMapping',
>> +   '*nvram-slots'   : [ 'NVRAMSlot' ],
>> +   '*supports-uefi-secure-boot' : 'bool',
>> +   '*supports-amd-sev'  : 'bool',
>> +   '*supports-acpi-s3'  : 'bool',
>> +   '*supports-acpi-s4'  : 'bool' } }
> 
> Elsewhere in the thread I mentioned that I think we should try to use a
> union approach to isolate which information is relevant to "flash" loader
> format and which is relevant to "memory" and "kernel". To try to illustrate
> what I mean by that I've knocked up an alternative structure. I also
> incorporated the points about features & target/machine types.  I've left
> out the read/write/etc fields, but they could be put back in at the
> relevant position

I think this looks very nice; with the addition of

- "requires-smm" to "SystemFirmwareFeature":

> { 'enum' : 'SystemFirmwareFeature',
>   'data': ['acpi-s3', 'acpi-s5', 'secure-boot', 'amd-sev' ]}

- and another feature flag (perhaps in SystemFirmwareFeature, perhaps in
SystemFirmwareBinaryFlashVars) for the cmdline option "-global
driver=cfi.pflash01,property=secure,value=on",

this could be called a day as far as SeaBIOS and OVMF are concerned.

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:55, Daniel P. Berrangé wrote:
> 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

I used a discriminated union specifically for pflash options in RFCv0,
which I didn't post. I felt that it wasn't flexible enough. :)

Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:34, Daniel P. Berrangé wrote:
> On Tue, Apr 10, 2018 at 11:16:01AM +0200, Laszlo Ersek wrote:
>> On 04/10/18 08:27, Gerd Hoffmann wrote:
>>>   Hi,
>>>
 - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
 present and future, for SMM-requiring OVMF builds), but then you get
 into version sorting and similar mess. I considered fnmatch() --
 basically simple ? and * wildcards -- but that's not expressive enough.
>>>
>>> I'd suggest whitelist with wildcards.  So the smm builds would get
>>> "pc-q35-*".
>>>
>>> libvirt knows about aliases, so it should be able to handle the "q35"
>>> shortcut like "pc-q35-${latest}".
>>>
>>> Or do you see another issue?
>>
>> Well, one issue I see is version sorting; I should say "Q35 but no
>> earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".
>>
>> Anyway (also asking for Thomas's input here): if we run with your idea
>> to refer to exact mapping methods / firmware *implementation* types that
>> we know libvirt implements / supports as a "white box", do we still deem
>> machine type identification necessary? Because, libvirt already knows
>> (for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
>> have to make a *reference* to that knowledge in the JSON file.
> 
> BTW, that's not quite correct - when libvirt handles the "smm" arg it
> checks if machine type == q35, and  QEMU version >= 2.4.
> 
> It is *not* checking the version of the machine type. ie it will happily
> use smm with  pc-q35-2.0, as long as QEMU version is 2.4. Perhaps this is
> not quite right,

(it's not)

> but we don't try to parse the version number out of the
> machine type, because we can't assume a specific format for the machine
> type version part. eg version can be "2.4", or it can be "rhel-7.0.0"
> or something else again on Ubuntu.

Indeed, that's exactly why I'm troubled about expressing a "minimum"
machine type version.

> 
> IMHO it would be valid to just keep life simple and only record the base
> machine type name that can use the firmware ie "pc", "q35", and ignore
> the fact that in some cases the firmware might require a specific version
> of the machine type.

Esp. with regard to SMM, there have been quite big jumps in usability /
stability across Q35 machtype versions. But, if it works for you, it
works for me.

(I double-checked Thomas's recent example about U-Boot, and he mentioned
the "ppce500" and "sam460ex" machine types, not machine type versions.)

Thanks,
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:32, Thomas Huth wrote:
> On 10.04.2018 11:22, Laszlo Ersek wrote:
>> On 04/10/18 09:33, Thomas Huth wrote:
> [...]
>>> Alternatively, what about providing some kind of "alias" or "nickname"
>>> setting here, too? So the EDK2 builds would get
>>> SystemFirmwareType="edk2" and "SystemFirmwareAlias="uefi" for example.
>>
>> I hope I understand you right -- I think your suggestion ties in with my
>> other email I just sent in this thread. So, we could tell libvirtd,
>> "this firmware is of type 'UEFI', and you must use the 'ovmf_smm'
>> mapping method to run it, with this file or that file as varstore template".
>>
>> We could even describe the parameters for this or that mapping method
>> structurally in the schema (in a discriminated union in QAPI JSON, or in
>> an XSD choice element). For example, "ovmf" and "ovmf_smm" would both
>> take "OvmfSplitFileOptions" -- a list of single varstore template files
>> with feature enum contants attached  --, while "SeaBiosOptions" would be
>> an empty structure.
> 
> Sorry, I've got no clue about ovmf_smm and the other things you've
> mentioned here ;-)
> 
>> I feel the key question here is whether we are allowed to directly
>> reference a mapping method we know libvirt implements. If we are, that
>> makes things a lot clearer (and easier, I should hope).
> 
> Key question is maybe rather: Do you want to design / implement
> something that is libvirt-only here, or rather something generic that
> could also be used for other upper layer tools that do not use libvirt?
> (... and looks like Daniel just had the same comment in another mail in
> this thread ...)

Yeah, we can't target libvirtd as the sole consumer.

Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:26, Thomas Huth wrote:
> On 10.04.2018 11:16, Laszlo Ersek wrote:
>> On 04/10/18 08:27, Gerd Hoffmann wrote:
>>>   Hi,
>>>
 - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
 present and future, for SMM-requiring OVMF builds), but then you get
 into version sorting and similar mess. I considered fnmatch() --
 basically simple ? and * wildcards -- but that's not expressive enough.
>>>
>>> I'd suggest whitelist with wildcards.  So the smm builds would get
>>> "pc-q35-*".
>>>
>>> libvirt knows about aliases, so it should be able to handle the "q35"
>>> shortcut like "pc-q35-${latest}".
>>>
>>> Or do you see another issue?
>>
>> Well, one issue I see is version sorting; I should say "Q35 but no
>> earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".
>>
>> Anyway (also asking for Thomas's input here): if we run with your idea
>> to refer to exact mapping methods / firmware *implementation* types that
>> we know libvirt implements / supports as a "white box", do we still deem
>> machine type identification necessary? Because, libvirt already knows
>> (for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
>> have to make a *reference* to that knowledge in the JSON file.
> 
> I think you really need a way to specify the machine there. Latest
> example from QEMU 2.12: We've now got two uboot binaries in the tree,
> pc-bios/u-boot.e500 and pc-bios/u-boot-sam460-20100605.bin. Both are
> uboot, both are for ppc, but u-boot.e500 only works with the "ppce500"
> machine and the other one only works with the "sam460ex" machine. How
> would you teach libvirt such a relationship without an explicit machine
> type identification field there?

My idea was to assign different "map method" enumeration constants to
them, and libvirtd would associate those with different machtype
requirements.

But, as Daniel explained, we cannot reference libvirtd features, so I
agree we have to express machine types somehow. I don't know how though.

For example, can we take it for granted that a machtype version number,
if it exists in the first place, always follows the last hyphen in the
machtype identifier? Say, "virt-2.11" / aarch64 conforms, "pc-q35-2.4"
and "pc-i440fx-2.12" conform too. But, is that a guarantee that covers
all arches and all boards?

Because, I don't think:
- machine-type-family = q35
- minimum-machine-type = 2.4

will work. Will every application that manages QEMU learn that "q35" is
short-hand for "pc-q35-XXX", and (again) that 2.12 sorts *after* 2.4?

Thanks,
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Tue, Apr 10, 2018 at 12:48:28PM +0100, Peter Maydell wrote:
> On 10 April 2018 at 12:34, Daniel P. Berrangé  wrote:
> > On Tue, Apr 10, 2018 at 01:27:18PM +0200, Laszlo Ersek wrote:
> >
> >> Please go through the rest of the emails in this thread, and advise:
> >> - if the firmware descriptor schema may perhaps live in the libvirt tree,
> >> - accordingly, if the schema could be expressed as an XSD (and firmware
> >> packages should provide the descriptor documents as XMLs)
> >> - if you agree that the descriptor document can uniquely reference
> >> mapping methods implemented in libvirtd by simple enum constants (with
> >> necessary parameters provided).
> >
> > No to all three. This is the responsibility of QEMU to define, because
> > this information is relevant to anything managing QEMU not just libvirt.
> 
> (Please consider this as more of a grenade lobbed into the conversation
> rather than a carefully thought out proposal...)
> 
> My inclination is to say that it's not really the responsibility
> of QEMU to define either -- we provide emulated models of hardware,
> and it's up to the user or the management layer or the provider
> of the firmware to specify what guest code they want to run and how
> it needs to run on that emulated hardware...
> 
> Where the QEMU upstream itself is providing firmware blobs
> (in tarballs etc) it's probably our job to specify how they work,
> but if the firmware is compiled and provided by the distro (as eg happens
> for Arm UEFI blobs at the moment) then I don't see how upstream QEMU
> can reliably define how that firmware needs to be loaded.

QEMU should not provide the actual metadata files themselves - it just
has to the define the file format. The relevant firmware upstreams and
or distros, can provide the metadata files for the blobs they choose
to ship for use with QEMU.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Tue, Apr 10, 2018 at 01:44:13PM +0200, Laszlo Ersek wrote:
> On 04/10/18 13:34, Daniel P. Berrangé wrote:
> > On Tue, Apr 10, 2018 at 01:27:18PM +0200, Laszlo Ersek wrote:
> > 
> >> Please go through the rest of the emails in this thread, and advise:
> >> - if the firmware descriptor schema may perhaps live in the libvirt tree,
> >> - accordingly, if the schema could be expressed as an XSD (and firmware
> >> packages should provide the descriptor documents as XMLs)
> >> - if you agree that the descriptor document can uniquely reference
> >> mapping methods implemented in libvirtd by simple enum constants (with
> >> necessary parameters provided).
> > 
> > No to all three. This is the responsibility of QEMU to define, because
> > this information is relevant to anything managing QEMU not just libvirt.
> 
> In that case, how do you suggest we describe the QEMU command line
> options that are (a) necessary, (b) "discoverable" to the management
> application? Should we provide verbatim command line fragments (option
> templates)? Is this feature meant to replace the cmdline generation
> logic that already exists in libvirtd?

Each part of the schema should have docs describing what CLI args it
corresponds to. eg document that when device=memory, corresponds
to -bios, that device=flash, corresponds to -drive if=pflash, etc

We've not trying to replace the cmdline generator in libvirt. We just
want to know that when we see a particular field present in the schema,
that it corresponds to a particular cli arg.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Peter Maydell]

On 10 April 2018 at 12:34, Daniel P. Berrangé  wrote:
> On Tue, Apr 10, 2018 at 01:27:18PM +0200, Laszlo Ersek wrote:
>
>> Please go through the rest of the emails in this thread, and advise:
>> - if the firmware descriptor schema may perhaps live in the libvirt tree,
>> - accordingly, if the schema could be expressed as an XSD (and firmware
>> packages should provide the descriptor documents as XMLs)
>> - if you agree that the descriptor document can uniquely reference
>> mapping methods implemented in libvirtd by simple enum constants (with
>> necessary parameters provided).
>
> No to all three. This is the responsibility of QEMU to define, because
> this information is relevant to anything managing QEMU not just libvirt.

(Please consider this as more of a grenade lobbed into the conversation
rather than a carefully thought out proposal...)

My inclination is to say that it's not really the responsibility
of QEMU to define either -- we provide emulated models of hardware,
and it's up to the user or the management layer or the provider
of the firmware to specify what guest code they want to run and how
it needs to run on that emulated hardware...

Where the QEMU upstream itself is providing firmware blobs
(in tarballs etc) it's probably our job to specify how they work,
but if the firmware is compiled and provided by the distro (as eg happens
for Arm UEFI blobs at the moment) then I don't see how upstream QEMU
can reliably define how that firmware needs to be loaded.

thanks
-- PMM

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 13:34, Daniel P. Berrangé wrote:
> On Tue, Apr 10, 2018 at 01:27:18PM +0200, Laszlo Ersek wrote:
> 
>> Please go through the rest of the emails in this thread, and advise:
>> - if the firmware descriptor schema may perhaps live in the libvirt tree,
>> - accordingly, if the schema could be expressed as an XSD (and firmware
>> packages should provide the descriptor documents as XMLs)
>> - if you agree that the descriptor document can uniquely reference
>> mapping methods implemented in libvirtd by simple enum constants (with
>> necessary parameters provided).
> 
> No to all three. This is the responsibility of QEMU to define, because
> this information is relevant to anything managing QEMU not just libvirt.

In that case, how do you suggest we describe the QEMU command line
options that are (a) necessary, (b) "discoverable" to the management
application? Should we provide verbatim command line fragments (option
templates)? Is this feature meant to replace the cmdline generation
logic that already exists in libvirtd?

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:05, Daniel P. Berrangé wrote:
> On Mon, Apr 09, 2018 at 06:34:41PM +0200, Laszlo Ersek wrote:
>> On 04/09/18 09:26, Thomas Huth wrote:
>>>  Hi Laszlo,
>>>
>>> On 07.04.2018 02:01, 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.
>>> [...]
 +##
 +# @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' ] }
>>>
>>> This is not fully clear to me... what is this exactly good for? Is
>>> this a way to say how the firmware should be loaded, i.e. via
>>> "-bios", "-kernel" or "-pflash" parameter? If so, the term "memory"
>>> is quite misleading since files that are loaded via -bios can also
>>> end up in an emulated ROM chip.
>>
>> I threw in "-kernel" because, although it also (usually?) means
>> "memory", I expected people would want it separate.
>
> What platform / scenario actually uses -kernel to load firmware. If
> you have loaded firmware using -kernel, how do you then load the
> actual kernel ?

AAVMF has a build called "ArmVirtQemuKernel" where the firmware is
loaded with the -kernel switch.

commit 8de84d4242215252af9d2afecd45e2419689ee5f
Author: Ard Biesheuvel 
Date:   Fri Feb 5 14:57:57 2016 +0100

ArmVirtPkg: implement ArmVirtQemuKernel

This implements a version of ArmVirtQemu that does not execute in place from
emulated NOR flash, but implements the Linux kernel boot protocol, and 
executes
from DRAM instead. This allows UEFI to be loaded as a payload by a previous
bootloader stage such as ARM Trusted Firmware/OP-TEE.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 

My understanding is that in this scenario you cannot use -kernel for
loading a Linux kernel; instead you have to boot the Linux OS off of
some other media (CD-ROM, disk, network...) Personally I never use this
AAVMF build, but I know it exists and Ard uses it at least occasionally.

Thanks,
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Tue, Apr 10, 2018 at 01:27:18PM +0200, Laszlo Ersek wrote:

> Please go through the rest of the emails in this thread, and advise:
> - if the firmware descriptor schema may perhaps live in the libvirt tree,
> - accordingly, if the schema could be expressed as an XSD (and firmware
> packages should provide the descriptor documents as XMLs)
> - if you agree that the descriptor document can uniquely reference
> mapping methods implemented in libvirtd by simple enum constants (with
> necessary parameters provided).

No to all three. This is the responsibility of QEMU to define, because
this information is relevant to anything managing QEMU not just libvirt.


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 11:18, Daniel P. Berrangé wrote:
> 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" 
 Cc: Alexander Graf 
 Cc: Ard Biesheuvel 
 Cc: David Gibson 
 Cc: Eric Blake 
 Cc: Gary Ching-Pang Lin 
 Cc: Gerd Hoffmann 
 Cc: Kashyap Chamarthy 
 Cc: Markus Armbruster 
 Cc: Michael Roth 
 Cc: Michal Privoznik 
 Cc: Peter Krempa 
 Cc: Peter Maydell 
 Cc: Thomas Huth 
 Signed-off-by: Laszlo Ersek 
 ---

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

I agree, now that I've read about Gerd's similar argument.

There I made the suggestion that the schema could define enum constants
(mapping identifiers) that directly refer to libvirtd's existing logic
to map various firmware types.

> 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

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

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

> diff --git a/qapi/firmware.json b/qapi/firmware.json
> new file mode 100644
> index ..f267240f44dd
> --- /dev/null
> +++ b/qapi/firmware.json

[snip]

> +{ 'struct' : 'SystemFirmware',
> +  'data'   : { 'executable' : 'FirmwareFile',
> +   'type'   : 'SystemFirmwareType',
> +   'targets': [ 'str' ],
> +   'sysfw-map'  : 'FirmwareMapping',
> +   '*nvram-slots'   : [ 'NVRAMSlot' ],
> +   '*supports-uefi-secure-boot' : 'bool',
> +   '*supports-amd-sev'  : 'bool',
> +   '*supports-acpi-s3'  : 'bool',
> +   '*supports-acpi-s4'  : 'bool' } }

Elsewhere in the thread I mentioned that I think we should try to use a
union approach to isolate which information is relevant to "flash" loader
format and which is relevant to "memory" and "kernel". To try to illustrate
what I mean by that I've knocked up an alternative structure. I also
incorporated the points about features & target/machine types.  I've left
out the read/write/etc fields, but they could be put back in at the
relevant position


{ 'enum' : 'SystemFirmwareType',
  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }

{ 'enum' : 'SystemFirmwareDevice',
  'data' : [ 'memory', 'kernel', 'flash' ] }

{ 'enum' : 'SystemFirmwareArchitecture',
  'data':  ['x86_64', 'i386', ..etc.. ] }
  
{ 'enum' : 'SystemFirmwareFeature',
  'data': ['acpi-s3', 'acpi-s5', 'secure-boot', 'amd-sev' ]}


## Struct(s) for device==memory

{ 'struct': 'SystemFirmwareBinaryMemory',
  'data': { 'pathname': 'str' } }


## Struct(s) for device==kernel

{ 'struct': 'SystemFirmwareBinaryKernel',
  'data': { 'pathname': 'str' } }


## Struct(s) for device==flash

{ 'struct': 'SystemFirmwareBinaryFlashFile',
  'data':  { 'filename': 'str',
 'format': 'BlockdevDriver' } }

{ 'struct': 'SystemFirmwareBinaryFlashCode',
  'base': 'SystemFirmwareBinaryFlashFile' }

{ 'struct': 'SystemFirmwareBinaryFlashVars',
  'base': 'SystemFirmwareBinaryFlashFile',
  'data': { 'secure-boot-key-enroll': 'bool' } }

{ 'struct': 'SystemFirmwareBinaryFlash',
  'data': { 'code': 'SystemFirmwareBinaryFlashCode',
'vars': ['SystemFirmwareBinaryFlashVars' ] } }


## Discriminated struct for different loading approaches

{ 'union': 'SystemFirmwareBinary',
  'base': { 'device': 'SystemFirmwareDevice' },
  'discriminator': 'device',
  'data': { 'memory': 'SystemFirmwareBinaryMemory',
'kernel': 'SystemFirmwareBinaryKernel',
'flash': 'SystemFirmwareBinaryFlash' } }



{ 'struct' : 'SystemFirmwareTarget',
  'data': { 'architecture': 'SystemFirmwareArchitecture',
'machines': [ 'str' ] } }


{ 'struct' : 'SystemFirmware',
  'data'   : {
  'description'  : 'str',
  'type' : 'SystemFirmwareType',
  'binary'   : 'SystemFirmwareBinary',
  'targets'  : [ 'SystemFirmwareTarget' ],
  'features' : ['SystemFirmwareFeature'] } } 



# Examples:
#
# {
#'description': 'SeaBIOS 256k',
#'type': 'bios',
#'binary': {
#'type': 'memory',
#'filename': '/path/to/seabios/rom-256k',
#}
#'targets':  {
#'x86_64': [ "pc", "q35"],
#'i386': [ "pc", "q35"],
#}
#'features': ['acpi-s3', 'acpi-s5'],
# }
# {
#'description': 'SeaBIOS 128k',
#'type': 'bios',
#'binary': {
#'type': 'memory',
#'filename': '/path/to/seabios/rom-128k',
#}
#'targets':  {
#'x86_64': [ "isapc"],
#'i386': [ "isapc"],
#}
#'features': [],
# }
# {
#'description': 'OVMF',
#'type': 'uefi'
#'binary': {
#'type': 'flash',
#'code': {
#  'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd',
#  'format': 'raw',
#},
#'vars': [
#   {
#  'filename': '/usr/share/OVMF/OVMF_VARS.fd',
#  'format': 'raw',
#  'secure=boot-key-enroll': false,
#   },
#   {
#  'filename': '/usr/share/OVMF/OVMF_VARS.secboot.fd',
#  'format': 'raw',
#  'secure=boot-key-enroll': true,
#   }
# 

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

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

There also is (4) elf binary loadable with -kernel.  Not sure we should
include that case.  u-boot can be loaded that way.  The elf binary seems
to be more a side product of the build proccess, I always have both
u-boot (elf binary) and u-boot.bin (binary blob loadable with -bios).
So maybe we should put aside -kernel for now, and maybe reconsider once
a real need for it shows up.

So maybe Firmware{Device,Access,Mapping} should be replaced with a
FirmwareImageType [ 'single', 'code+vars', 'code+protectedvars' ] ?

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Tue, Apr 10, 2018 at 11:16:01AM +0200, Laszlo Ersek wrote:
> On 04/10/18 08:27, Gerd Hoffmann wrote:
> >   Hi,
> > 
> >> - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
> >> present and future, for SMM-requiring OVMF builds), but then you get
> >> into version sorting and similar mess. I considered fnmatch() --
> >> basically simple ? and * wildcards -- but that's not expressive enough.
> > 
> > I'd suggest whitelist with wildcards.  So the smm builds would get
> > "pc-q35-*".
> > 
> > libvirt knows about aliases, so it should be able to handle the "q35"
> > shortcut like "pc-q35-${latest}".
> > 
> > Or do you see another issue?
> 
> Well, one issue I see is version sorting; I should say "Q35 but no
> earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".
> 
> Anyway (also asking for Thomas's input here): if we run with your idea
> to refer to exact mapping methods / firmware *implementation* types that
> we know libvirt implements / supports as a "white box", do we still deem
> machine type identification necessary? Because, libvirt already knows
> (for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
> have to make a *reference* to that knowledge in the JSON file.

BTW, that's not quite correct - when libvirt handles the "smm" arg it
checks if machine type == q35, and  QEMU version >= 2.4.

It is *not* checking the version of the machine type. ie it will happily
use smm with  pc-q35-2.0, as long as QEMU version is 2.4. Perhaps this is
not quite right, but we don't try to parse the version number out of the
machine type, because we can't assume a specific format for the machine
type version part. eg version can be "2.4", or it can be "rhel-7.0.0"
or something else again on Ubuntu.

IMHO it would be valid to just keep life simple and only record the base
machine type name that can use the firmware ie "pc", "q35", and ignore
the fact that in some cases the firmware might require a specific version
of the machine type.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 10.04.2018 11:22, Laszlo Ersek wrote:
> On 04/10/18 09:33, Thomas Huth wrote:
[...]
>> Alternatively, what about providing some kind of "alias" or "nickname"
>> setting here, too? So the EDK2 builds would get
>> SystemFirmwareType="edk2" and "SystemFirmwareAlias="uefi" for example.
> 
> I hope I understand you right -- I think your suggestion ties in with my
> other email I just sent in this thread. So, we could tell libvirtd,
> "this firmware is of type 'UEFI', and you must use the 'ovmf_smm'
> mapping method to run it, with this file or that file as varstore template".
> 
> We could even describe the parameters for this or that mapping method
> structurally in the schema (in a discriminated union in QAPI JSON, or in
> an XSD choice element). For example, "ovmf" and "ovmf_smm" would both
> take "OvmfSplitFileOptions" -- a list of single varstore template files
> with feature enum contants attached  --, while "SeaBiosOptions" would be
> an empty structure.

Sorry, I've got no clue about ovmf_smm and the other things you've
mentioned here ;-)

> I feel the key question here is whether we are allowed to directly
> reference a mapping method we know libvirt implements. If we are, that
> makes things a lot clearer (and easier, I should hope).

Key question is maybe rather: Do you want to design / implement
something that is libvirt-only here, or rather something generic that
could also be used for other upper layer tools that do not use libvirt?
(... and looks like Daniel just had the same comment in another mail in
this thread ...)

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 10.04.2018 11:16, Laszlo Ersek wrote:
> On 04/10/18 08:27, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
>>> present and future, for SMM-requiring OVMF builds), but then you get
>>> into version sorting and similar mess. I considered fnmatch() --
>>> basically simple ? and * wildcards -- but that's not expressive enough.
>>
>> I'd suggest whitelist with wildcards.  So the smm builds would get
>> "pc-q35-*".
>>
>> libvirt knows about aliases, so it should be able to handle the "q35"
>> shortcut like "pc-q35-${latest}".
>>
>> Or do you see another issue?
> 
> Well, one issue I see is version sorting; I should say "Q35 but no
> earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".
> 
> Anyway (also asking for Thomas's input here): if we run with your idea
> to refer to exact mapping methods / firmware *implementation* types that
> we know libvirt implements / supports as a "white box", do we still deem
> machine type identification necessary? Because, libvirt already knows
> (for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
> have to make a *reference* to that knowledge in the JSON file.

I think you really need a way to specify the machine there. Latest
example from QEMU 2.12: We've now got two uboot binaries in the tree,
pc-bios/u-boot.e500 and pc-bios/u-boot-sam460-20100605.bin. Both are
uboot, both are for ppc, but u-boot.e500 only works with the "ppce500"
machine and the other one only works with the "sam460ex" machine. How
would you teach libvirt such a relationship without an explicit machine
type identification field there?

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Tue, Apr 10, 2018 at 11:16:01AM +0200, Laszlo Ersek wrote:
> On 04/10/18 08:27, Gerd Hoffmann wrote:
> >   Hi,
> > 
> >> - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
> >> present and future, for SMM-requiring OVMF builds), but then you get
> >> into version sorting and similar mess. I considered fnmatch() --
> >> basically simple ? and * wildcards -- but that's not expressive enough.
> > 
> > I'd suggest whitelist with wildcards.  So the smm builds would get
> > "pc-q35-*".
> > 
> > libvirt knows about aliases, so it should be able to handle the "q35"
> > shortcut like "pc-q35-${latest}".
> > 
> > Or do you see another issue?
> 
> Well, one issue I see is version sorting; I should say "Q35 but no
> earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".
> 
> Anyway (also asking for Thomas's input here): if we run with your idea
> to refer to exact mapping methods / firmware *implementation* types that
> we know libvirt implements / supports as a "white box", do we still deem
> machine type identification necessary? Because, libvirt already knows
> (for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
> have to make a *reference* to that knowledge in the JSON file.
> 
> And, really, this seems to reinforce my point that the schema should
> live in the libvirtd tree, not in the QEMU tree. In that case, perhaps
> it would be a better fit to work with an XSD, and firmware packages
> should install XML files? Personally I'm a lot more attracted to
> XML/XSD; I think the tooling is better too. I just don't see how QEMU is
> involved.

This is defining a set of metadata that is required to use various firmware
files in combination with QEMU, along with defining a mapping to QEMU command
line arguments and/or features. Essentially, while I wish everyone used
libvirt, libvirt is not the only thing that manages QEMU. This information is
relevant to anyone managing QEMU, so it doesn't belong in libvirt's realm,
it is clear QEMU is best placed to declare this information.


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 09:33, Thomas Huth wrote:
> On 09.04.2018 18:50, Laszlo Ersek wrote:
>> On 04/09/18 10:19, Gerd Hoffmann wrote:
> +{ 'enum' : 'SystemFirmwareType',
> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }

 The naming here is quite a bad mixture between firmware interface
 ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
 could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
 so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
>>>
>>> uboot for example implements uefi unterfaces too (dunno how complete,
>>> but reportly recent versions can run uefi shell and grub just fine).
>>
>> Indeed: when I was struggling with this enum type and tried to look for
>> more firmware types to add, my googling turned up the "UEFI on Top of
>> U-Boot" whitepaper, from Alex and Andreas :)
>>
>> Again, this reaches to the root of the problem: when a user creates a
>> new domain, using high-level tools, they just want to tick "UEFI". (Dan
>> has emphasized this to me several times, so I think I get the idea by
>> now, if not the full environment.) We cannot ask the user, "please be
>> more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"
> 
> But you are designing a rather low-level interface here, which should
> IMHO rather be precise than fuzzy. So should this "just want to tick
> UEFI" rather be handled in the high-level tools instead?
> 
> Alternatively, what about providing some kind of "alias" or "nickname"
> setting here, too? So the EDK2 builds would get
> SystemFirmwareType="edk2" and "SystemFirmwareAlias="uefi" for example.

I hope I understand you right -- I think your suggestion ties in with my
other email I just sent in this thread. So, we could tell libvirtd,
"this firmware is of type 'UEFI', and you must use the 'ovmf_smm'
mapping method to run it, with this file or that file as varstore template".

We could even describe the parameters for this or that mapping method
structurally in the schema (in a discriminated union in QAPI JSON, or in
an XSD choice element). For example, "ovmf" and "ovmf_smm" would both
take "OvmfSplitFileOptions" -- a list of single varstore template files
with feature enum contants attached  --, while "SeaBiosOptions" would be
an empty structure.

I feel the key question here is whether we are allowed to directly
reference a mapping method we know libvirt implements. If we are, that
makes things a lot clearer (and easier, I should hope).

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 10.04.2018 11:05, Daniel P. Berrangé wrote:
> On Mon, Apr 09, 2018 at 06:34:41PM +0200, Laszlo Ersek wrote:
>> On 04/09/18 09:26, Thomas Huth wrote:
>>>  Hi Laszlo,
>>>
>>> On 07.04.2018 02:01, 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.
>>> [...]
 +##
 +# @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' ] }
>>>
>>> This is not fully clear to me... what is this exactly good for? Is this
>>> a way to say how the firmware should be loaded, i.e. via "-bios",
>>> "-kernel" or "-pflash" parameter? If so, the term "memory" is quite
>>> misleading since files that are loaded via -bios can also end up in an
>>> emulated ROM chip.
>>
>> I threw in "-kernel" because, although it also (usually?) means
>> "memory", I expected people would want it separate.
> 
> What platform / scenario actually uses -kernel to load firmware.

I think uboot uses -kernel in certain cases, see e.g.:

https://balau82.wordpress.com/2010/03/10/u-boot-for-arm-on-qemu/

> If you
> have loaded firmware using -kernel, how do you then load the actual
> kernel ?

The kernel is then loaded from disk or network or another boot device.

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [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" 
> >> Cc: Alexander Graf 
> >> Cc: Ard Biesheuvel 
> >> Cc: David Gibson 
> >> Cc: Eric Blake 
> >> Cc: Gary Ching-Pang Lin 
> >> Cc: Gerd Hoffmann 
> >> Cc: Kashyap Chamarthy 
> >> Cc: Markus Armbruster 
> >> Cc: Michael Roth 
> >> Cc: Michal Privoznik 
> >> Cc: Peter Krempa 
> >> Cc: Peter Maydell 
> >> Cc: Thomas Huth 
> >> Signed-off-by: Laszlo Ersek 
> >> ---
> >>
> >> 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 ..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 p

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 08:27, Gerd Hoffmann wrote:
>   Hi,
> 
>> - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
>> present and future, for SMM-requiring OVMF builds), but then you get
>> into version sorting and similar mess. I considered fnmatch() --
>> basically simple ? and * wildcards -- but that's not expressive enough.
> 
> I'd suggest whitelist with wildcards.  So the smm builds would get
> "pc-q35-*".
> 
> libvirt knows about aliases, so it should be able to handle the "q35"
> shortcut like "pc-q35-${latest}".
> 
> Or do you see another issue?

Well, one issue I see is version sorting; I should say "Q35 but no
earlier than 2.4", and lexicographically, "2.11" sorts before "2.4".

Anyway (also asking for Thomas's input here): if we run with your idea
to refer to exact mapping methods / firmware *implementation* types that
we know libvirt implements / supports as a "white box", do we still deem
machine type identification necessary? Because, libvirt already knows
(for example) that "ovmf_smm" requires pc-q35-2.4 or later. So we just
have to make a *reference* to that knowledge in the JSON file.

And, really, this seems to reinforce my point that the schema should
live in the libvirtd tree, not in the QEMU tree. In that case, perhaps
it would be a better fit to work with an XSD, and firmware packages
should install XML files? Personally I'm a lot more attracted to
XML/XSD; I think the tooling is better too. I just don't see how QEMU is
involved.

Opinions please :)
Thanks!
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 08:18, Gerd Hoffmann wrote:
>   Hi,
> 
>>> uboot for example implements uefi unterfaces too (dunno how complete,
>>> but reportly recent versions can run uefi shell and grub just fine).
>>
>> Indeed: when I was struggling with this enum type and tried to look for
>> more firmware types to add, my googling turned up the "UEFI on Top of
>> U-Boot" whitepaper, from Alex and Andreas :)
> 
> In case you wanna play: uboot supports x86 qemu meanwhile, so you can
> try install u-boot.git-x86 from my firmware repo, then run
> "qemu-system-x86_64 -bios /usr/share/u-boot.git/x86/qemu-pc/u-boot.rom".
> 
> It certainly isn't a useful edk2 replacement atm.  It has no virtio
> drivers.  And even when using ide storage its not like it would happily
> boot a fedora live iso.  So I certainly wouldn't tag that as uefi today.
> That might change at some point in the future though.
> 
>> Again, this reaches to the root of the problem: when a user creates a
>> new domain, using high-level tools, they just want to tick "UEFI". (Dan
>> has emphasized this to me several times, so I think I get the idea by
>> now, if not the full environment.) We cannot ask the user, "please be
>> more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"
> 
> Well, in case the uefi support in u-boot is good enough some day then it
> doesn't matter to the user whenever uboot or edk2 boots the efi guest
> from disk/iso, right?

I believe that's correct.

Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Mon, Apr 09, 2018 at 06:50:12PM +0200, Laszlo Ersek wrote:
> On 04/09/18 10:19, Gerd Hoffmann wrote:
> >>> +{ 'enum' : 'SystemFirmwareType',
> >>> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> >>
> >> The naming here is quite a bad mixture between firmware interface
> >> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
> >> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
> >> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
> > 
> > uboot for example implements uefi unterfaces too (dunno how complete,
> > but reportly recent versions can run uefi shell and grub just fine).
> 
> Indeed: when I was struggling with this enum type and tried to look for
> more firmware types to add, my googling turned up the "UEFI on Top of
> U-Boot" whitepaper, from Alex and Andreas :)
> 
> Again, this reaches to the root of the problem: when a user creates a
> new domain, using high-level tools, they just want to tick "UEFI". (Dan
> has emphasized this to me several times, so I think I get the idea by
> now, if not the full environment.) We cannot ask the user, "please be
> more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"
> 
> Instead, each of those firmware images will have to come with a JSON
> document that states "uefi" in SystemFirmware.type, and the host admin
> will be responsible for establishing a priority order between them.
> Then, when the user asks for "UEFI" (and no more details), they'll get
> (compatibly with the target architecture) whichever firmware the host
> admin marked as "higher priority".

Yep, I don't think there's any problem here. If they have asked for
"uefi", they'll get whichever UEFI implementation is the default for
that host. Today it'll be an EDK2 impl, but if there's a uboot impl
of UEFI available instead, that's fine too. If both are available
we'll have some deterministic manner in which we pick one, even if it
as simple as which has alphabetically first filename.

This is really only about getting good default choices. If the user
really badly wants to have a specific firmware, they can still provide
a path to it themselves instead of having libvirt choose it.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 07:59, Gerd Hoffmann wrote:
>   Hi,
> 
>> I threw in "-kernel" because, although it also (usually?) means
>> "memory", I expected people would want it separate.
>>
>> Regarding memory vs. pflash, I thought that these two, combined with the
>> access permissions, could cover all of RAM, ROM, and read-only and
>> read-write pflash too.
>>
>> So, "-bios" (-> ROM) boils down to "memory", with write access denied --
>> please see the SeaBIOS example near the end.
> 
> 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.

So "ovmf" would refer precisely to point (3) in my email
<3381bdf1-62ea-9da7-c654-032c0c11fb4e@redhat.com">http://mid.mail-archive.com/3381bdf1-62ea-9da7-c654-032c0c11fb4e@redhat.com>,
and "ovmf_smm" would refer to point (4) in that email.

Let me post the next version soon with this idea, focusing just on OVMF
and maybe SeaBIOS. Then let us see if that RFCv2 format lends itself
easily to extensions by Thomas. :)

Thanks!
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Mon, Apr 09, 2018 at 06:34:41PM +0200, Laszlo Ersek wrote:
> On 04/09/18 09:26, Thomas Huth wrote:
> >  Hi Laszlo,
> > 
> > On 07.04.2018 02:01, 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.
> > [...]
> >> +##
> >> +# @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' ] }
> > 
> > This is not fully clear to me... what is this exactly good for? Is this
> > a way to say how the firmware should be loaded, i.e. via "-bios",
> > "-kernel" or "-pflash" parameter? If so, the term "memory" is quite
> > misleading since files that are loaded via -bios can also end up in an
> > emulated ROM chip.
> 
> I threw in "-kernel" because, although it also (usually?) means
> "memory", I expected people would want it separate.

What platform / scenario actually uses -kernel to load firmware. If you
have loaded firmware using -kernel, how do you then load the actual
kernel ?


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/10/18 09:44, Thomas Huth wrote:
> On 09.04.2018 18:34, Laszlo Ersek wrote:
>> On 04/09/18 09:26, Thomas Huth wrote:
>>>  Hi Laszlo,
>>>
>>> On 07.04.2018 02:01, 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.
>>> [...]
 +##
 +# @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' ] }
>>>
>>> This is not fully clear to me... what is this exactly good for? Is this
>>> a way to say how the firmware should be loaded, i.e. via "-bios",
>>> "-kernel" or "-pflash" parameter? If so, the term "memory" is quite
>>> misleading since files that are loaded via -bios can also end up in an
>>> emulated ROM chip.
>>
>> I threw in "-kernel" because, although it also (usually?) means
>> "memory", I expected people would want it separate.
>>
>> Regarding memory vs. pflash, I thought that these two, combined with the
>> access permissions, could cover all of RAM, ROM, and read-only and
>> read-write pflash too.
>>
>> So, "-bios" (-> ROM) boils down to "memory", with write access denied --
>> please see the SeaBIOS example near the end.
> 
> Let me ask the other way round: How does a high-level tool know whether
> it should use "-bios", "-kernel", "-pflash", "-device generic-loader" or
> "-younameit" for loading the firmware?

I expect it knows that because its developers investigate all the
supported firmware options and write dedicated code for those. My
understanding is that this JSON is not supposed to inform the mgmt layer
about unknown firmware, but to expose enough information for the mgmt
layer to pick a firmware and to compose a known & supported cmdline
config for it.


 +   'nvram-map' : 'FirmwareMapping',
 +   'templates' : [ 'FirmwareFile' ] } }
 +
 +##
 +# @SystemFirmwareType:
 +#
 +# Lists system firmware types commonly used with QEMU virtual machines.
 +#
 +# @bios: The system firmware was built from the SeaBIOS project.
 +#
 +# @slof: The system firmware was built from the Slimline Open Firmware 
 project.
 +#
 +# @uboot: The system firmware was built from the U-Boot project.
 +#
 +# @uefi: The system firmware was built from the edk2 (EFI Development Kit 
 II)
 +#project.
 +#
 +# Since: 2.13
 +##
 +{ 'enum' : 'SystemFirmwareType',
 +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>>
>>> The naming here is quite a bad mixture between firmware interface
>>> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
>>> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
>>> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
>>
>> Sure, I'm totally ready to follow community advice here (too).
>>
>> In fact this is the one element I dislike the most about the schema --
>> it's the fuzziest part, yet it is the most important element for
>> libvirt. Because users and higher level apps just want to say "give me
>> UEFI". If I have to ask "OK, but UEFI built from edk2 or something
>> else?", then it's a lost cause already.
>>
>> It's hard to find the right level of abstraction in the naming when the
>> higher level tools (and/or ultimately the users) don't know enough to
>> ask for specifics -- I'm not saying that's bad; it's quite natural, but
>> makes things very difficult. So this enum aims to match the user story
>> "gimme UEFI and be done with it". I figure users will just utter the
>> most common buzzword form of the concept they have in mind. "edk2"
>> doesn't tell them as much as "uefi".
> 
> OK, I see your point. But I still think we should not design fuzzy
> interfaces here at this low level, this will only lead to other trouble
> later. ... thinking about this again, users seem to mix up firmware
>

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 09.04.2018 18:34, Laszlo Ersek wrote:
> On 04/09/18 09:26, Thomas Huth wrote:
>>  Hi Laszlo,
>>
>> On 07.04.2018 02:01, 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.
>> [...]
>>> +##
>>> +# @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' ] }
>>
>> This is not fully clear to me... what is this exactly good for? Is this
>> a way to say how the firmware should be loaded, i.e. via "-bios",
>> "-kernel" or "-pflash" parameter? If so, the term "memory" is quite
>> misleading since files that are loaded via -bios can also end up in an
>> emulated ROM chip.
> 
> I threw in "-kernel" because, although it also (usually?) means
> "memory", I expected people would want it separate.
> 
> Regarding memory vs. pflash, I thought that these two, combined with the
> access permissions, could cover all of RAM, ROM, and read-only and
> read-write pflash too.
> 
> So, "-bios" (-> ROM) boils down to "memory", with write access denied --
> please see the SeaBIOS example near the end.

Let me ask the other way round: How does a high-level tool know whether
it should use "-bios", "-kernel", "-pflash", "-device generic-loader" or
"-younameit" for loading the firmware?

>>> +   'nvram-map' : 'FirmwareMapping',
>>> +   'templates' : [ 'FirmwareFile' ] } }
>>> +
>>> +##
>>> +# @SystemFirmwareType:
>>> +#
>>> +# Lists system firmware types commonly used with QEMU virtual machines.
>>> +#
>>> +# @bios: The system firmware was built from the SeaBIOS project.
>>> +#
>>> +# @slof: The system firmware was built from the Slimline Open Firmware 
>>> project.
>>> +#
>>> +# @uboot: The system firmware was built from the U-Boot project.
>>> +#
>>> +# @uefi: The system firmware was built from the edk2 (EFI Development Kit 
>>> II)
>>> +#project.
>>> +#
>>> +# Since: 2.13
>>> +##
>>> +{ 'enum' : 'SystemFirmwareType',
>>> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>
>> The naming here is quite a bad mixture between firmware interface
>> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
>> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
>> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
> 
> Sure, I'm totally ready to follow community advice here (too).
> 
> In fact this is the one element I dislike the most about the schema --
> it's the fuzziest part, yet it is the most important element for
> libvirt. Because users and higher level apps just want to say "give me
> UEFI". If I have to ask "OK, but UEFI built from edk2 or something
> else?", then it's a lost cause already.
> 
> It's hard to find the right level of abstraction in the naming when the
> higher level tools (and/or ultimately the users) don't know enough to
> ask for specifics -- I'm not saying that's bad; it's quite natural, but
> makes things very difficult. So this enum aims to match the user story
> "gimme UEFI and be done with it". I figure users will just utter the
> most common buzzword form of the concept they have in mind. "edk2"
> doesn't tell them as much as "uefi".

OK, I see your point. But I still think we should not design fuzzy
interfaces here at this low level, this will only lead to other trouble
later. ... thinking about this again, users seem to mix up firmware
interfaces / families with concrete implementations. So maybe we need
something like:

 { 'enum' : 'SystemFirmwareType',
   'data' : [ 'seabios', 'slof', 'uboot', 'edk2', 'openbios' ] }

*and* :

 { 'enum' : 'SystemFirmwareInterface',  /* or: 'SystemFirmwareFamily' */
   'data' : [ 'bios', 'uefi', 'openfirmware' ] }

Then a high level tool can check both and pick the best match?

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 09.04.2018 18:50, Laszlo Ersek wrote:
> On 04/09/18 10:19, Gerd Hoffmann wrote:
 +{ 'enum' : 'SystemFirmwareType',
 +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>>
>>> The naming here is quite a bad mixture between firmware interface
>>> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
>>> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
>>> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
>>
>> uboot for example implements uefi unterfaces too (dunno how complete,
>> but reportly recent versions can run uefi shell and grub just fine).
> 
> Indeed: when I was struggling with this enum type and tried to look for
> more firmware types to add, my googling turned up the "UEFI on Top of
> U-Boot" whitepaper, from Alex and Andreas :)
> 
> Again, this reaches to the root of the problem: when a user creates a
> new domain, using high-level tools, they just want to tick "UEFI". (Dan
> has emphasized this to me several times, so I think I get the idea by
> now, if not the full environment.) We cannot ask the user, "please be
> more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"

But you are designing a rather low-level interface here, which should
IMHO rather be precise than fuzzy. So should this "just want to tick
UEFI" rather be handled in the high-level tools instead?

Alternatively, what about providing some kind of "alias" or "nickname"
setting here, too? So the EDK2 builds would get
SystemFirmwareType="edk2" and "SystemFirmwareAlias="uefi" for example.

 Thomas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

  Hi,

> - I considered adding wildcards (say, blacklist "all" i440fx machtypes,
> present and future, for SMM-requiring OVMF builds), but then you get
> into version sorting and similar mess. I considered fnmatch() --
> basically simple ? and * wildcards -- but that's not expressive enough.

I'd suggest whitelist with wildcards.  So the smm builds would get
"pc-q35-*".

libvirt knows about aliases, so it should be able to handle the "q35"
shortcut like "pc-q35-${latest}".

Or do you see another issue?

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

  Hi,

> > uboot for example implements uefi unterfaces too (dunno how complete,
> > but reportly recent versions can run uefi shell and grub just fine).
> 
> Indeed: when I was struggling with this enum type and tried to look for
> more firmware types to add, my googling turned up the "UEFI on Top of
> U-Boot" whitepaper, from Alex and Andreas :)

In case you wanna play: uboot supports x86 qemu meanwhile, so you can
try install u-boot.git-x86 from my firmware repo, then run
"qemu-system-x86_64 -bios /usr/share/u-boot.git/x86/qemu-pc/u-boot.rom".

It certainly isn't a useful edk2 replacement atm.  It has no virtio
drivers.  And even when using ide storage its not like it would happily
boot a fedora live iso.  So I certainly wouldn't tag that as uefi today.
That might change at some point in the future though.

> Again, this reaches to the root of the problem: when a user creates a
> new domain, using high-level tools, they just want to tick "UEFI". (Dan
> has emphasized this to me several times, so I think I get the idea by
> now, if not the full environment.) We cannot ask the user, "please be
> more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"

Well, in case the uefi support in u-boot is good enough some day then it
doesn't matter to the user whenever uboot or edk2 boots the efi guest
from disk/iso, right?

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

  Hi,

> I threw in "-kernel" because, although it also (usually?) means
> "memory", I expected people would want it separate.
> 
> Regarding memory vs. pflash, I thought that these two, combined with the
> access permissions, could cover all of RAM, ROM, and read-only and
> read-write pflash too.
> 
> So, "-bios" (-> ROM) boils down to "memory", with write access denied --
> please see the SeaBIOS example near the end.

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.

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

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" 
>> Cc: Alexander Graf 
>> Cc: Ard Biesheuvel 
>> Cc: David Gibson 
>> Cc: Eric Blake 
>> Cc: Gary Ching-Pang Lin 
>> Cc: Gerd Hoffmann 
>> Cc: Kashyap Chamarthy 
>> Cc: Markus Armbruster 
>> Cc: Michael Roth 
>> Cc: Michal Privoznik 
>> Cc: Peter Krempa 
>> Cc: Peter Maydell 
>> Cc: Thomas Huth 
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> 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 ..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.

For example, OVMF can be loaded in a multitude of ways:

(1) "OVMF.fd" (a unified image that contains an executable and a live
variable store too) can be loaded with "-bios". This will place the full
image into ROM (that is FirmwareDevice=memory, read and exec: permitted,
write: denied). This will not provide a spec-compatible UEFI variable
service to the guest, but many people use OVMF like this. The libvirt
domain XML can accommodate this case:

  OVMF.fd

(2) "OVMF.fd" can be loaded into a single pflash chip (single pflash
drive, read/write). The command line switch is "-drive
if=pflash,format=raw,file=OVMF.fd,unit=0,readonly=off". This gives the
guest a spec-compliant UEFI variable service; however, the variable
store is inseparable from the firmware binary, and upgrading the latter
without losing the former is not possible, from a packaging perspective.
This maps to FirmwareDevice=flash, with all of
read/write/exec=permitted. Libvirt can describe this too in the domain XML:

  OVMF.fd

(3) Now we're coming to the split image, namely OVMF_CODE.fd, and a
*copy of* the OVMF_VARS.fd template. This is mapped with

  -drive if=pflash,format=raw,file=OVMF_CODE.fd,unit=0,readonly=on \
  -drive if=pflash,format=raw,file=varstore.fd,unit=1,readonly=off

To the guest, it looks like (2), but on the host

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/09/18 10:26, Gerd Hoffmann wrote:
>> +# {
>> +# "executable": {
>> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
>> +# "description": "OVMF with Secure Boot and SMM-protected varstore",
>> +# "tags": [
>> +# "FD_SIZE_4MB",
>> +# "IA32X64",
>> +# "SECURE_BOOT_ENABLE",
>> +# "SMM_REQUIRE"
>> +# ]
>> +# },
>> +# "type": "uefi",
>> +# "targets": [
>> +# "x86_64"
>> +# ],
>> +# "sysfw-map": {
>> +# "device": "flash",
>> +# "write": "denied"
>> +# },
>> +# "nvram-slots": [
>> +# {
>> +# "slot-id": 1,
>> +# "nvram-map" : {
>> +# "device": "flash",
>> +# "write": "restricted-to-secure-context"
>> +# },
> 
> What is "slot-id"?  The pflash index?

Yes, it might be defined like that, for the i440fx and q35 machine
types. This correspondence would be implemented in libvirtd, I suppose.

However, I don't think such a correspondence is mandatory. At first
approach, slot-id is just the key that tells the nvramslots apart.

> shouldn't we also specify the
> index for the executable somewhere?

Maybe :)

> Maybe the field should be moved
> into FirmwareMapping?

I couldn't come up with a good use case where you wouldn't map the
*system* firmware in a predefined pflash unit (or other device unit). So
I thought that needed no slot-id.

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/09/18 10:19, Gerd Hoffmann wrote:
>>> +{ 'enum' : 'SystemFirmwareType',
>>> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>
>> The naming here is quite a bad mixture between firmware interface
>> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
>> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
>> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.
> 
> uboot for example implements uefi unterfaces too (dunno how complete,
> but reportly recent versions can run uefi shell and grub just fine).

Indeed: when I was struggling with this enum type and tried to look for
more firmware types to add, my googling turned up the "UEFI on Top of
U-Boot" whitepaper, from Alex and Andreas :)

Again, this reaches to the root of the problem: when a user creates a
new domain, using high-level tools, they just want to tick "UEFI". (Dan
has emphasized this to me several times, so I think I get the idea by
now, if not the full environment.) We cannot ask the user, "please be
more specific, do you want UEFI from edk2, or UEFI on top of U-Boot?"

Instead, each of those firmware images will have to come with a JSON
document that states "uefi" in SystemFirmware.type, and the host admin
will be responsible for establishing a priority order between them.
Then, when the user asks for "UEFI" (and no more details), they'll get
(compatibly with the target architecture) whichever firmware the host
admin marked as "higher priority".

(Please be aware that with this argument, I'm trying to put myself into
Dan's shoes. It doesn't come *naturally* to me; in fact I'm viscerally
screaming inside at this amount of "fuzz". Personally I'd say, "I can
give you what you *say*, not what you *mean*, so *say* what you mean".
Apparently, that cannot work here, because what users mean is "UEFI" and
nothing more. I have to accept that.)

Thanks
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/09/18 10:08, Thomas Huth wrote:
> On 07.04.2018 02:01, Laszlo Ersek wrote:
>> Add a schema that describes the properties of virtual machine firmware.
> [...]
>> +##
>> +# @SystemFirmware:
>> +#
>> +# Describes a system firmware binary and any NVRAM slots that it requires.
>> +#
>> +# @executable: Identifies the platform firmware executable.
>> +#
>> +# @type: The type by which the system firmware is commonly known. This is 
>> the
>> +#main search key by which management software looks up a system
>> +#firmware image for a new domain.
>> +#
>> +# @targets: a non-empty list of target architectures that are capable of
>> +#   executing the system firmware
>> +#
>> +# @sysfw-map: the mapping requirements of the system firmware binary
>> +#
>> +# @nvram-slots: A list of NVRAM slots that are required by the system 
>> firmware.
>> +#   The @slot-id field must be unique across the list. 
>> Importantly,
>> +#   if any @FirmwareAccess is @restricted-to-secure-context in
>> +#   @sysfw-map or in any @nvram-map in @nvram-slots, then (a) 
>> the
>> +#   virtual machine configuration is required to emulate the 
>> secure
>> +#   code execution context (as defined for @targets), and (b) 
>> the
>> +#   virtual machine configuration is required to actually 
>> restrict
>> +#   the access in question to the secure execution context.
>> +#
>> +# @supports-uefi-secure-boot: Whether the system firmware implements the
>> +# software interfaces for UEFI Secure Boot, as
>> +# defined in the UEFI specification. If the 
>> field
>> +# is missing, its assumed value is 'false'.
>> +#
>> +# @supports-amd-sev: Whether the system firmware supports running under AMD
>> +#Secure Encrypted Virtualization, as specified in the 
>> AMD64
>> +#Architecture Programmer's Manual. If the field is 
>> missing,
>> +#its assumed value is 'false'.
>> +#
>> +# @supports-acpi-s3: Whether the system firmware supports S3 sleep (suspend 
>> to
>> +#RAM), as defined in the ACPI specification. If the 
>> field
>> +#is missing, its assumed value is 'false'.
>> +#
>> +# @supports-acpi-s4: Whether the system firmware supports S4 hibernation
>> +#(suspend to disk), as defined in the ACPI 
>> specification.
>> +#If the field is missing, its assumed value is 'false'.
>> +#
>> +# Since: 2.13
>> +#
>> +# Examples:
>> +#
>> +# {
>> +# "executable": {
>> +# "pathname": "/usr/share/seabios/bios-256k.bin",
>> +# "description": "SeaBIOS",
>> +# "tags": [
>> +# "CONFIG_ROM_SIZE=256"
>> +# ]
>> +# },
>> +# "type": "bios",
>> +# "targets": [
>> +# "i386",
>> +# "x86_64"
>> +# ],
>> +# "sysfw-map": {
>> +# "device": "memory",
>> +# "write": "denied"
>> +# },
>> +# "supports-acpi-s3": true,
>> +# "supports-acpi-s4": true
>> +# }
>> +#
>> +# {
>> +# "executable": {
>> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
>> +# "description": "OVMF with Secure Boot and SMM-protected varstore",
>> +# "tags": [
>> +# "FD_SIZE_4MB",
>> +# "IA32X64",
>> +# "SECURE_BOOT_ENABLE",
>> +# "SMM_REQUIRE"
>> +# ]
>> +# },
>> +# "type": "uefi",
>> +# "targets": [
>> +# "x86_64"
>> +# ],
>> +# "sysfw-map": {
>> +# "device": "flash",
>> +# "write": "denied"
>> +# },
>> +# "nvram-slots": [
>> +# {
>> +# "slot-id": 1,
>> +# "nvram-map" : {
>> +# "device": "flash",
>> +# "write": "restricted-to-secure-context"
>> +# },
>> +# "templates": [
>> +# {
>> +# "pathname": "/usr/share/OVMF/OVMF_VARS.fd",
>> +# "description": "empty varstore template"
>> +# },
>> +# {
>> +# "pathname": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
>> +# "description": "varstore template with the Microsoft 
>> certificates enrolled for Secure Boot",
>> +# "tags": [
>> +# "mscerts"
>> +# ]
>> +# }
>> +# ]
>> +# }
>> +# ],
>> +# "supports-uefi-secure-boot": true,
>> +# "supports-amd-sev": true,
>> +# "supports-acpi-s3": true
>> +# }
>> +#
>> +# {
>> +# "executable": {
>> +# "pathname": "/usr/share/AAVMF/AAVMF_CODE.fd",
>> +# "description": "ARM64 UEFI firmware",
>> +# "tags": [
>> +# "AARCH64"
>> +# ]
>> +# },
>> +# "type": "uefi",
>> +# "targets": [
>> +# "

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/09/18 09:26, Thomas Huth wrote:
>  Hi Laszlo,
> 
> On 07.04.2018 02:01, 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.
> [...]
>> +##
>> +# @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' ] }
> 
> This is not fully clear to me... what is this exactly good for? Is this
> a way to say how the firmware should be loaded, i.e. via "-bios",
> "-kernel" or "-pflash" parameter? If so, the term "memory" is quite
> misleading since files that are loaded via -bios can also end up in an
> emulated ROM chip.

I threw in "-kernel" because, although it also (usually?) means
"memory", I expected people would want it separate.

Regarding memory vs. pflash, I thought that these two, combined with the
access permissions, could cover all of RAM, ROM, and read-only and
read-write pflash too.

So, "-bios" (-> ROM) boils down to "memory", with write access denied --
please see the SeaBIOS example near the end.

> 
> [...]
>> +##
>> +# @NVRAMSlot:
>> +#
>> +# Defines the mapping properties of an NVRAM slot, and associates compatible
>> +# NVRAM templates with the NVRAM slot.
>> +#
>> +# @slot-id: The numeric identifier of the NVRAM slot. The interpretation of
>> +#   @slot-id is specific to the target architecture and the chosen
>> +#   system firmware.
>> +#
>> +# @nvram-map: the mapping requirements of this NVRAM slot
>> +#
>> +# @templates: A non-empty list of @FirmwareFile elements. Any @FirmwareFile
>> +# identified by this list as an NVRAM template can be copied to
>> +# create an actual NVRAM file, and the NVRAM file can be mapped
>> +# into the NVRAM slot identified by @slot-id, subject to the
>> +# mapping requirements in @nvram-map.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct' : 'NVRAMSlot',
>> +  'data'   : { 'slot-id'   : 'uint64',
> 
> Not sure whether I've got the idea here right, so take this with a grain
> of salt: Maybe 'uint64' is not flexible enough here. PAPR uses both, an
> ID and a name (string), see chapter 8.3 in
> https://members.openpowerfoundation.org/document/dl/469 ... but I guess
> we could start with the 'slot-id' here and add a name field later if
> necessary.

I only added "slot-id" as an initial place holder for telling apart the
individual NVRAMSlot elements in "SystemFirmware.nvram-slots". I
expected a scalar wouldn't be expressive enough for all arches, but, as
you say, it can be extended later.

> 
>> +   'nvram-map' : 'FirmwareMapping',
>> +   'templates' : [ 'FirmwareFile' ] } }
>> +
>> +##
>> +# @SystemFirmwareType:
>> +#
>> +# Lists system firmware types commonly used with QEMU virtual machines.
>> +#
>> +# @bios: The system firmware was built from the SeaBIOS project.
>> +#
>> +# @slof: The system firmware was built from the Slimline Open Firmware 
>> project.
>> +#
>> +# @uboot: The system firmware was built from the U-Boot project.
>> +#
>> +# @uefi: The system firmware was built from the edk2 (EFI Development Kit 
>> II)
>> +#project.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum' : 'SystemFirmwareType',
>> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> 
> The naming here is quite a bad mixture between firmware interface
> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.

Sure, I'm totally ready to follow community advice here (too).

In fact this is the one element I dislike the most about the schema --
it's the fuzziest part, yet it is the most important element for
libvirt. Because users and higher level apps just want to say "give me
UEFI". If I have to ask "OK, but UEFI built from edk2 or something
else?", then it's a lost cause already.

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Daniel P . Berrangé]

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" 
> Cc: Alexander Graf 
> Cc: Ard Biesheuvel 
> Cc: David Gibson 
> Cc: Eric Blake 
> Cc: Gary Ching-Pang Lin 
> Cc: Gerd Hoffmann 
> Cc: Kashyap Chamarthy 
> Cc: Markus Armbruster 
> Cc: Michael Roth 
> Cc: Michal Privoznik 
> Cc: Peter Krempa 
> Cc: Peter Maydell 
> Cc: Thomas Huth 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> 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 ..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 ?

> +
> +##
> +# @FirmwareMapping:
> +#
> +# Collects the mapping device type and the access permissions to that device
> +# for system firmware and for NVRAM slots.
> +#
> +# @device: The system firmware or the NVRAM slot must reside in a device of
> +#  this type.
> +#
> +# @read: Permission for the guest to read the device that maps the firmware
> +#file. If the field is missing, @permitted is assumed.
> +#
> +# @write: Permission for the guest to write the device that maps the firmware
> +# file. If the field is missing, @permitted is assumed.
> +#
> +# @execute: Permission for the guest to execute code from the device that 
> maps
> +#   the firmware file. If the field is missing, @permitted is 
> assumed.
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMapping',
> +  'data'   : { 'device'   : 'FirmwareDevice',
> +   '*read': 'FirmwareAccess',
> +   '*write'   : 'FirmwareAccess',
> +   '*execute' : 'FirmwareAccess' } }

Again, what this this map to on the command line ?

> +##
> +# @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 interpr

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

> +# {
> +# "executable": {
> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +# "description": "OVMF with Secure Boot and SMM-protected varstore",
> +# "tags": [
> +# "FD_SIZE_4MB",
> +# "IA32X64",
> +# "SECURE_BOOT_ENABLE",
> +# "SMM_REQUIRE"
> +# ]
> +# },
> +# "type": "uefi",
> +# "targets": [
> +# "x86_64"
> +# ],
> +# "sysfw-map": {
> +# "device": "flash",
> +# "write": "denied"
> +# },
> +# "nvram-slots": [
> +# {
> +# "slot-id": 1,
> +# "nvram-map" : {
> +# "device": "flash",
> +# "write": "restricted-to-secure-context"
> +# },

What is "slot-id"?  The pflash index?  shouldn't we also specify the
index for the executable somewhere?  Maybe the field should be moved
into FirmwareMapping?

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Gerd Hoffmann]

> > +{ 'enum' : 'SystemFirmwareType',
> > +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> 
> The naming here is quite a bad mixture between firmware interface
> ('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
> could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
> so I'd suggest to rather name them 'seabios' and 'edk2' here instead.

uboot for example implements uefi unterfaces too (dunno how complete,
but reportly recent versions can run uefi shell and grub just fine).

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

On 07.04.2018 02:01, Laszlo Ersek wrote:
> Add a schema that describes the properties of virtual machine firmware.
[...]
> +##
> +# @SystemFirmware:
> +#
> +# Describes a system firmware binary and any NVRAM slots that it requires.
> +#
> +# @executable: Identifies the platform firmware executable.
> +#
> +# @type: The type by which the system firmware is commonly known. This is the
> +#main search key by which management software looks up a system
> +#firmware image for a new domain.
> +#
> +# @targets: a non-empty list of target architectures that are capable of
> +#   executing the system firmware
> +#
> +# @sysfw-map: the mapping requirements of the system firmware binary
> +#
> +# @nvram-slots: A list of NVRAM slots that are required by the system 
> firmware.
> +#   The @slot-id field must be unique across the list. 
> Importantly,
> +#   if any @FirmwareAccess is @restricted-to-secure-context in
> +#   @sysfw-map or in any @nvram-map in @nvram-slots, then (a) the
> +#   virtual machine configuration is required to emulate the 
> secure
> +#   code execution context (as defined for @targets), and (b) the
> +#   virtual machine configuration is required to actually 
> restrict
> +#   the access in question to the secure execution context.
> +#
> +# @supports-uefi-secure-boot: Whether the system firmware implements the
> +# software interfaces for UEFI Secure Boot, as
> +# defined in the UEFI specification. If the field
> +# is missing, its assumed value is 'false'.
> +#
> +# @supports-amd-sev: Whether the system firmware supports running under AMD
> +#Secure Encrypted Virtualization, as specified in the 
> AMD64
> +#Architecture Programmer's Manual. If the field is 
> missing,
> +#its assumed value is 'false'.
> +#
> +# @supports-acpi-s3: Whether the system firmware supports S3 sleep (suspend 
> to
> +#RAM), as defined in the ACPI specification. If the field
> +#is missing, its assumed value is 'false'.
> +#
> +# @supports-acpi-s4: Whether the system firmware supports S4 hibernation
> +#(suspend to disk), as defined in the ACPI specification.
> +#If the field is missing, its assumed value is 'false'.
> +#
> +# Since: 2.13
> +#
> +# Examples:
> +#
> +# {
> +# "executable": {
> +# "pathname": "/usr/share/seabios/bios-256k.bin",
> +# "description": "SeaBIOS",
> +# "tags": [
> +# "CONFIG_ROM_SIZE=256"
> +# ]
> +# },
> +# "type": "bios",
> +# "targets": [
> +# "i386",
> +# "x86_64"
> +# ],
> +# "sysfw-map": {
> +# "device": "memory",
> +# "write": "denied"
> +# },
> +# "supports-acpi-s3": true,
> +# "supports-acpi-s4": true
> +# }
> +#
> +# {
> +# "executable": {
> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +# "description": "OVMF with Secure Boot and SMM-protected varstore",
> +# "tags": [
> +# "FD_SIZE_4MB",
> +# "IA32X64",
> +# "SECURE_BOOT_ENABLE",
> +# "SMM_REQUIRE"
> +# ]
> +# },
> +# "type": "uefi",
> +# "targets": [
> +# "x86_64"
> +# ],
> +# "sysfw-map": {
> +# "device": "flash",
> +# "write": "denied"
> +# },
> +# "nvram-slots": [
> +# {
> +# "slot-id": 1,
> +# "nvram-map" : {
> +# "device": "flash",
> +# "write": "restricted-to-secure-context"
> +# },
> +# "templates": [
> +# {
> +# "pathname": "/usr/share/OVMF/OVMF_VARS.fd",
> +# "description": "empty varstore template"
> +# },
> +# {
> +# "pathname": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
> +# "description": "varstore template with the Microsoft 
> certificates enrolled for Secure Boot",
> +# "tags": [
> +# "mscerts"
> +# ]
> +# }
> +# ]
> +# }
> +# ],
> +# "supports-uefi-secure-boot": true,
> +# "supports-amd-sev": true,
> +# "supports-acpi-s3": true
> +# }
> +#
> +# {
> +# "executable": {
> +# "pathname": "/usr/share/AAVMF/AAVMF_CODE.fd",
> +# "description": "ARM64 UEFI firmware",
> +# "tags": [
> +# "AARCH64"
> +# ]
> +# },
> +# "type": "uefi",
> +# "targets": [
> +# "aarch64"
> +# ],

Another thought: I think that "targets" field is not enough. You also
need to map firmware images to certain machines. For example, on
aarch64, the AAVMF firmware only works on t

Re: [libvirt] [qemu RFC] qapi: add "firmware.json"

[Thomas Huth]

 Hi Laszlo,

On 07.04.2018 02:01, 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.
[...]
> +##
> +# @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' ] }

This is not fully clear to me... what is this exactly good for? Is this
a way to say how the firmware should be loaded, i.e. via "-bios",
"-kernel" or "-pflash" parameter? If so, the term "memory" is quite
misleading since files that are loaded via -bios can also end up in an
emulated ROM chip.

[...]
> +##
> +# @NVRAMSlot:
> +#
> +# Defines the mapping properties of an NVRAM slot, and associates compatible
> +# NVRAM templates with the NVRAM slot.
> +#
> +# @slot-id: The numeric identifier of the NVRAM slot. The interpretation of
> +#   @slot-id is specific to the target architecture and the chosen
> +#   system firmware.
> +#
> +# @nvram-map: the mapping requirements of this NVRAM slot
> +#
> +# @templates: A non-empty list of @FirmwareFile elements. Any @FirmwareFile
> +# identified by this list as an NVRAM template can be copied to
> +# create an actual NVRAM file, and the NVRAM file can be mapped
> +# into the NVRAM slot identified by @slot-id, subject to the
> +# mapping requirements in @nvram-map.
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'NVRAMSlot',
> +  'data'   : { 'slot-id'   : 'uint64',

Not sure whether I've got the idea here right, so take this with a grain
of salt: Maybe 'uint64' is not flexible enough here. PAPR uses both, an
ID and a name (string), see chapter 8.3 in
https://members.openpowerfoundation.org/document/dl/469 ... but I guess
we could start with the 'slot-id' here and add a name field later if
necessary.

> +   'nvram-map' : 'FirmwareMapping',
> +   'templates' : [ 'FirmwareFile' ] } }
> +
> +##
> +# @SystemFirmwareType:
> +#
> +# Lists system firmware types commonly used with QEMU virtual machines.
> +#
> +# @bios: The system firmware was built from the SeaBIOS project.
> +#
> +# @slof: The system firmware was built from the Slimline Open Firmware 
> project.
> +#
> +# @uboot: The system firmware was built from the U-Boot project.
> +#
> +# @uefi: The system firmware was built from the edk2 (EFI Development Kit II)
> +#project.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'SystemFirmwareType',
> +  'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }

The naming here is quite a bad mixture between firmware interface
('bios', 'uefi') and firmware implementations ('slof', 'uboot'). There
could be other implementations of BIOS or UEFI than SeaBIOS and EDK2 ...
so I'd suggest to rather name them 'seabios' and 'edk2' here instead.

QEMU also ships with a couple of OpenBIOS images, so you should include
that in this list here, too.

And since the SystemFirmwareType field in the SystemFirmware struct
below is not optional, you should also include an 'other' type here. Or
make the SystemFirmwareType in SystemFirmware optional. Otherwise it's
not possible to specify other firmware implementations with this scheme.

> +##
> +# @SystemFirmware:
> +#
> +# Describes a system firmware binary and any NVRAM slots that it requires.
> +#
> +# @executable: Identifies the platform firmware executable.
> +#
> +# @type: The type by which the system firmware is commonly known. This is the
> +#main search key by which management software looks up a system
> +#firmware image for a new domain.
> +#
> +# @targets: a non-empty list of target architectures that are capable of
> +#   executing the system firmware
> +#
> +# @sysfw-map: the mapping requirements of the system firmware binary
> +#
> +# @nvram-slots: A list of NVRAM slots that are required by the system 
> firmware.
> +#   The @slot-id field must be unique across the list. 
> Importantly,
> +#   if any @Firmware

[libvirt] [qemu RFC] qapi: add "firmware.json"

[Laszlo Ersek]

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" 
Cc: Alexander Graf 
Cc: Ard Biesheuvel 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gary Ching-Pang Lin 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: Michal Privoznik 
Cc: Peter Krempa 
Cc: Peter Maydell 
Cc: Thomas Huth 
Signed-off-by: Laszlo Ersek 
---

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/Makefile b/Makefile
index 727ef118f3d9..e088e3c1b39f 100644
--- a/Makefile
+++ b/Makefile
@@ -97,6 +97,7 @@ GENERATED_FILES += qapi/qapi-types-block.h 
qapi/qapi-types-block.c
 GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c
 GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c
 GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
+GENERATED_FILES += qapi/qapi-types-firmware.h qapi/qapi-types-firmware.c
 GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
 GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
 GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c
@@ -115,6 +116,7 @@ GENERATED_FILES += qapi/qapi-visit-block.h 
qapi/qapi-visit-block.c
 GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c
 GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c
 GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
+GENERATED_FILES += qapi/qapi-visit-firmware.h qapi/qapi-visit-firmware.c
 GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c
 GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c
 GENERATED_FILES += qapi/qapi-visit-misc.h qapi/qapi-visit-misc.c
@@ -132,6 +134,7 @@ GENERATED_FILES += qapi/qapi-commands-block.h 
qapi/qapi-commands-block.c
 GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c
 GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c
 GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c
+GENERATED_FILES += qapi/qapi-commands-firmware.h qapi/qapi-commands-firmware.c
 GENERATED_FILES += qapi/qapi-commands-introspect.h 
qapi/qapi-commands-introspect.c
 GENERATED_FILES += qapi/qapi-commands-migration.h 
qapi/qapi-commands-migration.c
 GENERATED_FILES += qapi/qapi-commands-misc.h qapi/qapi-commands-misc.c
@@ -149,6 +152,7 @@ GENERATED_FILES += qapi/qapi-events-block.h 
qapi/qapi-events-block.c
 GENERATED_FILES += qapi/qapi-events-char.h qapi/qapi-events-char.c
 GENERATED_FILES += qapi/qapi-events-common.h qapi/qapi-events-common.c
 GENERATED_FILES += qapi/qapi-events-crypto.h qapi/qapi-events-crypto.c
+GENERATED_FILES += qapi/qapi-events-firmware.h qapi/qapi-events-firmware.c
 GENERATED_FILES += qapi/qapi-events-introspect.h qapi/qapi-events-introspect.c
 GENERATED_FILES += qapi/qapi-events-migration.h qapi/qapi-events-migration.c
 GENERATED_FILES += qapi/qapi-events-misc.h qapi/qapi-events-misc.c
@@ -581,6 +585,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
$(SRC_PATH)/qapi/char.json \
$(SRC_PATH)/qapi/crypto.json \
+   $(SRC_PATH)/qapi/firmware.json \
$(SRC_PATH)/qapi/introspect.json \
$(SRC_PATH)/qapi/migration.json \
$(SRC_PATH)/qapi/misc.json \
@@ -600,6 +605,7 @@ qapi/qapi-types-block.c qapi/qapi-types-block.h \
 qapi/qapi-types-char.c qapi/qapi-types-char.h \
 qapi/qapi-types-common.c qapi/qapi-types-common.h \
 qapi/qapi-types-crypto.c qapi/qapi-types-crypto.h \
+qapi/qapi-types-firmware.c qapi/qapi-types-firmware.h \
 qapi/qapi-types-introspect.c qapi/qapi-types-introspect.h \
 qapi/qapi-types-migration.c qapi/qapi-types-migration.h \
 qapi/qapi-types-misc.c qapi/qapi-types-misc.h \
@@ -618,6 +624,7 @@ qapi/qapi-visit-block.c qapi/qapi-visit-block.h \
 qapi/qa