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

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

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

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

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

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

Sure thing.

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.

That's actually a feature, not a bug. Whenever a new firmware *type* surfaces in our communities, such that libvirt should integrate it, it has to become addressible by a simple "type" moniker. That's the entire goal of this exercise -- the initial domain XML wants to say "gimme FOOBAR", and FOOBAR it shall be, whatever it takes. Not "other". Nobody will say 'give me a VM with "other" 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" +# ], +# "sysfw-map": { +# "device": "flash", +# "write": "denied" +# }, +# "nvram-slots": [ +# { +# "slot-id": 1, +# "nvram-map" : { +# "device": "flash" +# }, +# "templates": [ +# { +# "pathname": "/usr/share/AAVMF/AAVMF_VARS.fd", +# "description": "empty varstore template" +# } +# ] +# } +# ] +# } +# +# { +# "executable": { +# "pathname": "/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd", +# "description": "32-bit OVMF with unprotected varstore and no Secure Boot", +# "tags": [ +# "FD_SIZE_2MB", +# "IA32" +# ] +# }, +# "type": "uefi", +# "targets": [ +# "i386", +# "x86_64" +# ], +# "sysfw-map": { +# "device": "flash", +# "write": "denied" +# }, +# "nvram-slots": [ +# { +# "slot-id": 1, +# "nvram-map" : { +# "device": "flash" +# }, +# "templates": [ +# { +# "pathname": "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd", +# "description": "empty varstore template" +# } +# ] +# } +# ], +# "supports-acpi-s3": true +# } +## +{ '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' } } + +## +# @x-check-firmware: +# +# Accept a @SystemFirmware object and do nothing, successfully. This command +# can be used in the QMP shell to validate @SystemFirmware JSON against the +# schema, and to pretty print it. +# +# @sysfw: ignored +# +# Since: 2.13 +## +{ 'command' : 'x-check-firmware', + 'data' : { 'sysfw' : 'SystemFirmware' } }

Maybe it would be more useful to have a way to load a firmware image via such a json file? E.g. run "qemu-system-x86_64 -bios seabios.json" or something similar?

I think that's very much out of scope... for me anyway :) I believe (but I could be wrong) that the *only* reason I posted this patch for QEMU is that QEMU is the one project with schema administration / coordination. I don't really think QEMU actually has to implement any features around this, it just has to manage the schema and ensure well-formedness. I remain somewhat unconvinced QEMU is the best place for this schema, but I couldn't suggest anything better. Basically the producers of JSON documents that conform to this schema are the firmware packages that get installed on host systems. The consumer is libvirtd (to my understanding), and through libvirtd, higher level applications. I don't fully know with what project that leaves us for hosting the schema itself. I originally thought QEMU had nothing to do with it, but that seemed counter to the consensus :)

Or a QAPI command that can be used to load a firmware image this way? (which would only work before the guest has been started of course)

Thanks! Laszlo

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

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

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

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

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 <ard.biesheuvel@linaro.org> 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 <ard.biesheuvel@linaro.org> Acked-by: Laszlo Ersek <lersek@redhat.com> 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

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": [ +# "aarch64" +# ],

Another thought: I think that "targets" field is not enough. You also need to map firmware images to certain machines.

I expected this :)

For example, on aarch64, the AAVMF firmware only works on the "virt" machine, but not on "raspi2" (I guess). On ppc64, SLOF only works for "pseries", but not for the other machines like "mac99" or "g3beige". And on x86_64, EDK2 only works for "q35" and "pc-i440fx", but not for "isapc".

What's more, the SMM driver stack of edk2, when it's built into OVMF (x86), doesn't work on i440fx at all; it requires Q35, and even from that, a 2.4+ version (if memory serves). Here's my problem with machine types in this schema (because, believe it or not, I added such element(s) at first, but then gave up and killed them): - if you add a *whitelist* of compatible machine types, then every time you install a new QEMU version on the host (with a new, most-recent, machine type), that machine type will not be on the whitelist (because, remember, the JSON document comes with the firmware package). So, you'll have to keep extending the JSON installed with the firmware image. Bad. - if you add a *blacklist* of incompatible machine types, then the same issue arises -- a new i440fx machine type, or even a completely new machine type family, have to be blacklisted when they come out. They should not be assumed SMM-compatible, for example, until proved so. - 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. So, my thinking was, all these "smarts" belong in actual libvirtd code (C code), and the schema should only allow libvirtd to *recognize* what is what. Thanks, Laszlo

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

On 10/04/2018 11:23, Daniel P. Berrangé wrote:

...

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.

QEMU is best placed to _standardize_ how to provide this information (and where in the file system to place it), but really it's up to firmware packages to provide it. We can of course define the schema in QAPI terms for ease of validation (machine-readable specs are nice to have), but really this should just be a file in docs/interop/. No code is needed in QEMU. Paolo

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

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

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

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

On Tue, Apr 10, 2018 at 12:48:28PM +0100, Peter Maydell wrote:

On 10 April 2018 at 12:34, Daniel P. Berrangé <berrange@redhat.com> 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 :|

On Tue, Apr 10, 2018 at 11:20:33AM +0100, 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.

...

diff --git a/qapi/firmware.json b/qapi/firmware.json new file mode 100644 index 000000000000..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, # }

It occurs to me that we are actually over-thinking things, by making it possible to list a choice of vars files per firmware. We could remove this special case by just having separate tpo level firmware entries and a main feature flag to say if it is enrolled or not - see below example

# ], # }, # 'targets': { # 'x86_64': [ "q35"], # } # 'features': ['acpi-s3', 'acpi-s5', 'secure-boot'], # } #

{ 'description': 'OVMF secboot', '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', }, }, 'targets': { 'x86_64': [ "q35"], } 'features': ['acpi-s3', 'acpi-s5', 'secure-boot'], } { 'description': 'OVMF secboot enrolled', 'type': 'uefi' 'binary': { 'type': 'flash', 'code': { 'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd', 'format': 'raw', }, 'vars': { 'filename': '/usr/share/OVMF/OVMF_VARS.secboot.fd', 'format': 'raw', } }, 'targets': { 'x86_64': [ "q35"], } 'features': ['acpi-s3', 'acpi-s5', 'secure-boot', "secure-boot-enrolled-keys"], } Avoiding recording the notion of secureboot enrollment against the VARs files, means that you have more flexibility. One could just have a single file containing both CODE+VARS, which is enrolled instead of separating them. 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 :|

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