Makefile | 9 + Makefile.objs | 4 + qapi/firmware.json | 503 ++++++++++++++++++++++++++++++++++++++++++++++++++ qapi/qapi-schema.json | 1 + qmp.c | 5 + .gitignore | 4 + 6 files changed, 526 insertions(+) create mode 100644 qapi/firmware.json
Add a schema that describes the different uses and properties of virtual
machine firmware.
Each firmware executable installed on a host system should come with at
least one JSON file that conforms to this schema. Each file informs the
management applications about the firmware's properties and one possible
use case / feature set.
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 description 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 for a
certain use case or feature set.
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Gibson <dgibson@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Kashyap Chamarthy <kchamart@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Michal Privoznik <mprivozn@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Krempa <pkrempa@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
RFCv2:
- previous version (RFCv1) was posted at
<http://mid.mail-archive.com/20180407000117.25640-1-lersek@redhat.com>
- v2 is basically a rewrite from scratch, starting out with Dan's
definitions from
<http://mid.mail-archive.com/20180410102033.GL5155@redhat.com> and
<http://mid.mail-archive.com/20180410110357.GP5155@redhat.com>,
hopefully addressing others' feedback as well
RFCv1:
- 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 | 503 ++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/qapi-schema.json | 1 +
qmp.c | 5 +
.gitignore | 4 +
6 files changed, 526 insertions(+)
create mode 100644 qapi/firmware.json
diff --git a/Makefile b/Makefile
index d71dd5bea416..32034abe1583 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/qapi-visit-char.c qapi/qapi-visit-char.h \
qapi/qapi-visit-common.c qapi/qapi-visit-common.h \
qapi/qapi-visit-crypto.c qapi/qapi-visit-crypto.h \
+qapi/qapi-visit-firmware.c qapi/qapi-visit-firmware.h \
qapi/qapi-visit-introspect.c qapi/qapi-visit-introspect.h \
qapi/qapi-visit-migration.c qapi/qapi-visit-migration.h \
qapi/qapi-visit-misc.c qapi/qapi-visit-misc.h \
@@ -635,6 +642,7 @@ qapi/qapi-commands-block.c qapi/qapi-commands-block.h \
qapi/qapi-commands-char.c qapi/qapi-commands-char.h \
qapi/qapi-commands-common.c qapi/qapi-commands-common.h \
qapi/qapi-commands-crypto.c qapi/qapi-commands-crypto.h \
+qapi/qapi-commands-firmware.c qapi/qapi-commands-firmware.h \
qapi/qapi-commands-introspect.c qapi/qapi-commands-introspect.h \
qapi/qapi-commands-migration.c qapi/qapi-commands-migration.h \
qapi/qapi-commands-misc.c qapi/qapi-commands-misc.h \
@@ -652,6 +660,7 @@ qapi/qapi-events-block.c qapi/qapi-events-block.h \
qapi/qapi-events-char.c qapi/qapi-events-char.h \
qapi/qapi-events-common.c qapi/qapi-events-common.h \
qapi/qapi-events-crypto.c qapi/qapi-events-crypto.h \
+qapi/qapi-events-firmware.c qapi/qapi-events-firmware.h \
qapi/qapi-events-introspect.c qapi/qapi-events-introspect.h \
qapi/qapi-events-migration.c qapi/qapi-events-migration.h \
qapi/qapi-events-misc.c qapi/qapi-events-misc.h \
diff --git a/Makefile.objs b/Makefile.objs
index c6c9b8fc2177..6ed4e0010b10 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,7 @@ util-obj-y += qapi/qapi-types-block.o
util-obj-y += qapi/qapi-types-char.o
util-obj-y += qapi/qapi-types-common.o
util-obj-y += qapi/qapi-types-crypto.o
+util-obj-y += qapi/qapi-types-firmware.o
util-obj-y += qapi/qapi-types-introspect.o
util-obj-y += qapi/qapi-types-migration.o
util-obj-y += qapi/qapi-types-misc.o
@@ -27,6 +28,7 @@ util-obj-y += qapi/qapi-visit-block.o
util-obj-y += qapi/qapi-visit-char.o
util-obj-y += qapi/qapi-visit-common.o
util-obj-y += qapi/qapi-visit-crypto.o
+util-obj-y += qapi/qapi-visit-firmware.o
util-obj-y += qapi/qapi-visit-introspect.o
util-obj-y += qapi/qapi-visit-migration.o
util-obj-y += qapi/qapi-visit-misc.o
@@ -44,6 +46,7 @@ util-obj-y += qapi/qapi-events-block.o
util-obj-y += qapi/qapi-events-char.o
util-obj-y += qapi/qapi-events-common.o
util-obj-y += qapi/qapi-events-crypto.o
+util-obj-y += qapi/qapi-events-firmware.o
util-obj-y += qapi/qapi-events-introspect.o
util-obj-y += qapi/qapi-events-migration.o
util-obj-y += qapi/qapi-events-misc.o
@@ -139,6 +142,7 @@ common-obj-y += qapi/qapi-commands-block.o
common-obj-y += qapi/qapi-commands-char.o
common-obj-y += qapi/qapi-commands-common.o
common-obj-y += qapi/qapi-commands-crypto.o
+common-obj-y += qapi/qapi-commands-firmware.o
common-obj-y += qapi/qapi-commands-introspect.o
common-obj-y += qapi/qapi-commands-migration.o
common-obj-y += qapi/qapi-commands-misc.o
diff --git a/qapi/firmware.json b/qapi/firmware.json
new file mode 100644
index 000000000000..3653b4628a5b
--- /dev/null
+++ b/qapi/firmware.json
@@ -0,0 +1,503 @@
+# -*- Mode: Python -*-
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# Authors:
+# Daniel P. Berrange <berrange@redhat.com>
+# Laszlo Ersek <lersek@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later. See
+# the COPYING file in the top-level directory.
+
+##
+# = Firmware
+##
+
+{ 'include' : 'block-core.json' }
+
+##
+# @FirmwareType:
+#
+# Lists firmware types commonly used with QEMU virtual machines.
+#
+# @bios: The firmware was built from the SeaBIOS project.
+#
+# @slof: The firmware was built from the Slimline Open Firmware project.
+#
+# @uboot: The firmware was built from the U-Boot project.
+#
+# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
+#
+# Since: 2.13
+##
+{ 'enum' : 'FirmwareType',
+ 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
+
+##
+# @FirmwareDevice:
+#
+# Defines the device types that firmware can be mapped into.
+#
+# @flash: The firmware executable and its accompanying NVRAM file are to be
+# mapped into a pflash chip each.
+#
+# @kernel: The firmware 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 and machine type.
+#
+# @memory: The firmware is to be mapped into memory.
+#
+# Since: 2.13
+##
+{ 'enum' : 'FirmwareDevice',
+ 'data' : [ 'flash', 'kernel', 'memory' ] }
+
+##
+# @FirmwareArchitecture:
+#
+# Defines the target architectures (emulator binaries) that firmware may
+# execute on.
+#
+# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
+#
+# @arm: The firmware can be executed by the qemu-system-arm emulator.
+#
+# @i386: The firmware can be executed by the qemu-system-i386 emulator.
+#
+# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
+#
+# Since: 2.13
+##
+{ 'enum' : 'FirmwareArchitecture',
+ 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
+
+##
+# @FirmwareTarget:
+#
+# Defines the machine types that firmware may execute on.
+#
+# @architecture: Determines the emulation target (the QEMU system emulator)
+# that can execute the firmware.
+#
+# @machines: Lists the machine types (known by the emulator that is specified
+# through @architecture) that can execute the firmware. Elements of
+# @machines are not supposed to be versioned; if a machine type is
+# versioned in QEMU (e.g. "pc-i440fx-2.12"), then its unversioned
+# variant, which typically refers to the latest version (e.g. "pc"),
+# should be listed in @machines. On the QEMU command line, "-machine
+# type=..." specifies the requested machine type.
+#
+# Since: 2.13
+##
+{ 'struct' : 'FirmwareTarget',
+ 'data' : { 'architecture' : 'FirmwareArchitecture',
+ 'machines' : [ 'str' ] } }
+
+##
+# @FirmwareFeature:
+#
+# Defines the features that firmware may support, and the platform requirements
+# that firmware may present.
+#
+# @acpi-s3: The firmware supports S3 sleep (suspend to RAM), as defined in the
+# ACPI specification. On the "pc" machine type of the @i386 and
+# @x86_64 emulation targets, S3 can be enabled with "-global
+# PIIX4_PM.disable_s3=0" and disabled with "-global
+# PIIX4_PM.disable_s3=1". On the "q35" machine type of the @i386 and
+# @x86_64 emulation targets, S3 can be enabled with "-global
+# ICH9-LPC.disable_s3=0" and disabled with "-global
+# ICH9-LPC.disable_s3=1".
+#
+# @acpi-s4: The firmware supports S4 hibernation (suspend to disk), as defined
+# in the ACPI specification. On the "pc" machine type of the @i386
+# and @x86_64 emulation targets, S4 can be enabled with "-global
+# PIIX4_PM.disable_s4=0" and disabled with "-global
+# PIIX4_PM.disable_s4=1". On the "q35" machine type of the @i386 and
+# @x86_64 emulation targets, S4 can be enabled with "-global
+# ICH9-LPC.disable_s4=0" and disabled with "-global
+# ICH9-LPC.disable_s4=1".
+#
+# @amd-sev: The firmware supports running under AMD Secure Encrypted
+# Virtualization, as specified in the AMD64 Architecture Programmer's
+# Manual. QEMU command line options related to this feature are
+# documented in "docs/amd-memory-encryption.txt".
+#
+# @requires-smm: The firmware requires the platform to emulate SMM (System
+# Management Mode), as defined in the AMD64 Architecture
+# Programmer's Manual, and in the Intel(R)64 and IA-32
+# Architectures Software Developer's Manual. On the "q35"
+# machine type of the @i386 and @x86_64 emulation targets, SMM
+# emulation can be enabled with "-machine smm=on". (On the "q35"
+# machine type of the @i386 emulation target, @requires-smm
+# presents further CPU requirements; one combination known to
+# work is "-cpu coreduo,-nx".) If the firmware is marked as both
+# @secure-boot and @requires-smm, then write accesses to the
+# pflash chip (NVRAM) that holds the UEFI variable store must be
+# restricted to code that executes in SMM, using the additional
+# option "-global driver=cfi.pflash01,property=secure,value=on".
+# Furthermore, a large guest-physical address space (comprising
+# guest RAM, memory hotplug range, and 64-bit PCI MMIO
+# aperture), and/or a high VCPU count, may present high SMRAM
+# requirements from the firmware. On the "q35" machine type of
+# the @i386 and @x86_64 emulation targets, the SMRAM size may be
+# increased above the default 16MB with the "-global
+# mch.extended-tseg-mbytes=uint16" option. As a rule of thumb,
+# the default 16MB size suffices for 1TB of guest-phys address
+# space and a few tens of VCPUs; for every further TB of
+# guest-phys address space, add 8MB of SMRAM. 48MB should
+# suffice for 4TB of guest-phys address space and 2-3 hundred
+# VCPUs.
+#
+# @secure-boot: The firmware implements the software interfaces for UEFI Secure
+# Boot, as defined in the UEFI specification. Note that without
+# @requires-smm, guest code running with kernel privileges can
+# undermine the security of Secure Boot.
+#
+# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated
+# with the firmware binary has the Secure Boot
+# operational mode enabled, and -- at least -- the
+# following certificates enrolled. (1) As Platform
+# Key (PK), and as one Key Exchange Key (KEK), a
+# self-signed certificate issued by the firmware
+# distributor is enrolled. (2) As another Key
+# Exchange Key (KEK), the "Microsoft Corporation
+# KEK CA 2011" certificate is enrolled. The UEFI
+# Forum releases updates for the Secure Boot
+# Signature/Certificate Blacklist ("dbx")
+# periodically at
+# <http://www.uefi.org/revocationlistfile>, signed
+# with a certificate chain anchored in this
+# certificate. (3) As one Secure Boot
+# Signature/Certificate ("db"), the "Microsoft
+# Windows Production PCA 2011" certificate is
+# enrolled. This certificate verifies Windows 8,
+# Windows Server 2012 R2, etc boot loaders. (4) As
+# another Secure Boot Signature/Certificate ("db"),
+# the "Microsoft Corporation UEFI CA 2011"
+# certificate is enrolled. This certificate
+# verifies the "shim" UEFI application, and PCI
+# expansion ROMs. @secure-boot-enrolled-keys is
+# only valid if the firmware also supports
+# @secure-boot.
+#
+# @verbose-dynamic: When firmware log capture is enabled, the firmware logs a
+# large amount of debug messages, which may impact boot
+# performance. With log capture disabled, there is no boot
+# performance impact. On the "pc" and "q35" machine types of
+# the @i386 and @x86_64 emulation targets, firmware log
+# capture can be enabled with the QEMU command line options
+# "-chardev file,id=fwdebug,path=LOGFILEPATH -device
+# isa-debugcon,iobase=0x402,chardev=fwdebug".
+# @verbose-dynamic is mutually exclusive with
+# @verbose-static.
+#
+# @verbose-static: The firmware unconditionally produces a large amount of
+# debug messages, which may impact boot performance. This
+# feature may typically be carried by certain UEFI firmware
+# for the "virt" machine type of the @arm and @aarch64
+# emulation targets, where the debug messages are written to
+# the first (always present) PL011 UART. @verbose-static is
+# mutually exclusive with @verbose-dynamic.
+#
+# Since: 2.13
+##
+{ 'enum' : 'FirmwareFeature',
+ 'data' : [ 'acpi-s3', 'acpi-s4', 'amd-sev', 'requires-smm', 'secure-boot',
+ 'secure-boot-enrolled-keys', 'verbose-dynamic',
+ 'verbose-static' ] }
+
+##
+# @FirmwareFlashFile:
+#
+# Defines common properties that are necessary for loading a firmware file into
+# a pflash chip. The corresponding QEMU command line option is "-drive
+# file=@pathname,format=@format". Note however that the option-argument shown
+# here is incomplete; it is completed under @FirmwareMappingFlash.
+#
+# @pathname: Specifies the pathname on the host filesystem where the firmware
+# file can be found.
+#
+# @format: Specifies the block format of the file pointed-to by @pathname, such
+# as @raw or @qcow2.
+#
+# Since: 2.13
+##
+{ 'struct' : 'FirmwareFlashFile',
+ 'data' : { 'pathname' : 'str',
+ 'format' : 'BlockdevDriver' } }
+
+##
+# @FirmwareMappingFlash:
+#
+# Describes loading and mapping properties for the firmware executable and its
+# accompanying NVRAM file, when @FirmwareDevice is @flash.
+#
+# @executable: Identifies the firmware executable. The firmware executable may
+# be shared by multiple virtual machine definitions. The
+# corresponding QEMU command line option is "-drive
+# if=pflash,unit=0,readonly=on,file=@executable.@pathname,format=@executable.@format".
+#
+# @nvram_template: Identifies the NVRAM template compatible with @executable.
+# Management software instantiates an individual copy -- a
+# specific NVRAM file -- from @nvram_template.@pathname for
+# each new virtual machine definition created.
+# @nvram_template.@pathname itself is never mapped into
+# virtual machines, only individual copies of it are. An NVRAM
+# file is typically used for persistently storing the
+# non-volatile UEFI variables of a virtual machine definition.
+# The corresponding QEMU command line option is "-drive
+# if=pflash,unit=1,readonly=off,file=PATHNAME_OF_PRIVATE_NVRAM_FILE,format=@nvram_template.@format".
+#
+# Since: 2.13
+##
+{ 'struct' : 'FirmwareMappingFlash',
+ 'data' : { 'executable' : 'FirmwareFlashFile',
+ 'nvram_template' : 'FirmwareFlashFile' } }
+
+##
+# @FirmwareMappingKernel:
+#
+# Describes loading and mapping properties for the firmware executable, when
+# @FirmwareDevice is @kernel.
+#
+# @pathname: Identifies the firmware executable. The firmware executable may be
+# shared by multiple virtual machine definitions. The corresponding
+# QEMU command line option is "-kernel @pathname".
+#
+# Since: 2.13
+##
+{ 'struct' : 'FirmwareMappingKernel',
+ 'data' : { 'pathname' : 'str' } }
+
+##
+# @FirmwareMappingMemory:
+#
+# Describes loading and mapping properties for the firmware executable, when
+# @FirmwareDevice is @memory.
+#
+# @pathname: Identifies the firmware executable. The firmware executable may be
+# shared by multiple virtual machine definitions. The corresponding
+# QEMU command line option is "-bios @pathname".
+#
+# Since: 2.13
+##
+{ 'struct' : 'FirmwareMappingMemory',
+ 'data' : { 'pathname' : 'str' } }
+
+##
+# @FirmwareMapping:
+#
+# Provides a discriminated structure for firmware to describe its loading /
+# mapping properties.
+#
+# @device: Selects the device type that the firmware must be mapped into.
+#
+# Since: 2.13
+##
+{ 'union' : 'FirmwareMapping',
+ 'base' : { 'device' : 'FirmwareDevice' },
+ 'discriminator' : 'device',
+ 'data' : { 'flash' : 'FirmwareMappingFlash',
+ 'kernel' : 'FirmwareMappingKernel',
+ 'memory' : 'FirmwareMappingMemory' } }
+
+##
+# @Firmware:
+#
+# Describes a firmware (or a firmware use case) to management software.
+#
+# @description: Provides a human-readable description of the firmware.
+# Management software may or may not display @description.
+#
+# @type: Exposes the type of the firmware. @type is generally the primary key
+# for which management software looks when picking a firmware for a new
+# virtual machine definition.
+#
+# @mapping: Describes the loading / mapping properties of the firmware.
+#
+# @targets: Collects the target architectures (QEMU system emulators) and their
+# machine types that may execute the firmware.
+#
+# @features: Lists the features that the firmware supports, and the platform
+# requirements it presents.
+#
+# @tags: A list of auxiliary strings associated with the firmware for which
+# @description is not approprite, due to the latter's possible exposure
+# to the end-user. @tags serves development and debugging purposes only,
+# and management software shall explicitly ignore it.
+#
+# Since: 2.13
+#
+# Examples:
+#
+# {
+# "description": "SeaBIOS",
+# "type": "bios",
+# "mapping": {
+# "device": "memory",
+# "pathname": "/usr/share/seabios/bios-256k.bin"
+# },
+# "targets": [
+# {
+# "architecture": "i386",
+# "machines": [
+# "pc",
+# "q35"
+# ]
+# },
+# {
+# "architecture": "x86_64",
+# "machines": [
+# "pc",
+# "q35"
+# ]
+# }
+# ],
+# "features": [
+# "acpi-s3",
+# "acpi-s4"
+# ],
+# "tags": [
+# "CONFIG_BOOTSPLASH=n",
+# "CONFIG_ROM_SIZE=256",
+# "CONFIG_USE_SMM=n"
+# ]
+# }
+#
+# {
+# "description": "OVMF with SB+SMM, empty varstore",
+# "type": "uefi",
+# "mapping": {
+# "device": "flash",
+# "executable": {
+# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+# "format": "raw"
+# },
+# "nvram_template": {
+# "pathname": "/usr/share/OVMF/OVMF_VARS.fd",
+# "format": "raw"
+# }
+# },
+# "targets": [
+# {
+# "architecture": "x86_64",
+# "machines": [
+# "q35"
+# ]
+# }
+# ],
+# "features": [
+# "acpi-s3",
+# "amd-sev",
+# "requires-smm",
+# "secure-boot",
+# "verbose-dynamic"
+# ],
+# "tags": [
+# "-a IA32",
+# "-a X64",
+# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
+# "-t GCC48",
+# "-b DEBUG",
+# "-D SMM_REQUIRE",
+# "-D SECURE_BOOT_ENABLE",
+# "-D FD_SIZE_4MB"
+# ]
+# }
+#
+# {
+# "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled",
+# "type": "uefi",
+# "mapping": {
+# "device": "flash",
+# "executable": {
+# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+# "format": "raw"
+# },
+# "nvram_template": {
+# "pathname": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
+# "format": "raw"
+# }
+# },
+# "targets": [
+# {
+# "architecture": "x86_64",
+# "machines": [
+# "q35"
+# ]
+# }
+# ],
+# "features": [
+# "acpi-s3",
+# "amd-sev",
+# "requires-smm",
+# "secure-boot",
+# "secure-boot-enrolled-keys",
+# "verbose-dynamic"
+# ],
+# "tags": [
+# "-a IA32",
+# "-a X64",
+# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
+# "-t GCC48",
+# "-b DEBUG",
+# "-D SMM_REQUIRE",
+# "-D SECURE_BOOT_ENABLE",
+# "-D FD_SIZE_4MB"
+# ]
+# }
+#
+# {
+# "description": "UEFI firmware for ARM64 virtual machines",
+# "type": "uefi",
+# "mapping": {
+# "device": "flash",
+# "executable": {
+# "pathname": "/usr/share/AAVMF/AAVMF_CODE.fd",
+# "format": "raw"
+# },
+# "nvram_template": {
+# "pathname": "/usr/share/AAVMF/AAVMF_VARS.fd",
+# "format": "raw"
+# }
+# },
+# "targets": [
+# {
+# "architecture": "aarch64",
+# "machines": [
+# "virt"
+# ]
+# }
+# ],
+# "features": [
+#
+# ],
+# "tags": [
+# "-a AARCH64",
+# "-p ArmVirtPkg/ArmVirtQemu.dsc",
+# "-t GCC48",
+# "-b DEBUG",
+# "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000"
+# ]
+# }
+##
+{ 'struct' : 'Firmware',
+ 'data' : { 'description' : 'str',
+ 'type' : 'FirmwareType',
+ 'mapping' : 'FirmwareMapping',
+ 'targets' : [ 'FirmwareTarget' ],
+ 'features' : [ 'FirmwareFeature' ],
+ 'tags' : [ 'str' ] } }
+
+##
+# @x-check-firmware:
+#
+# Accept a @Firmware object and do nothing, successfully. This command can be
+# used in the QMP shell to validate @Firmware JSON against the schema.
+#
+# @fw: ignored
+#
+# Since: 2.13
+##
+{ 'command' : 'x-check-firmware',
+ 'data' : { 'fw' : 'Firmware' } }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 25bce78352b8..2d6339ca8c99 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -92,4 +92,5 @@
{ 'include': 'transaction.json' }
{ 'include': 'trace.json' }
{ 'include': 'introspect.json' }
+{ 'include': 'firmware.json' }
{ 'include': 'misc.json' }
diff --git a/qmp.c b/qmp.c
index f72261667f92..2141ebe6f027 100644
--- a/qmp.c
+++ b/qmp.c
@@ -34,6 +34,7 @@
#include "qapi/qapi-commands-block-core.h"
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-ui.h"
+#include "qapi/qapi-commands-firmware.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qobject-input-visitor.h"
@@ -781,3 +782,7 @@ void qmp_x_oob_test(bool lock, Error **errp)
qemu_sem_post(&x_oob_test_sem);
}
}
+
+void qmp_x_check_firmware(Firmware *fw, Error **errp)
+{
+}
diff --git a/.gitignore b/.gitignore
index 4055e12ee85d..1d8d1066d3d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,7 @@
/qapi/qapi-commands-char.[ch]
/qapi/qapi-commands-common.[ch]
/qapi/qapi-commands-crypto.[ch]
+/qapi/qapi-commands-firmware.[ch]
/qapi/qapi-commands-introspect.[ch]
/qapi/qapi-commands-migration.[ch]
/qapi/qapi-commands-misc.[ch]
@@ -52,6 +53,7 @@
/qapi/qapi-events-char.[ch]
/qapi/qapi-events-common.[ch]
/qapi/qapi-events-crypto.[ch]
+/qapi/qapi-events-firmware.[ch]
/qapi/qapi-events-introspect.[ch]
/qapi/qapi-events-migration.[ch]
/qapi/qapi-events-misc.[ch]
@@ -70,6 +72,7 @@
/qapi/qapi-types-char.[ch]
/qapi/qapi-types-common.[ch]
/qapi/qapi-types-crypto.[ch]
+/qapi/qapi-types-firmware.[ch]
/qapi/qapi-types-introspect.[ch]
/qapi/qapi-types-migration.[ch]
/qapi/qapi-types-misc.[ch]
@@ -87,6 +90,7 @@
/qapi/qapi-visit-char.[ch]
/qapi/qapi-visit-common.[ch]
/qapi/qapi-visit-crypto.[ch]
+/qapi/qapi-visit-firmware.[ch]
/qapi/qapi-visit-introspect.[ch]
/qapi/qapi-visit-migration.[ch]
/qapi/qapi-visit-misc.[ch]
--
2.14.1.3.gb7cf6e02401b
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 18, 2018 at 12:40:54AM +0200, Laszlo Ersek wrote:
> Add a schema that describes the different uses and properties of virtual
> machine firmware.
Looks good to me overall.
> +{ 'enum' : 'FirmwareType',
> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
openbios missing.
> +{ 'enum' : 'FirmwareArchitecture',
> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
ppc(64) missing (but you have slof above ;) ...
s390 too.
> +# @machines: Lists the machine types (known by the emulator that is specified
> +# through @architecture) that can execute the firmware. Elements of
> +# @machines are not supposed to be versioned; if a machine type is
> +# versioned in QEMU (e.g. "pc-i440fx-2.12"), then its unversioned
> +# variant, which typically refers to the latest version (e.g. "pc"),
> +# should be listed in @machines. On the QEMU command line, "-machine
> +# type=..." specifies the requested machine type.
Hmm, I'd tend to ignore the aliases here (pc, q35, virt) and use
wildcards instead (pc-i440fx-*, pc-q35-*, virt-*).
I think that will be easier for libvirt to work with because it always
resolves aliases to actual machine types when storing them in the domain
xml.
> +# @secure-boot: The firmware implements the software interfaces for UEFI Secure
> +# Boot, as defined in the UEFI specification. Note that without
> +# @requires-smm, guest code running with kernel privileges can
> +# undermine the security of Secure Boot.
> +#
> +# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated
I think "enrolled-keys" should better be a separate feature.
cheers,
Gerd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 08:02, Gerd Hoffmann wrote:
> On Wed, Apr 18, 2018 at 12:40:54AM +0200, Laszlo Ersek wrote:
>> Add a schema that describes the different uses and properties of
>> virtual machine firmware.
>
> Looks good to me overall.
>
>> +{ 'enum' : 'FirmwareType',
>> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>
> openbios missing.
>
>> +{ 'enum' : 'FirmwareArchitecture',
>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>
> ppc(64) missing (but you have slof above ;) ...
> s390 too.
I figured those would be contributed by people that actually use them,
as separate patches :) In fact I would rather prefer removing "slof" and
"uboot" from this initial version, because I have zero clue about them.
Of course, if reviewers can tell me what *exact* enum constants I should
add, I'll comply.
>> +# @machines: Lists the machine types (known by the emulator that is specified
>> +# through @architecture) that can execute the firmware. Elements of
>> +# @machines are not supposed to be versioned; if a machine type is
>> +# versioned in QEMU (e.g. "pc-i440fx-2.12"), then its unversioned
>> +# variant, which typically refers to the latest version (e.g. "pc"),
>> +# should be listed in @machines. On the QEMU command line, "-machine
>> +# type=..." specifies the requested machine type.
>
> Hmm, I'd tend to ignore the aliases here (pc, q35, virt) and use
> wildcards instead (pc-i440fx-*, pc-q35-*, virt-*).
>
> I think that will be easier for libvirt to work with because it always
> resolves aliases to actual machine types when storing them in the
> domain xml.
This surfaced in the RFCv1 discussion, but Daniel suggested ignoring
version numbers:
http://mid.mail-archive.com/20180410093412.GI5155@redhat.com
On 04/10/18 11:34, Daniel P. Berrangé wrote:
> 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.
Continuing:
On 04/18/18 08:02, Gerd Hoffmann wrote:
>> +# @secure-boot: The firmware implements the software interfaces for UEFI Secure
>> +# Boot, as defined in the UEFI specification. Note that without
>> +# @requires-smm, guest code running with kernel privileges can
>> +# undermine the security of Secure Boot.
>> +#
>> +# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated
>
> I think "enrolled-keys" should better be a separate feature.
It's not possible from the edk2 side; without -D SECURE_BOOT_ENABLE, the
SB-related variables (SetupMode, PK, KEK, ...) don't work at all. For
example, if you try to run "EnrollDefaultKeys.efi" on an OVMF that was
built without SB, you get:
> FS1:\> EnrollDefaultKeys.efi
> error: GetVariable("SetupMode", 8BE4DF61-93CA-11D2-AA0D-00E098032B8C): Not Found
In other words, I don't see any use in a @Firmware element where
@nvram_template had the SB operational mode enabled and certificates
enrolled (hence listing the proposed @enrolled-keys in @features), while
@executable lacked the SB feature itself (hence not listing @secure-boot
in @features). In the first place, such a varstore file (to be saved as
@nvram_template) could never be *established* from within the guest,
using the @executable in question -- see the error message above.
Thanks!
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> This surfaced in the RFCv1 discussion, but Daniel suggested ignoring > version numbers: > > http://mid.mail-archive.com/20180410093412.GI5155@redhat.com > > On 04/10/18 11:34, Daniel P. Berrangé wrote: > > 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. IIRC this bit referes to the fact that SMM requires qemu >= 2.x (don't remember which x) to work. So smm-enabled edk2 would just say "pc-q35-*" instead of trying to specifying a version range somehow. > Continuing: > > On 04/18/18 08:02, Gerd Hoffmann wrote: > >> +# @secure-boot: The firmware implements the software interfaces for UEFI Secure > >> +# Boot, as defined in the UEFI specification. Note that without > >> +# @requires-smm, guest code running with kernel privileges can > >> +# undermine the security of Secure Boot. > >> +# > >> +# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated > > > > I think "enrolled-keys" should better be a separate feature. > > It's not possible from the edk2 side; without -D SECURE_BOOT_ENABLE, the > SB-related variables (SetupMode, PK, KEK, ...) don't work at all. Sure. The firmware builds will advertise both "secure-boot" and "enrolled-keys" features to specify that. But I think it should be enough to check for "secure-boot" feature to figure whenever a given firmware build supports secure boot, not both "secure-boot" and "secure-boot-plus-something-else". cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 11:04, Gerd Hoffmann wrote: >> This surfaced in the RFCv1 discussion, but Daniel suggested ignoring >> version numbers: >> >> http://mid.mail-archive.com/20180410093412.GI5155@redhat.com >> >> On 04/10/18 11:34, Daniel P. Berrangé wrote: >>> 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. > > IIRC this bit referes to the fact that SMM requires qemu >= 2.x (don't > remember which x) to work. So smm-enabled edk2 would just say > "pc-q35-*" instead of trying to specifying a version range somehow. OK. I'm fine either way; Dan, can you please confirm you are OK with the suggested wildcard format? (I.e., we still shouldn't include actual version numbers in the supported machtypes list, but we should be more specific than just "pc" and "q35" -- if the machine type is versioned, use an asterisk for covering the version number.) >> Continuing: >> >> On 04/18/18 08:02, Gerd Hoffmann wrote: >>>> +# @secure-boot: The firmware implements the software interfaces for UEFI Secure >>>> +# Boot, as defined in the UEFI specification. Note that without >>>> +# @requires-smm, guest code running with kernel privileges can >>>> +# undermine the security of Secure Boot. >>>> +# >>>> +# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated >>> >>> I think "enrolled-keys" should better be a separate feature. >> >> It's not possible from the edk2 side; without -D SECURE_BOOT_ENABLE, the >> SB-related variables (SetupMode, PK, KEK, ...) don't work at all. > > Sure. The firmware builds will advertise both "secure-boot" and > "enrolled-keys" features to specify that. > > But I think it should be enough to check for "secure-boot" feature to > figure whenever a given firmware build supports secure boot, not both > "secure-boot" and "secure-boot-plus-something-else". Hmmm, I'm not sure I agree. One use case is that you want a domain config in which well-known OS-es, signed by the MS UEFI certs, just boot with SB enabled. (Some of our internal folks really want this.) Another use case is that you want a domain in which SB *can* be enabled, but your installer is signed with a different certificate chain (or it is unsigned), and with *just* the MS certs enrolled, it wouldn't boot at all. So you want the SB *feature*, but definitely not the initial enrollment / SB *operational mode*. For me to understand you better, are you suggesting merely that I: - rename @secure-boot-enrolled-keys to @enrolled-keys, and - drop the reference to @secure-boot from the end of the @enrolled-keys documentation paragraph? (Namely, "@secure-boot-enrolled-keys is only valid if the firmware also supports @secure-boot"). Or are you suggesting something more? Thanks! Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 18, 2018 at 01:48:10PM +0200, Laszlo Ersek wrote: > On 04/18/18 11:04, Gerd Hoffmann wrote: > >> This surfaced in the RFCv1 discussion, but Daniel suggested ignoring > >> version numbers: > >> > >> http://mid.mail-archive.com/20180410093412.GI5155@redhat.com > >> > >> On 04/10/18 11:34, Daniel P. Berrangé wrote: > >>> 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. > > > > IIRC this bit referes to the fact that SMM requires qemu >= 2.x (don't > > remember which x) to work. So smm-enabled edk2 would just say > > "pc-q35-*" instead of trying to specifying a version range somehow. > > OK. I'm fine either way; Dan, can you please confirm you are OK with the > suggested wildcard format? (I.e., we still shouldn't include actual > version numbers in the supported machtypes list, but we should be more > specific than just "pc" and "q35" -- if the machine type is versioned, > use an asterisk for covering the version number.) My suggestion to use a bare "pc" is effectively doing globbing anyway, just that we've left off the "*". Gerd is right that it is probably better to be explicit and include the wildcard. So now the question is should this string be declared to be glob format, or regex format. Regex is more flexible, but regex syntax is ill-defined because every regex engine is slightly different. 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
Hi, > So now the question is should this string be declared to be > glob format, or regex format. Regex is more flexible, but > regex syntax is ill-defined because every regex engine is > slightly different. I'd go for glob. It's good enough for the version number and most likely we don't need anything else. And the machine types are a list, so there is still the option to just have multiple entries in case it turns out we need something glob can't cover with a single entry. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 18, 2018 at 02:30:02PM +0200, Gerd Hoffmann wrote: > Hi, > > > So now the question is should this string be declared to be > > glob format, or regex format. Regex is more flexible, but > > regex syntax is ill-defined because every regex engine is > > slightly different. > > I'd go for glob. It's good enough for the version number and > most likely we don't need anything else. > > And the machine types are a list, so there is still the option > to just have multiple entries in case it turns out we need > something glob can't cover with a single entry. Ok, works for me. 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
On 04/18/18 14:32, Daniel P. Berrangé wrote: > On Wed, Apr 18, 2018 at 02:30:02PM +0200, Gerd Hoffmann wrote: >> Hi, >> >>> So now the question is should this string be declared to be >>> glob format, or regex format. Regex is more flexible, but >>> regex syntax is ill-defined because every regex engine is >>> slightly different. >> >> I'd go for glob. It's good enough for the version number and >> most likely we don't need anything else. >> >> And the machine types are a list, so there is still the option >> to just have multiple entries in case it turns out we need >> something glob can't cover with a single entry. > > Ok, works for me. Me too, thanks. Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi, > Hmmm, I'm not sure I agree. One use case is that you want a domain > config in which well-known OS-es, signed by the MS UEFI certs, just boot > with SB enabled. (Some of our internal folks really want this.) > > Another use case is that you want a domain in which SB *can* be enabled, > but your installer is signed with a different certificate chain (or it > is unsigned), and with *just* the MS certs enrolled, it wouldn't boot at > all. So you want the SB *feature*, but definitely not the initial > enrollment / SB *operational mode*. So "secure-boot-enrolled-keys" also has SB turned on. > For me to understand you better, are you suggesting merely that I: > > - rename @secure-boot-enrolled-keys to @enrolled-keys, and > > - drop the reference to @secure-boot from the end of the @enrolled-keys > documentation paragraph? (Namely, "@secure-boot-enrolled-keys is only > valid if the firmware also supports @secure-boot"). Yes. So "secure-boot" specifies "firmware binary supports secure-boot" and "enrolled-keys" specifies "firmware nvram template has keys enrolled (and SB enabled). Other question: Do we want allow to specify which certs/keys are enrolled? Which would probably mean to drop "enrolled-keys" from features and make it an optional string instead, then specify "'enrolled-keys' : 'Microsoft'" in the json file. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 14:23, Gerd Hoffmann wrote: > Hi, > >> Hmmm, I'm not sure I agree. One use case is that you want a domain >> config in which well-known OS-es, signed by the MS UEFI certs, just boot >> with SB enabled. (Some of our internal folks really want this.) >> >> Another use case is that you want a domain in which SB *can* be enabled, >> but your installer is signed with a different certificate chain (or it >> is unsigned), and with *just* the MS certs enrolled, it wouldn't boot at >> all. So you want the SB *feature*, but definitely not the initial >> enrollment / SB *operational mode*. > > So "secure-boot-enrolled-keys" also has SB turned on. Yes. >> For me to understand you better, are you suggesting merely that I: >> >> - rename @secure-boot-enrolled-keys to @enrolled-keys, and >> >> - drop the reference to @secure-boot from the end of the @enrolled-keys >> documentation paragraph? (Namely, "@secure-boot-enrolled-keys is only >> valid if the firmware also supports @secure-boot"). > > Yes. So "secure-boot" specifies "firmware binary supports secure-boot" > and "enrolled-keys" specifies "firmware nvram template has keys enrolled > (and SB enabled). OK. > Other question: Do we want allow to specify which certs/keys are > enrolled? Which would probably mean to drop "enrolled-keys" from > features and make it an optional string instead, Not an enum? "Microsoft" below should be an enum constant, shouldn't it? > then specify > "'enrolled-keys' : 'Microsoft'" in the json file. If this is really necessary (up to Dan :) ), I'm down with it. Thanks Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
> > Other question: Do we want allow to specify which certs/keys are > > enrolled? Which would probably mean to drop "enrolled-keys" from > > features and make it an optional string instead, > > Not an enum? "Microsoft" below should be an enum constant, shouldn't it? I don't think so. If we want allow other certificate providers (not sure it makes sense as all physical hardware actually runs with the microsoft certificates), then we don't want a fixed list here. So any CA can be listed, be it microsoft, redhat, canonical, verisign or kraxel.org ;) cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 15:06, Gerd Hoffmann wrote: >>> Other question: Do we want allow to specify which certs/keys are >>> enrolled? Which would probably mean to drop "enrolled-keys" from >>> features and make it an optional string instead, >> >> Not an enum? "Microsoft" below should be an enum constant, shouldn't it? > > I don't think so. If we want allow other certificate providers (not > sure it makes sense as all physical hardware actually runs with the > microsoft certificates), then we don't want a fixed list here. So any > CA can be listed, be it microsoft, redhat, canonical, verisign or > kraxel.org ;) I agree (obviously), but then, at what detail do we stop? Because, if we go for full flexibility, then we should formalize: - the certificate that goes into PK, - the list of certificates that go into KEK, - the list of certificates that go into db, and we should likely formalize "certificate" itself as the following pair: - human-readable description (possibly the Common Name of the Subject), - SHA256 digest (fingerprint) of the certificate. I do think this would totally be overkill, but I don't know where to draw the line - between the currently proposed @enrolled-keys (or similar enums that stand for well-defined "constellations") - and the full details as described above. A simple string like "Microsoft" doesn't seem to me helpful for either the user or management software -- the former won't know what "Microsoft" stands for, and the latter won't want to look for free-form strings. (Same issue as with @tags vs. @features.) Thanks Laszlo
On Wed, Apr 18, 2018 at 03:30:36PM +0200, Laszlo Ersek wrote: > On 04/18/18 15:06, Gerd Hoffmann wrote: > >>> Other question: Do we want allow to specify which certs/keys are > >>> enrolled? Which would probably mean to drop "enrolled-keys" from > >>> features and make it an optional string instead, > >> > >> Not an enum? "Microsoft" below should be an enum constant, shouldn't it? > > > > I don't think so. If we want allow other certificate providers (not > > sure it makes sense as all physical hardware actually runs with the > > microsoft certificates), then we don't want a fixed list here. So any > > CA can be listed, be it microsoft, redhat, canonical, verisign or > > kraxel.org ;) > > I agree (obviously), but then, at what detail do we stop? > > Because, if we go for full flexibility, then we should formalize: > - the certificate that goes into PK, > - the list of certificates that go into KEK, > - the list of certificates that go into db, > > and we should likely formalize "certificate" itself as the following pair: > - human-readable description (possibly the Common Name of the Subject), > - SHA256 digest (fingerprint) of the certificate. > > I do think this would totally be overkill, but I don't know where to > draw the line > - between the currently proposed @enrolled-keys (or similar enums that > stand for well-defined "constellations") > - and the full details as described above. > > A simple string like "Microsoft" doesn't seem to me helpful for either > the user or management software -- the former won't know what > "Microsoft" stands for, and the latter won't want to look for free-form > strings. (Same issue as with @tags vs. @features.) Ignoring secboot cert name for a minute, ultimately no matter what we do in terms of metadata there is always going to be the possibility that many firmware images all match the criteria libvirt is searching on. Apps thus need rules to pick one of the many matches, and users need the ability to override distro defaults. We can achieve that as follows: Recommend that firmware files are created with a double-digit prefix eg 50-ovmf.json 50-seabios-256k.json, etc, etc, so they can be sorted in predictable order Second, we should define three well known directory locations - /usr/share/qemu/firmware (used by distros packages) - /etc/qemu/firmware (exclusively for sysadmin local additions) - $HOME/.config/qemu/firmware (exclusively for per-user local additions) Apps like libvirt should build list of files from all three locations, then sort by filename. If a local directory has a file with same name as the distro directory, then it should replace the distro provided file. A zero length file should be simply hide the distro provided file So for example, distro ships /usr/share/qemu/firmware/50-ovmf.json /usr/share/qemu/firmware/50-seabios-256k.json The sysadmin can now prevent the default ovmf being used at all with $ touch /etc/qemu/firmware/50-ovmf.json The sysadmin can replace/alter the distro default ovmf with $ vim /etc/qemu/firmware/50-ovmf.json Or they can provide a parallel ovmf with higher priority $ vim /etc/qemu/firmware/10-ovmf.json Or they can provide a parallel ovmf with lower priority $ vim /etc/qemu/firmware/99-ovmf.json $HOME/.config/qemu/firmware would take prior over /etc/qemu and /usr/share/qemu. Getting back to the question of many ovmf images with various different secboot keys. The distro can now provide many ovmf images each with different keys, if desired and determine which one is picked by default. The end user can provide their over ovmf with personal keys that replaces the distro microsoft enrolled one entirely, etc. IOW, don't think we need to record which certs are use for secboot in the JSON metadata. Just lets overrides & ordering take care of 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 :|
On 04/18/18 15:53, Daniel P. Berrangé wrote: > On Wed, Apr 18, 2018 at 03:30:36PM +0200, Laszlo Ersek wrote: >> On 04/18/18 15:06, Gerd Hoffmann wrote: >>>>> Other question: Do we want allow to specify which certs/keys are >>>>> enrolled? Which would probably mean to drop "enrolled-keys" from >>>>> features and make it an optional string instead, >>>> >>>> Not an enum? "Microsoft" below should be an enum constant, shouldn't it? >>> >>> I don't think so. If we want allow other certificate providers (not >>> sure it makes sense as all physical hardware actually runs with the >>> microsoft certificates), then we don't want a fixed list here. So any >>> CA can be listed, be it microsoft, redhat, canonical, verisign or >>> kraxel.org ;) >> >> I agree (obviously), but then, at what detail do we stop? >> >> Because, if we go for full flexibility, then we should formalize: >> - the certificate that goes into PK, >> - the list of certificates that go into KEK, >> - the list of certificates that go into db, >> >> and we should likely formalize "certificate" itself as the following pair: >> - human-readable description (possibly the Common Name of the Subject), >> - SHA256 digest (fingerprint) of the certificate. >> >> I do think this would totally be overkill, but I don't know where to >> draw the line >> - between the currently proposed @enrolled-keys (or similar enums that >> stand for well-defined "constellations") >> - and the full details as described above. >> >> A simple string like "Microsoft" doesn't seem to me helpful for either >> the user or management software -- the former won't know what >> "Microsoft" stands for, and the latter won't want to look for free-form >> strings. (Same issue as with @tags vs. @features.) > > Ignoring secboot cert name for a minute, ultimately no matter what we do > in terms of metadata there is always going to be the possibility that > many firmware images all match the criteria libvirt is searching on. > > Apps thus need rules to pick one of the many matches, and users need the > ability to override distro defaults. We can achieve that as follows: > > Recommend that firmware files are created with a double-digit prefix > eg 50-ovmf.json 50-seabios-256k.json, etc, etc, so they can be > sorted in predictable order > > Second, we should define three well known directory locations > > - /usr/share/qemu/firmware (used by distros packages) > - /etc/qemu/firmware (exclusively for sysadmin local additions) > - $HOME/.config/qemu/firmware (exclusively for per-user local additions) > > Apps like libvirt should build list of files from all three locations, > then sort by filename. If a local directory has a file with same > name as the distro directory, then it should replace the distro provided > file. A zero length file should be simply hide the distro provided file > > So for example, distro ships > > /usr/share/qemu/firmware/50-ovmf.json > /usr/share/qemu/firmware/50-seabios-256k.json > > The sysadmin can now prevent the default ovmf being used at all with > > $ touch /etc/qemu/firmware/50-ovmf.json > > The sysadmin can replace/alter the distro default ovmf with > > $ vim /etc/qemu/firmware/50-ovmf.json > > Or they can provide a parallel ovmf with higher priority > > $ vim /etc/qemu/firmware/10-ovmf.json > > Or they can provide a parallel ovmf with lower priority > > $ vim /etc/qemu/firmware/99-ovmf.json > > $HOME/.config/qemu/firmware would take prior over /etc/qemu and > /usr/share/qemu. > > > Getting back to the question of many ovmf images with various different > secboot keys. The distro can now provide many ovmf images each with > different keys, if desired and determine which one is picked by default. > > The end user can provide their over ovmf with personal keys that replaces > the distro microsoft enrolled one entirely, etc. > > IOW, don't think we need to record which certs are use for secboot in > the JSON metadata. Just lets overrides & ordering take care of it. OK, thank you. Three more questions: - Would you like me to capture the directory paths in the firmware.json file, or in the commit message for the patch? - Should we keep @secure-boot-enrolled-keys (or, as Gerd suggests, @enrolled-keys) in the schema, as a feature enum constant, but remove the documentation of the actual certificates? (I.e., say "an unspecified set of certificates has been enrolled and SB mode has been enabled".) - Or else, should we drop the feature flag that stands for enrollment completely? Thanks, Laszlo
On Wed, Apr 18, 2018 at 04:03:03PM +0200, Laszlo Ersek wrote: > On 04/18/18 15:53, Daniel P. Berrangé wrote: > > On Wed, Apr 18, 2018 at 03:30:36PM +0200, Laszlo Ersek wrote: > >> On 04/18/18 15:06, Gerd Hoffmann wrote: > >>>>> Other question: Do we want allow to specify which certs/keys are > >>>>> enrolled? Which would probably mean to drop "enrolled-keys" from > >>>>> features and make it an optional string instead, > >>>> > >>>> Not an enum? "Microsoft" below should be an enum constant, shouldn't it? > >>> > >>> I don't think so. If we want allow other certificate providers (not > >>> sure it makes sense as all physical hardware actually runs with the > >>> microsoft certificates), then we don't want a fixed list here. So any > >>> CA can be listed, be it microsoft, redhat, canonical, verisign or > >>> kraxel.org ;) > >> > >> I agree (obviously), but then, at what detail do we stop? > >> > >> Because, if we go for full flexibility, then we should formalize: > >> - the certificate that goes into PK, > >> - the list of certificates that go into KEK, > >> - the list of certificates that go into db, > >> > >> and we should likely formalize "certificate" itself as the following pair: > >> - human-readable description (possibly the Common Name of the Subject), > >> - SHA256 digest (fingerprint) of the certificate. > >> > >> I do think this would totally be overkill, but I don't know where to > >> draw the line > >> - between the currently proposed @enrolled-keys (or similar enums that > >> stand for well-defined "constellations") > >> - and the full details as described above. > >> > >> A simple string like "Microsoft" doesn't seem to me helpful for either > >> the user or management software -- the former won't know what > >> "Microsoft" stands for, and the latter won't want to look for free-form > >> strings. (Same issue as with @tags vs. @features.) > > > > Ignoring secboot cert name for a minute, ultimately no matter what we do > > in terms of metadata there is always going to be the possibility that > > many firmware images all match the criteria libvirt is searching on. > > > > Apps thus need rules to pick one of the many matches, and users need the > > ability to override distro defaults. We can achieve that as follows: > > > > Recommend that firmware files are created with a double-digit prefix > > eg 50-ovmf.json 50-seabios-256k.json, etc, etc, so they can be > > sorted in predictable order > > > > Second, we should define three well known directory locations > > > > - /usr/share/qemu/firmware (used by distros packages) > > - /etc/qemu/firmware (exclusively for sysadmin local additions) > > - $HOME/.config/qemu/firmware (exclusively for per-user local additions) > > > > Apps like libvirt should build list of files from all three locations, > > then sort by filename. If a local directory has a file with same > > name as the distro directory, then it should replace the distro provided > > file. A zero length file should be simply hide the distro provided file > > > > So for example, distro ships > > > > /usr/share/qemu/firmware/50-ovmf.json > > /usr/share/qemu/firmware/50-seabios-256k.json > > > > The sysadmin can now prevent the default ovmf being used at all with > > > > $ touch /etc/qemu/firmware/50-ovmf.json > > > > The sysadmin can replace/alter the distro default ovmf with > > > > $ vim /etc/qemu/firmware/50-ovmf.json > > > > Or they can provide a parallel ovmf with higher priority > > > > $ vim /etc/qemu/firmware/10-ovmf.json > > > > Or they can provide a parallel ovmf with lower priority > > > > $ vim /etc/qemu/firmware/99-ovmf.json > > > > $HOME/.config/qemu/firmware would take prior over /etc/qemu and > > /usr/share/qemu. > > > > > > Getting back to the question of many ovmf images with various different > > secboot keys. The distro can now provide many ovmf images each with > > different keys, if desired and determine which one is picked by default. > > > > The end user can provide their over ovmf with personal keys that replaces > > the distro microsoft enrolled one entirely, etc. > > > > IOW, don't think we need to record which certs are use for secboot in > > the JSON metadata. Just lets overrides & ordering take care of it. > > OK, thank you. Three more questions: > > - Would you like me to capture the directory paths in the firmware.json > file, or in the commit message for the patch? Should be in whatever file ends up in the docs/specs directory eventually. > > - Should we keep @secure-boot-enrolled-keys (or, as Gerd suggests, > @enrolled-keys) in the schema, as a feature enum constant, but remove > the documentation of the actual certificates? (I.e., say "an > unspecified set of certificates has been enrolled and SB mode has been > enabled".) I think it is worth keeping the feature flag - we simply don't need to say /what/ keys. > > - Or else, should we drop the feature flag that stands for enrollment > completely? 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
On 04/18/18 16:06, Daniel P. Berrangé wrote: > On Wed, Apr 18, 2018 at 04:03:03PM +0200, Laszlo Ersek wrote: >> - Would you like me to capture the directory paths in the firmware.json >> file, or in the commit message for the patch? > > Should be in whatever file ends up in the docs/specs directory eventually. > >> >> - Should we keep @secure-boot-enrolled-keys (or, as Gerd suggests, >> @enrolled-keys) in the schema, as a feature enum constant, but remove >> the documentation of the actual certificates? (I.e., say "an >> unspecified set of certificates has been enrolled and SB mode has been >> enabled".) > > I think it is worth keeping the feature flag - we simply don't need > to say /what/ keys. All clear, thanks. Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 18 Apr 2018 10:32:06 +0200
Laszlo Ersek <lersek@redhat.com> wrote:
> On 04/18/18 08:02, Gerd Hoffmann wrote:
> [...]
> [...]
> >
> > Looks good to me overall.
> >
> >> +{ 'enum' : 'FirmwareType',
> >> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> >
> > openbios missing.
> >
> >> +{ 'enum' : 'FirmwareArchitecture',
> >> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
> >
> > ppc(64) missing (but you have slof above ;) ...
> > s390 too.
>
> I figured those would be contributed by people that actually use them,
> as separate patches :) In fact I would rather prefer removing "slof" and
> "uboot" from this initial version, because I have zero clue about them.
I've only been able to skim this discussion, so apologies if I've
missed things. I'm pretty unclear on the overall purpose of this, but
in particular this FirmwareType field seems pretty weird.
Specifically the things in the list don't really seem comparable to
each other: UEFI is a specified interface, BIOS is a de-facto
interface. So far so good. But SLOF is a specific implementation of
Open Firmware (of which we have a couple of other partial
implementations used for other qemu platforms). U-Boot is somewhere in
between the two, a specific implementation that defines a fair bunch of
its own interfaces.
--
David Gibson <dgibson@redhat.com>
Principal Software Engineer, Virtualization, Red Hat
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/19/18 02:09, David Gibson wrote:
> On Wed, 18 Apr 2018 10:32:06 +0200
> Laszlo Ersek <lersek@redhat.com> wrote:
>
>> On 04/18/18 08:02, Gerd Hoffmann wrote:
>> [...]
>> [...]
>>>
>>> Looks good to me overall.
>>>
>>>> +{ 'enum' : 'FirmwareType',
>>>> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>>
>>> openbios missing.
>>>
>>>> +{ 'enum' : 'FirmwareArchitecture',
>>>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>>>
>>> ppc(64) missing (but you have slof above ;) ...
>>> s390 too.
>>
>> I figured those would be contributed by people that actually use them,
>> as separate patches :) In fact I would rather prefer removing "slof" and
>> "uboot" from this initial version, because I have zero clue about them.
>
> I've only been able to skim this discussion, so apologies if I've
> missed things. I'm pretty unclear on the overall purpose of this, but
> in particular this FirmwareType field seems pretty weird.
>
> Specifically the things in the list don't really seem comparable to
> each other: UEFI is a specified interface, BIOS is a de-facto
> interface. So far so good. But SLOF is a specific implementation of
> Open Firmware (of which we have a couple of other partial
> implementations used for other qemu platforms).
Thank you -- I will replace SLOF with "openfirmware".
This also implies I shouldn't add "openbios" separately, which was
suggested earlier by Gerd -- according to
<https://en.wikipedia.org/wiki/OpenBIOS>, OpenBIOS is another
implementation of OFW.
> U-Boot is somewhere in
> between the two, a specific implementation that defines a fair bunch of
> its own interfaces.
Right, this is about interfaces, so I'll keep "uboot".
Thank you!
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 19 Apr 2018 10:09:30 +0200 Laszlo Ersek <lersek@redhat.com> wrote: > On 04/19/18 02:09, David Gibson wrote: > [...] > [...] > [...] > [...] > [...] > [...] > [...] > [...] > [...] > > Thank you -- I will replace SLOF with "openfirmware". > > This also implies I shouldn't add "openbios" separately, which was > suggested earlier by Gerd -- according to > <https://en.wikipedia.org/wiki/OpenBIOS>, OpenBIOS is another > implementation of OFW. Right. Although I think OpenBIOS and SLOF support a disjoint set of machines. Openhackware which is (was?) used on some machines is yet another (very partial) openfirmware implementation. > > [...] > > Right, this is about interfaces, so I'll keep "uboot". Ok. More specifically I guess this is about firmware <-> guest OS, and firmware <-> interactive user interfaces. Firmware <-> qemu interfaces (fw_cfg, device tree) are a different question. -- David Gibson <dgibson@redhat.com> Principal Software Engineer, Virtualization, Red Hat -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 20/04/2018 03:03, David Gibson wrote: >> This also implies I shouldn't add "openbios" separately, which was >> suggested earlier by Gerd -- according to >> <https://en.wikipedia.org/wiki/OpenBIOS>, OpenBIOS is another >> implementation of OFW. > > Right. Although I think OpenBIOS and SLOF support a disjoint set of > machines. Openhackware which is (was?) used on some machines is yet > another (very partial) openfirmware implementation. We can: 1a) replace the architecture field with an ABI field (seems wrong to me) 1b) add an "ABI" field (containing e.g. prep, spapr, etc.) to complement the architecture field 2) we include directly a glob pattern for the QEMU machine types (also seems wrong). 3) just like we effectively moved the guest ABI to the features field, we split the features into host-features and guest-features, and the host ABI (prep, spapr, etc.; but also OVMF's SMM requirement fits here for example) moves into host-features. Again, just like 1a/1b, this can be replace or complement the architecture field, and I think I'd prefer the latter Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/20/18 10:47, Paolo Bonzini wrote: > On 20/04/2018 03:03, David Gibson wrote: >>> This also implies I shouldn't add "openbios" separately, which was >>> suggested earlier by Gerd -- according to >>> <https://en.wikipedia.org/wiki/OpenBIOS>, OpenBIOS is another >>> implementation of OFW. >> >> Right. Although I think OpenBIOS and SLOF support a disjoint set of >> machines. Openhackware which is (was?) used on some machines is yet >> another (very partial) openfirmware implementation. > > We can: > > 1a) replace the architecture field with an ABI field (seems wrong to me) > > 1b) add an "ABI" field (containing e.g. prep, spapr, etc.) to complement > the architecture field > > 2) we include directly a glob pattern for the QEMU machine types (also > seems wrong). Using globs in machine types was what Gerd and Daniel agreed upon, and I was planning to do for v3. :( > 3) just like we effectively moved the guest ABI to the features field, No, we didn't -- while it's true that a single firmware image can provide multiple guest ABIs, the implementation quality of those guest ABIs is not identical, and there is a preference order between them. The latest idea was to keep the @type field, but turn it into an ordered list of enum constants. Whichever constant is near the top is the more preferred ABI, and mgmt tools, when looking for "the" type of the firware, would consider the topmost (first) element. > we split the features into host-features and guest-features, and the > host ABI (prep, spapr, etc.; but also OVMF's SMM requirement fits here > for example) moves into host-features. Again, just like 1a/1b, this can > be replace or complement the architecture field, and I think I'd prefer > the latter I'd prefer if we could mostly stick with the structure of the schema as seen in RFCv2 -- all the feedback until now implies that's possible, with minor tweaks. Let me post an RFCv3 soon, so that we have a more up-to-date basis for the discussion. (I've been waiting for some of the architecture (emulation target) technicalities to clear up, but I guess I had better postpone that now, because I see the discussion diverging.) Thanks! Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 18.04.2018 08:02, Gerd Hoffmann wrote:
> On Wed, Apr 18, 2018 at 12:40:54AM +0200, Laszlo Ersek wrote:
>> Add a schema that describes the different uses and properties of virtual
>> machine firmware.
>
> Looks good to me overall.
>
>> +{ 'enum' : 'FirmwareType',
>> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>
> openbios missing.
As mentioned somewhere else in this thread, I think it's best to just
use "openfirmware" here instead of "slof" and "openbios"
>> +{ 'enum' : 'FirmwareArchitecture',
>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>
> ppc(64) missing (but you have slof above ;) ...
Since you distinguish between 32-bit and 64-bit, we need both "ppc"
(32-bit) and "ppc64" (64-bit) here.
> s390 too.
Please name that "s390x". "s390" without x means the old 31-bit
architecture which is hardly used anymore today. "s390x" is the modern
64-bit architecture.
Thomas
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Mostly quick QAPI schema style review, as I know next to nothing about
the subject matter.
Laszlo Ersek <lersek@redhat.com> writes:
> Add a schema that describes the different uses and properties of virtual
> machine firmware.
>
> Each firmware executable installed on a host system should come with at
> least one JSON file that conforms to this schema. Each file informs the
> management applications about the firmware's properties and one possible
> use case / feature set.
>
> 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 description 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 for a
> certain use case or feature set.
>
> Cc: "Daniel P. Berrange" <berrange@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: David Gibson <dgibson@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Michal Privoznik <mprivozn@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Krempa <pkrempa@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
[...]
> diff --git a/qapi/firmware.json b/qapi/firmware.json
> new file mode 100644
> index 000000000000..3653b4628a5b
> --- /dev/null
> +++ b/qapi/firmware.json
> @@ -0,0 +1,503 @@
> +# -*- Mode: Python -*-
> +#
> +# Copyright (C) 2018 Red Hat, Inc.
> +#
> +# Authors:
> +# Daniel P. Berrange <berrange@redhat.com>
> +# Laszlo Ersek <lersek@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later. See
> +# the COPYING file in the top-level directory.
> +
> +##
> +# = Firmware
> +##
> +
> +{ 'include' : 'block-core.json' }
> +
> +##
> +# @FirmwareType:
> +#
> +# Lists firmware types commonly used with QEMU virtual machines.
> +#
> +# @bios: The firmware was built from the SeaBIOS project.
> +#
> +# @slof: The firmware was built from the Slimline Open Firmware project.
> +#
> +# @uboot: The firmware was built from the U-Boot project.
> +#
> +# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareType',
> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> +
> +##
> +# @FirmwareDevice:
> +#
> +# Defines the device types that firmware can be mapped into.
> +#
> +# @flash: The firmware executable and its accompanying NVRAM file are to be
> +# mapped into a pflash chip each.
> +#
> +# @kernel: The firmware 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 and machine type.
> +#
> +# @memory: The firmware is to be mapped into memory.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareDevice',
> + 'data' : [ 'flash', 'kernel', 'memory' ] }
> +
> +##
> +# @FirmwareArchitecture:
> +#
> +# Defines the target architectures (emulator binaries) that firmware may
> +# execute on.
> +#
> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
> +#
> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
> +#
> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
> +#
> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
Partial dupe of CpuInfoArch from misc.json. Neither covers all our
target architectures. Should we have one that does in common.json
instead?
> +
> +##
> +# @FirmwareTarget:
> +#
> +# Defines the machine types that firmware may execute on.
> +#
> +# @architecture: Determines the emulation target (the QEMU system emulator)
> +# that can execute the firmware.
> +#
> +# @machines: Lists the machine types (known by the emulator that is specified
> +# through @architecture) that can execute the firmware. Elements of
> +# @machines are not supposed to be versioned; if a machine type is
> +# versioned in QEMU (e.g. "pc-i440fx-2.12"), then its unversioned
> +# variant, which typically refers to the latest version (e.g. "pc"),
> +# should be listed in @machines. On the QEMU command line, "-machine
> +# type=..." specifies the requested machine type.
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareTarget',
> + 'data' : { 'architecture' : 'FirmwareArchitecture',
> + 'machines' : [ 'str' ] } }
> +
> +##
> +# @FirmwareFeature:
> +#
> +# Defines the features that firmware may support, and the platform requirements
> +# that firmware may present.
> +#
> +# @acpi-s3: The firmware supports S3 sleep (suspend to RAM), as defined in the
> +# ACPI specification. On the "pc" machine type of the @i386 and
> +# @x86_64 emulation targets, S3 can be enabled with "-global
> +# PIIX4_PM.disable_s3=0" and disabled with "-global
> +# PIIX4_PM.disable_s3=1". On the "q35" machine type of the @i386 and
> +# @x86_64 emulation targets, S3 can be enabled with "-global
> +# ICH9-LPC.disable_s3=0" and disabled with "-global
> +# ICH9-LPC.disable_s3=1".
> +#
> +# @acpi-s4: The firmware supports S4 hibernation (suspend to disk), as defined
> +# in the ACPI specification. On the "pc" machine type of the @i386
> +# and @x86_64 emulation targets, S4 can be enabled with "-global
> +# PIIX4_PM.disable_s4=0" and disabled with "-global
> +# PIIX4_PM.disable_s4=1". On the "q35" machine type of the @i386 and
> +# @x86_64 emulation targets, S4 can be enabled with "-global
> +# ICH9-LPC.disable_s4=0" and disabled with "-global
> +# ICH9-LPC.disable_s4=1".
Would you mind breaking documentation lines a bit ealier?
> +#
> +# @amd-sev: The firmware supports running under AMD Secure Encrypted
> +# Virtualization, as specified in the AMD64 Architecture Programmer's
> +# Manual. QEMU command line options related to this feature are
> +# documented in "docs/amd-memory-encryption.txt".
> +#
> +# @requires-smm: The firmware requires the platform to emulate SMM (System
> +# Management Mode), as defined in the AMD64 Architecture
> +# Programmer's Manual, and in the Intel(R)64 and IA-32
> +# Architectures Software Developer's Manual. On the "q35"
> +# machine type of the @i386 and @x86_64 emulation targets, SMM
> +# emulation can be enabled with "-machine smm=on". (On the "q35"
> +# machine type of the @i386 emulation target, @requires-smm
> +# presents further CPU requirements; one combination known to
> +# work is "-cpu coreduo,-nx".) If the firmware is marked as both
> +# @secure-boot and @requires-smm, then write accesses to the
> +# pflash chip (NVRAM) that holds the UEFI variable store must be
> +# restricted to code that executes in SMM, using the additional
> +# option "-global driver=cfi.pflash01,property=secure,value=on".
> +# Furthermore, a large guest-physical address space (comprising
> +# guest RAM, memory hotplug range, and 64-bit PCI MMIO
> +# aperture), and/or a high VCPU count, may present high SMRAM
> +# requirements from the firmware. On the "q35" machine type of
> +# the @i386 and @x86_64 emulation targets, the SMRAM size may be
> +# increased above the default 16MB with the "-global
> +# mch.extended-tseg-mbytes=uint16" option. As a rule of thumb,
> +# the default 16MB size suffices for 1TB of guest-phys address
> +# space and a few tens of VCPUs; for every further TB of
> +# guest-phys address space, add 8MB of SMRAM. 48MB should
> +# suffice for 4TB of guest-phys address space and 2-3 hundred
> +# VCPUs.
> +#
> +# @secure-boot: The firmware implements the software interfaces for UEFI Secure
> +# Boot, as defined in the UEFI specification. Note that without
> +# @requires-smm, guest code running with kernel privileges can
> +# undermine the security of Secure Boot.
> +#
> +# @secure-boot-enrolled-keys: The variable store (NVRAM) template associated
> +# with the firmware binary has the Secure Boot
> +# operational mode enabled, and -- at least -- the
> +# following certificates enrolled. (1) As Platform
> +# Key (PK), and as one Key Exchange Key (KEK), a
> +# self-signed certificate issued by the firmware
> +# distributor is enrolled. (2) As another Key
> +# Exchange Key (KEK), the "Microsoft Corporation
> +# KEK CA 2011" certificate is enrolled. The UEFI
> +# Forum releases updates for the Secure Boot
> +# Signature/Certificate Blacklist ("dbx")
> +# periodically at
> +# <http://www.uefi.org/revocationlistfile>, signed
> +# with a certificate chain anchored in this
> +# certificate. (3) As one Secure Boot
> +# Signature/Certificate ("db"), the "Microsoft
> +# Windows Production PCA 2011" certificate is
> +# enrolled. This certificate verifies Windows 8,
> +# Windows Server 2012 R2, etc boot loaders. (4) As
> +# another Secure Boot Signature/Certificate ("db"),
> +# the "Microsoft Corporation UEFI CA 2011"
> +# certificate is enrolled. This certificate
> +# verifies the "shim" UEFI application, and PCI
> +# expansion ROMs. @secure-boot-enrolled-keys is
> +# only valid if the firmware also supports
> +# @secure-boot.
> +#
> +# @verbose-dynamic: When firmware log capture is enabled, the firmware logs a
> +# large amount of debug messages, which may impact boot
> +# performance. With log capture disabled, there is no boot
> +# performance impact. On the "pc" and "q35" machine types of
> +# the @i386 and @x86_64 emulation targets, firmware log
> +# capture can be enabled with the QEMU command line options
> +# "-chardev file,id=fwdebug,path=LOGFILEPATH -device
> +# isa-debugcon,iobase=0x402,chardev=fwdebug".
> +# @verbose-dynamic is mutually exclusive with
> +# @verbose-static.
> +#
> +# @verbose-static: The firmware unconditionally produces a large amount of
> +# debug messages, which may impact boot performance. This
> +# feature may typically be carried by certain UEFI firmware
> +# for the "virt" machine type of the @arm and @aarch64
> +# emulation targets, where the debug messages are written to
> +# the first (always present) PL011 UART. @verbose-static is
> +# mutually exclusive with @verbose-dynamic.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareFeature',
> + 'data' : [ 'acpi-s3', 'acpi-s4', 'amd-sev', 'requires-smm', 'secure-boot',
> + 'secure-boot-enrolled-keys', 'verbose-dynamic',
> + 'verbose-static' ] }
> +
> +##
> +# @FirmwareFlashFile:
> +#
> +# Defines common properties that are necessary for loading a firmware file into
> +# a pflash chip. The corresponding QEMU command line option is "-drive
> +# file=@pathname,format=@format". Note however that the option-argument shown
> +# here is incomplete; it is completed under @FirmwareMappingFlash.
> +#
> +# @pathname: Specifies the pathname on the host filesystem where the firmware
> +# file can be found.
We use @filename elsewhere in the QAPI schema. Let's stick to that.
More of the same below.
> +#
> +# @format: Specifies the block format of the file pointed-to by @pathname, such
> +# as @raw or @qcow2.
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareFlashFile',
> + 'data' : { 'pathname' : 'str',
> + 'format' : 'BlockdevDriver' } }
> +
> +##
> +# @FirmwareMappingFlash:
> +#
> +# Describes loading and mapping properties for the firmware executable and its
> +# accompanying NVRAM file, when @FirmwareDevice is @flash.
> +#
> +# @executable: Identifies the firmware executable. The firmware executable may
> +# be shared by multiple virtual machine definitions. The
> +# corresponding QEMU command line option is "-drive
> +# if=pflash,unit=0,readonly=on,file=@executable.@pathname,format=@executable.@format".
I guess @executable.@pathname means member @pathname of @executable. I
read it as two distinct parameters first, then wondered where parameter
@pathname is. Perhaps @executable.pathname would be clearer.
> +#
> +# @nvram_template: Identifies the NVRAM template compatible with @executable.
> +# Management software instantiates an individual copy -- a
> +# specific NVRAM file -- from @nvram_template.@pathname for
> +# each new virtual machine definition created.
> +# @nvram_template.@pathname itself is never mapped into
> +# virtual machines, only individual copies of it are. An NVRAM
> +# file is typically used for persistently storing the
> +# non-volatile UEFI variables of a virtual machine definition.
> +# The corresponding QEMU command line option is "-drive
> +# if=pflash,unit=1,readonly=off,file=PATHNAME_OF_PRIVATE_NVRAM_FILE,format=@nvram_template.@format".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingFlash',
> + 'data' : { 'executable' : 'FirmwareFlashFile',
> + 'nvram_template' : 'FirmwareFlashFile' } }
> +
> +##
> +# @FirmwareMappingKernel:
> +#
> +# Describes loading and mapping properties for the firmware executable, when
> +# @FirmwareDevice is @kernel.
> +#
> +# @pathname: Identifies the firmware executable. The firmware executable may be
> +# shared by multiple virtual machine definitions. The corresponding
> +# QEMU command line option is "-kernel @pathname".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingKernel',
> + 'data' : { 'pathname' : 'str' } }
> +
> +##
> +# @FirmwareMappingMemory:
> +#
> +# Describes loading and mapping properties for the firmware executable, when
> +# @FirmwareDevice is @memory.
> +#
> +# @pathname: Identifies the firmware executable. The firmware executable may be
> +# shared by multiple virtual machine definitions. The corresponding
> +# QEMU command line option is "-bios @pathname".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingMemory',
> + 'data' : { 'pathname' : 'str' } }
> +
> +##
> +# @FirmwareMapping:
> +#
> +# Provides a discriminated structure for firmware to describe its loading /
> +# mapping properties.
> +#
> +# @device: Selects the device type that the firmware must be mapped into.
> +#
> +# Since: 2.13
> +##
> +{ 'union' : 'FirmwareMapping',
> + 'base' : { 'device' : 'FirmwareDevice' },
> + 'discriminator' : 'device',
> + 'data' : { 'flash' : 'FirmwareMappingFlash',
> + 'kernel' : 'FirmwareMappingKernel',
> + 'memory' : 'FirmwareMappingMemory' } }
The FirmwareMapping* all have a member @pathname. It could be moved to
the base. Makes sense if we think future FirmwareMappingFOOs will also
have it. Your choice.
> +
> +##
> +# @Firmware:
> +#
> +# Describes a firmware (or a firmware use case) to management software.
> +#
> +# @description: Provides a human-readable description of the firmware.
> +# Management software may or may not display @description.
> +#
> +# @type: Exposes the type of the firmware. @type is generally the primary key
> +# for which management software looks when picking a firmware for a new
> +# virtual machine definition.
> +#
> +# @mapping: Describes the loading / mapping properties of the firmware.
> +#
> +# @targets: Collects the target architectures (QEMU system emulators) and their
> +# machine types that may execute the firmware.
> +#
> +# @features: Lists the features that the firmware supports, and the platform
> +# requirements it presents.
> +#
> +# @tags: A list of auxiliary strings associated with the firmware for which
> +# @description is not approprite, due to the latter's possible exposure
s/approprite/appropriate/
Feed the new comments to a spell checker?
> +# to the end-user. @tags serves development and debugging purposes only,
> +# and management software shall explicitly ignore it.
> +#
> +# Since: 2.13
> +#
> +# Examples:
> +#
> +# {
> +# "description": "SeaBIOS",
> +# "type": "bios",
> +# "mapping": {
> +# "device": "memory",
> +# "pathname": "/usr/share/seabios/bios-256k.bin"
> +# },
> +# "targets": [
> +# {
> +# "architecture": "i386",
> +# "machines": [
> +# "pc",
> +# "q35"
> +# ]
> +# },
> +# {
> +# "architecture": "x86_64",
> +# "machines": [
> +# "pc",
> +# "q35"
> +# ]
> +# }
> +# ],
> +# "features": [
> +# "acpi-s3",
> +# "acpi-s4"
> +# ],
> +# "tags": [
> +# "CONFIG_BOOTSPLASH=n",
> +# "CONFIG_ROM_SIZE=256",
> +# "CONFIG_USE_SMM=n"
> +# ]
> +# }
> +#
> +# {
> +# "description": "OVMF with SB+SMM, empty varstore",
> +# "type": "uefi",
> +# "mapping": {
> +# "device": "flash",
> +# "executable": {
> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +# "format": "raw"
> +# },
> +# "nvram_template": {
> +# "pathname": "/usr/share/OVMF/OVMF_VARS.fd",
> +# "format": "raw"
> +# }
> +# },
> +# "targets": [
> +# {
> +# "architecture": "x86_64",
> +# "machines": [
> +# "q35"
> +# ]
> +# }
> +# ],
> +# "features": [
> +# "acpi-s3",
> +# "amd-sev",
> +# "requires-smm",
> +# "secure-boot",
> +# "verbose-dynamic"
> +# ],
> +# "tags": [
> +# "-a IA32",
> +# "-a X64",
> +# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
> +# "-t GCC48",
> +# "-b DEBUG",
> +# "-D SMM_REQUIRE",
> +# "-D SECURE_BOOT_ENABLE",
> +# "-D FD_SIZE_4MB"
> +# ]
> +# }
> +#
> +# {
> +# "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled",
> +# "type": "uefi",
> +# "mapping": {
> +# "device": "flash",
> +# "executable": {
> +# "pathname": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +# "format": "raw"
> +# },
> +# "nvram_template": {
> +# "pathname": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
> +# "format": "raw"
> +# }
> +# },
> +# "targets": [
> +# {
> +# "architecture": "x86_64",
> +# "machines": [
> +# "q35"
> +# ]
> +# }
> +# ],
> +# "features": [
> +# "acpi-s3",
> +# "amd-sev",
> +# "requires-smm",
> +# "secure-boot",
> +# "secure-boot-enrolled-keys",
> +# "verbose-dynamic"
> +# ],
> +# "tags": [
> +# "-a IA32",
> +# "-a X64",
> +# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
> +# "-t GCC48",
> +# "-b DEBUG",
> +# "-D SMM_REQUIRE",
> +# "-D SECURE_BOOT_ENABLE",
> +# "-D FD_SIZE_4MB"
> +# ]
> +# }
> +#
> +# {
> +# "description": "UEFI firmware for ARM64 virtual machines",
> +# "type": "uefi",
> +# "mapping": {
> +# "device": "flash",
> +# "executable": {
> +# "pathname": "/usr/share/AAVMF/AAVMF_CODE.fd",
> +# "format": "raw"
> +# },
> +# "nvram_template": {
> +# "pathname": "/usr/share/AAVMF/AAVMF_VARS.fd",
> +# "format": "raw"
> +# }
> +# },
> +# "targets": [
> +# {
> +# "architecture": "aarch64",
> +# "machines": [
> +# "virt"
> +# ]
> +# }
> +# ],
> +# "features": [
> +#
> +# ],
> +# "tags": [
> +# "-a AARCH64",
> +# "-p ArmVirtPkg/ArmVirtQemu.dsc",
> +# "-t GCC48",
> +# "-b DEBUG",
> +# "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000"
> +# ]
> +# }
> +##
> +{ 'struct' : 'Firmware',
> + 'data' : { 'description' : 'str',
> + 'type' : 'FirmwareType',
> + 'mapping' : 'FirmwareMapping',
> + 'targets' : [ 'FirmwareTarget' ],
> + 'features' : [ 'FirmwareFeature' ],
> + 'tags' : [ 'str' ] } }
> +
> +##
> +# @x-check-firmware:
> +#
> +# Accept a @Firmware object and do nothing, successfully. This command can be
> +# used in the QMP shell to validate @Firmware JSON against the schema.
> +#
> +# @fw: ignored
> +#
> +# Since: 2.13
> +##
> +{ 'command' : 'x-check-firmware',
> + 'data' : { 'fw' : 'Firmware' } }
Introspection has a similar need for validating data against the schema,
but it solves it with a test case without adding a QMP command. Commit
39a18158165:
A new test-qmp-input-visitor test case feeds its result for both
tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
QmpInputVisitor to verify it actually conforms to the schema.
The test case has since moved to tests/test-qobject-input-visitor.c,
function test_visitor_in_qmp_introspect(). Please check it out to see
whether you can do without a QMP command, too.
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 25bce78352b8..2d6339ca8c99 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -92,4 +92,5 @@
> { 'include': 'transaction.json' }
> { 'include': 'trace.json' }
> { 'include': 'introspect.json' }
> +{ 'include': 'firmware.json' }
> { 'include': 'misc.json' }
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 10:47, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
>> +##
>> +# @FirmwareArchitecture:
>> +#
>> +# Defines the target architectures (emulator binaries) that firmware may
>> +# execute on.
>> +#
>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
>> +#
>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
>> +#
>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
>> +#
>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum' : 'FirmwareArchitecture',
>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>
> Partial dupe of CpuInfoArch from misc.json. Neither covers all our
> target architectures. Should we have one that does in common.json
> instead?
If we had one there, I'd use it here :)
For collecting the arch identifiers, is it OK to run "./configure
--help", and look for the "*-softmmu" options under
"--target-list=LIST"? Because that's what I need here; the qemu-system-*
emulators.
>> +##
>> +# @FirmwareFeature:
>> +#
>> +# Defines the features that firmware may support, and the platform requirements
>> +# that firmware may present.
>> +#
>> +# @acpi-s3: The firmware supports S3 sleep (suspend to RAM), as defined in the
>> +# ACPI specification. On the "pc" machine type of the @i386 and
>> +# @x86_64 emulation targets, S3 can be enabled with "-global
>> +# PIIX4_PM.disable_s3=0" and disabled with "-global
>> +# PIIX4_PM.disable_s3=1". On the "q35" machine type of the @i386 and
>> +# @x86_64 emulation targets, S3 can be enabled with "-global
>> +# ICH9-LPC.disable_s3=0" and disabled with "-global
>> +# ICH9-LPC.disable_s3=1".
>> +#
>> +# @acpi-s4: The firmware supports S4 hibernation (suspend to disk), as defined
>> +# in the ACPI specification. On the "pc" machine type of the @i386
>> +# and @x86_64 emulation targets, S4 can be enabled with "-global
>> +# PIIX4_PM.disable_s4=0" and disabled with "-global
>> +# PIIX4_PM.disable_s4=1". On the "q35" machine type of the @i386 and
>> +# @x86_64 emulation targets, S4 can be enabled with "-global
>> +# ICH9-LPC.disable_s4=0" and disabled with "-global
>> +# ICH9-LPC.disable_s4=1".
>
> Would you mind breaking documentation lines a bit ealier?
Not at all; I aimed at 79 characters (like I do for the QEMU C source
code as well), but perhaps that's too wide for schema docs. What width
do you prefer?
While at it: is it possible to break the documentation for a single
@entry to multiple paragraphs? I tried it (simply by inserting an empty
line, without starting another @entry), and the generator didn't like it.
>> +##
>> +# @FirmwareFlashFile:
>> +#
>> +# Defines common properties that are necessary for loading a firmware file into
>> +# a pflash chip. The corresponding QEMU command line option is "-drive
>> +# file=@pathname,format=@format". Note however that the option-argument shown
>> +# here is incomplete; it is completed under @FirmwareMappingFlash.
>> +#
>> +# @pathname: Specifies the pathname on the host filesystem where the firmware
>> +# file can be found.
>
> We use @filename elsewhere in the QAPI schema. Let's stick to that.
> More of the same below.
Will do.
>> +#
>> +# @format: Specifies the block format of the file pointed-to by @pathname, such
>> +# as @raw or @qcow2.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct' : 'FirmwareFlashFile',
>> + 'data' : { 'pathname' : 'str',
>> + 'format' : 'BlockdevDriver' } }
>> +
>> +##
>> +# @FirmwareMappingFlash:
>> +#
>> +# Describes loading and mapping properties for the firmware executable and its
>> +# accompanying NVRAM file, when @FirmwareDevice is @flash.
>> +#
>> +# @executable: Identifies the firmware executable. The firmware executable may
>> +# be shared by multiple virtual machine definitions. The
>> +# corresponding QEMU command line option is "-drive
>> +# if=pflash,unit=0,readonly=on,file=@executable.@pathname,format=@executable.@format".
>
> I guess @executable.@pathname means member @pathname of @executable. I
> read it as two distinct parameters first, then wondered where parameter
> @pathname is. Perhaps @executable.pathname would be clearer.
I can try that, yes -- in the HTML documentation, will the monospace
style apply to the full field specification?
>
>> +#
>> +# @nvram_template: Identifies the NVRAM template compatible with @executable.
>> +# Management software instantiates an individual copy -- a
>> +# specific NVRAM file -- from @nvram_template.@pathname for
>> +# each new virtual machine definition created.
>> +# @nvram_template.@pathname itself is never mapped into
>> +# virtual machines, only individual copies of it are. An NVRAM
>> +# file is typically used for persistently storing the
>> +# non-volatile UEFI variables of a virtual machine definition.
>> +# The corresponding QEMU command line option is "-drive
>> +# if=pflash,unit=1,readonly=off,file=PATHNAME_OF_PRIVATE_NVRAM_FILE,format=@nvram_template.@format".
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct' : 'FirmwareMappingFlash',
>> + 'data' : { 'executable' : 'FirmwareFlashFile',
>> + 'nvram_template' : 'FirmwareFlashFile' } }
Here's one thing I'll comment on myself: "nvram_template" should have
been spelled "nvram-template" :)
>> +
>> +##
>> +# @FirmwareMappingKernel:
>> +#
>> +# Describes loading and mapping properties for the firmware executable, when
>> +# @FirmwareDevice is @kernel.
>> +#
>> +# @pathname: Identifies the firmware executable. The firmware executable may be
>> +# shared by multiple virtual machine definitions. The corresponding
>> +# QEMU command line option is "-kernel @pathname".
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct' : 'FirmwareMappingKernel',
>> + 'data' : { 'pathname' : 'str' } }
>> +
>> +##
>> +# @FirmwareMappingMemory:
>> +#
>> +# Describes loading and mapping properties for the firmware executable, when
>> +# @FirmwareDevice is @memory.
>> +#
>> +# @pathname: Identifies the firmware executable. The firmware executable may be
>> +# shared by multiple virtual machine definitions. The corresponding
>> +# QEMU command line option is "-bios @pathname".
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct' : 'FirmwareMappingMemory',
>> + 'data' : { 'pathname' : 'str' } }
>> +
>> +##
>> +# @FirmwareMapping:
>> +#
>> +# Provides a discriminated structure for firmware to describe its loading /
>> +# mapping properties.
>> +#
>> +# @device: Selects the device type that the firmware must be mapped into.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'union' : 'FirmwareMapping',
>> + 'base' : { 'device' : 'FirmwareDevice' },
>> + 'discriminator' : 'device',
>> + 'data' : { 'flash' : 'FirmwareMappingFlash',
>> + 'kernel' : 'FirmwareMappingKernel',
>> + 'memory' : 'FirmwareMappingMemory' } }
>
> The FirmwareMapping* all have a member @pathname. It could be moved to
> the base. Makes sense if we think future FirmwareMappingFOOs will also
> have it. Your choice.
I could do that, but there would be consequences:
- FirmwareMappingKernel and FirmwareMappingMemory would become empty
structures,
- in the documentation of @FirmwareMapping, I couldn't document @flash /
@kernel / @memory individually (I tried -- it doesn't work; the
generator wants to generate the documentation automatically).
The issue is that I would like to keep the QEMU cmdline options
"-kernel" and "-bios" *close* to @FirmwareMappingKernel and
@FirmwareMappingMemory. Now, if I make those structs empty, but keep the
-kernel / -bios cmdline options right under them, then I cannot refer to
@pathname (well, @filename) in those options -- because @filename will
no longer be defined under @FirmwareMappingKernel / @FirmwareMappingMemory.
And if I move the documentation of -kernel / -bios under
@FirmwareMapping, then it's awkward again, because then I have to dump
both -kernel and -bios into the documentation of @filename, and explain
which belongs to which union member / discriminator value.
So, if it's not a problem, I'd like to stick with this variant, for the
sake of documenting -kernel and -bios more clearly.
>> +##
>> +# @Firmware:
>> +#
>> +# Describes a firmware (or a firmware use case) to management software.
>> +#
>> +# @description: Provides a human-readable description of the firmware.
>> +# Management software may or may not display @description.
>> +#
>> +# @type: Exposes the type of the firmware. @type is generally the primary key
>> +# for which management software looks when picking a firmware for a new
>> +# virtual machine definition.
>> +#
>> +# @mapping: Describes the loading / mapping properties of the firmware.
>> +#
>> +# @targets: Collects the target architectures (QEMU system emulators) and their
>> +# machine types that may execute the firmware.
>> +#
>> +# @features: Lists the features that the firmware supports, and the platform
>> +# requirements it presents.
>> +#
>> +# @tags: A list of auxiliary strings associated with the firmware for which
>> +# @description is not approprite, due to the latter's possible exposure
>
> s/approprite/appropriate/
>
> Feed the new comments to a spell checker?
Right, I got "aspell" installed, and sometimes run it on my status
reports :) I was too tired to do it for the schema. Good catch, thanks.
> Introspection has a similar need for validating data against the schema,
> but it solves it with a test case without adding a QMP command. Commit
> 39a18158165:
>
> A new test-qmp-input-visitor test case feeds its result for both
> tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
> QmpInputVisitor to verify it actually conforms to the schema.
>
> The test case has since moved to tests/test-qobject-input-visitor.c,
> function test_visitor_in_qmp_introspect(). Please check it out to see
> whether you can do without a QMP command, too.
Paolo suggested earlier that no C code should be added ultimately; the
schema should be moved under "docs/interop/". I was OK with that, just
wanted to keep the examples verifiable against the schema until the
patch got committed:
http://mid.mail-archive.com/9d6b3e23-ed02-0079-50d0-15de8b11810f@redhat.com
So in the non-RFC version, the QMP command shouldn't exist at all.
Thanks,
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Laszlo Ersek <lersek@redhat.com> writes:
> On 04/18/18 10:47, Markus Armbruster wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>
>>> +##
>>> +# @FirmwareArchitecture:
>>> +#
>>> +# Defines the target architectures (emulator binaries) that firmware may
>>> +# execute on.
>>> +#
>>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
>>> +#
>>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
>>> +#
>>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
>>> +#
>>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
>>> +#
>>> +# Since: 2.13
>>> +##
>>> +{ 'enum' : 'FirmwareArchitecture',
>>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>>
>> Partial dupe of CpuInfoArch from misc.json. Neither covers all our
>> target architectures. Should we have one that does in common.json
>> instead?
>
> If we had one there, I'd use it here :)
>
> For collecting the arch identifiers, is it OK to run "./configure
> --help", and look for the "*-softmmu" options under
> "--target-list=LIST"? Because that's what I need here; the qemu-system-*
> emulators.
configure gets its list like this:
default_target_list=""
mak_wilds=""
if [ "$softmmu" = "yes" ]; then
mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
fi
if [ "$linux_user" = "yes" ]; then
mak_wilds="${mak_wilds} $source_path/default-configs/*-linux-user.mak"
fi
if [ "$bsd_user" = "yes" ]; then
mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
fi
for config in $mak_wilds; do
default_target_list="${default_target_list} $(basename "$config" .mak)"
done
Since we use QMP only in system emulation, a QAPI enum limited to the
system emulation targets makes sense.
Replacing CpuInfoArch by such an enum will change the discriminator
value from "other" to the real architecture, with the obvious
compatibility concerns. But we've accepted similar changes twice
already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
"other" was a bad idea. Hindsight 20/20.
Getting rid of it in one go rather than piecemeal seems like the least
bad way out. Too late for 2.12, though. Eric, what do you think?
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
>
> > On 04/18/18 10:47, Markus Armbruster wrote:
> >> Laszlo Ersek <lersek@redhat.com> writes:
> >
> >>> +##
> >>> +# @FirmwareArchitecture:
> >>> +#
> >>> +# Defines the target architectures (emulator binaries) that firmware may
> >>> +# execute on.
> >>> +#
> >>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
> >>> +#
> >>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
> >>> +#
> >>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
> >>> +#
> >>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
> >>> +#
> >>> +# Since: 2.13
> >>> +##
> >>> +{ 'enum' : 'FirmwareArchitecture',
> >>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
> >>
> >> Partial dupe of CpuInfoArch from misc.json. Neither covers all our
> >> target architectures. Should we have one that does in common.json
> >> instead?
> >
> > If we had one there, I'd use it here :)
> >
> > For collecting the arch identifiers, is it OK to run "./configure
> > --help", and look for the "*-softmmu" options under
> > "--target-list=LIST"? Because that's what I need here; the qemu-system-*
> > emulators.
>
> configure gets its list like this:
>
> default_target_list=""
>
> mak_wilds=""
>
> if [ "$softmmu" = "yes" ]; then
> mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
> fi
> if [ "$linux_user" = "yes" ]; then
> mak_wilds="${mak_wilds} $source_path/default-configs/*-linux-user.mak"
> fi
> if [ "$bsd_user" = "yes" ]; then
> mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
> fi
>
> for config in $mak_wilds; do
> default_target_list="${default_target_list} $(basename "$config" .mak)"
> done
>
> Since we use QMP only in system emulation, a QAPI enum limited to the
> system emulation targets makes sense.
>
> Replacing CpuInfoArch by such an enum will change the discriminator
> value from "other" to the real architecture, with the obvious
> compatibility concerns. But we've accepted similar changes twice
> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
>
> "other" was a bad idea. Hindsight 20/20.
>
> Getting rid of it in one go rather than piecemeal seems like the least
> bad way out. Too late for 2.12, though. Eric, what do you think?
Given the context in which this "other" value is used, I think it is
reasonable to kill it and put a full arch list in there.
No app is likely to be accessing the struct under "other" because it
is just an empty placeholder.
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
On 04/19/18 09:56, Daniel P. Berrangé wrote:
> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>>
>>> On 04/18/18 10:47, Markus Armbruster wrote:
>>>> Laszlo Ersek <lersek@redhat.com> writes:
>>>
>>>>> +##
>>>>> +# @FirmwareArchitecture:
>>>>> +#
>>>>> +# Defines the target architectures (emulator binaries) that firmware may
>>>>> +# execute on.
>>>>> +#
>>>>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
>>>>> +#
>>>>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
>>>>> +#
>>>>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
>>>>> +#
>>>>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
>>>>> +#
>>>>> +# Since: 2.13
>>>>> +##
>>>>> +{ 'enum' : 'FirmwareArchitecture',
>>>>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>>>>
>>>> Partial dupe of CpuInfoArch from misc.json. Neither covers all our
>>>> target architectures. Should we have one that does in common.json
>>>> instead?
>>>
>>> If we had one there, I'd use it here :)
>>>
>>> For collecting the arch identifiers, is it OK to run "./configure
>>> --help", and look for the "*-softmmu" options under
>>> "--target-list=LIST"? Because that's what I need here; the qemu-system-*
>>> emulators.
>>
>> configure gets its list like this:
>>
>> default_target_list=""
>>
>> mak_wilds=""
>>
>> if [ "$softmmu" = "yes" ]; then
>> mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
>> fi
>> if [ "$linux_user" = "yes" ]; then
>> mak_wilds="${mak_wilds} $source_path/default-configs/*-linux-user.mak"
>> fi
>> if [ "$bsd_user" = "yes" ]; then
>> mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
>> fi
>>
>> for config in $mak_wilds; do
>> default_target_list="${default_target_list} $(basename "$config" .mak)"
>> done
>>
>> Since we use QMP only in system emulation, a QAPI enum limited to the
>> system emulation targets makes sense.
>>
>> Replacing CpuInfoArch by such an enum will change the discriminator
>> value from "other" to the real architecture, with the obvious
>> compatibility concerns. But we've accepted similar changes twice
>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
>>
>> "other" was a bad idea. Hindsight 20/20.
>>
>> Getting rid of it in one go rather than piecemeal seems like the least
>> bad way out. Too late for 2.12, though. Eric, what do you think?
>
> Given the context in which this "other" value is used, I think it is
> reasonable to kill it and put a full arch list in there.
>
> No app is likely to be accessing the struct under "other" because it
> is just an empty placeholder.
Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
potential to confuse QMP clients that didn't expect "s390", but
otherwise it didn't mess with preexistent enum values / structures.
The same applies to commit 25fa194b7b1, just with "riscv" /
"CpuInfoRISCV" substituted.
Removing "other" might confuse QMP clients that expect it, except
(according to Daniel) no such client exists, probably.
However, replacing the current list of CpuInfoArch constants with the
system emulation target list would be more intrusive than all three of
the above. In particular there is no "x86" target; only i386 and x86_64
targets exist. For the firmware schema, this distinction is important,
but it would break QMP clients that expect "x86" (and such clients must
certainly exist).
The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
That is, at least the following constants in CpuInfoArch have no
corresponding (identical) mapping -- I'll state to the right of the
arrow the emulation targets which I know or presume to be associated
with the CpuInfoArch constant:
- x86 -> i386, x86_64
- sparc -> sparc, sparc64
- ppc -> ppc, ppc64, ppcemb
- mips -> mips, mips64, mips64el, mipsel
- s390 -> s390x
- riscv -> riscv32, riscv64
The only constant that seems to have a 1:1 mapping is "tricore", but I
could perfectly well be thinking even that just because I have no clue
about "tricore".
So, I don't think CpuInfoArch can be updated so that it both remains
compatible with current QMP clients, and serves "firmware.json". In the
firmware schema we don't just need CPU architecture, but actual emulator
names (and board / machine types).
I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
that remotely matches is the @TargetInfo structure, in which the @arch
field is a string, coming with the example "x86_64". The example also
names "i386" separately.
So what might make sense is to introduce a separate enum in
"qapi/misc.json" with all the softmmu targets I listed above, and change
the type of @TargetInfo.@arch to that enum. In parallel,
qmp_query_target() would have to be updated to look up the enum value
matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
for collecting the relevant arches here: @TargetInfo is only used in the
"query-target" QMP command, and Markus said above that QMP is only used
with system emulation.)
Should I do this?
Thanks,
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: > On 04/19/18 09:56, Daniel P. Berrangé wrote: > > On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: > >> Laszlo Ersek <lersek@redhat.com> writes: > >> > >>> On 04/18/18 10:47, Markus Armbruster wrote: > >>>> Laszlo Ersek <lersek@redhat.com> writes: > >> Replacing CpuInfoArch by such an enum will change the discriminator > >> value from "other" to the real architecture, with the obvious > >> compatibility concerns. But we've accepted similar changes twice > >> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. > >> > >> "other" was a bad idea. Hindsight 20/20. > >> > >> Getting rid of it in one go rather than piecemeal seems like the least > >> bad way out. Too late for 2.12, though. Eric, what do you think? > > > > Given the context in which this "other" value is used, I think it is > > reasonable to kill it and put a full arch list in there. > > > > No app is likely to be accessing the struct under "other" because it > > is just an empty placeholder. > > Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the > potential to confuse QMP clients that didn't expect "s390", but > otherwise it didn't mess with preexistent enum values / structures. NB, qemu-system-s390x would previously have returned "other" in this field, and now it returns "s390". So while it didn't remove "other" from the list of things that could potentially exist, it did change what the s390x binary will actually report. > The same applies to commit 25fa194b7b1, just with "riscv" / > "CpuInfoRISCV" substituted. > > Removing "other" might confuse QMP clients that expect it, except > (according to Daniel) no such client exists, probably. When I say removing "other", I imply that we add an explicit arch for all those which we currently are missing. IOW, all qemu-system-XXX binaries which currently report "other" would change to report their respective "XXX" values. So in this way, it is exactly the same as what we did when we introduced "s390" as an option. The only difference is that once we have every binary reporting the correct arch, we can now also remove "other" from the schema itself as it will then be unused. 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
On 04/19/18 11:12, Daniel P. Berrangé wrote: > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: >> On 04/19/18 09:56, Daniel P. Berrangé wrote: >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: >>>> Laszlo Ersek <lersek@redhat.com> writes: >>>> >>>>> On 04/18/18 10:47, Markus Armbruster wrote: >>>>>> Laszlo Ersek <lersek@redhat.com> writes: >>>> Replacing CpuInfoArch by such an enum will change the discriminator >>>> value from "other" to the real architecture, with the obvious >>>> compatibility concerns. But we've accepted similar changes twice >>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. >>>> >>>> "other" was a bad idea. Hindsight 20/20. >>>> >>>> Getting rid of it in one go rather than piecemeal seems like the least >>>> bad way out. Too late for 2.12, though. Eric, what do you think? >>> >>> Given the context in which this "other" value is used, I think it is >>> reasonable to kill it and put a full arch list in there. >>> >>> No app is likely to be accessing the struct under "other" because it >>> is just an empty placeholder. >> >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the >> potential to confuse QMP clients that didn't expect "s390", but >> otherwise it didn't mess with preexistent enum values / structures. > > NB, qemu-system-s390x would previously have returned "other" in > this field, and now it returns "s390". So while it didn't > remove "other" from the list of things that could potentially > exist, it did change what the s390x binary will actually report. > >> The same applies to commit 25fa194b7b1, just with "riscv" / >> "CpuInfoRISCV" substituted. >> >> Removing "other" might confuse QMP clients that expect it, except >> (according to Daniel) no such client exists, probably. > > When I say removing "other", I imply that we add an explicit arch > for all those which we currently are missing. IOW, all qemu-system-XXX > binaries which currently report "other" would change to report their > respective "XXX" values. > > So in this way, it is exactly the same as what we did when we > introduced "s390" as an option. > > The only difference is that once we have every binary reporting the > correct arch, we can now also remove "other" from the schema itself > as it will then be unused. Can we please translate this into more actionable items for me, because I'm getting confused :) First, if I add "i386" and "x86_64" to the enum list, we'll have all three of "i386", "x86_64" and "x86". Is that useful? How will that work? Second, assuming I add constants for the ~10 (?) softmmu arches, can I still use @CpuInfoOther as the type for the corresponding new members in @CpuInfo? What C code changes will be necessary? Thanks Laszlo
On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote: > On 04/19/18 11:12, Daniel P. Berrangé wrote: > > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: > >> On 04/19/18 09:56, Daniel P. Berrangé wrote: > >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: > >>>> Laszlo Ersek <lersek@redhat.com> writes: > >>>> > >>>>> On 04/18/18 10:47, Markus Armbruster wrote: > >>>>>> Laszlo Ersek <lersek@redhat.com> writes: > >>>> Replacing CpuInfoArch by such an enum will change the discriminator > >>>> value from "other" to the real architecture, with the obvious > >>>> compatibility concerns. But we've accepted similar changes twice > >>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. > >>>> > >>>> "other" was a bad idea. Hindsight 20/20. > >>>> > >>>> Getting rid of it in one go rather than piecemeal seems like the least > >>>> bad way out. Too late for 2.12, though. Eric, what do you think? > >>> > >>> Given the context in which this "other" value is used, I think it is > >>> reasonable to kill it and put a full arch list in there. > >>> > >>> No app is likely to be accessing the struct under "other" because it > >>> is just an empty placeholder. > >> > >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the > >> potential to confuse QMP clients that didn't expect "s390", but > >> otherwise it didn't mess with preexistent enum values / structures. > > > > NB, qemu-system-s390x would previously have returned "other" in > > this field, and now it returns "s390". So while it didn't > > remove "other" from the list of things that could potentially > > exist, it did change what the s390x binary will actually report. > > > >> The same applies to commit 25fa194b7b1, just with "riscv" / > >> "CpuInfoRISCV" substituted. > >> > >> Removing "other" might confuse QMP clients that expect it, except > >> (according to Daniel) no such client exists, probably. > > > > When I say removing "other", I imply that we add an explicit arch > > for all those which we currently are missing. IOW, all qemu-system-XXX > > binaries which currently report "other" would change to report their > > respective "XXX" values. > > > > So in this way, it is exactly the same as what we did when we > > introduced "s390" as an option. > > > > The only difference is that once we have every binary reporting the > > correct arch, we can now also remove "other" from the schema itself > > as it will then be unused. > > Can we please translate this into more actionable items for me, because > I'm getting confused :) > > First, if I add "i386" and "x86_64" to the enum list, we'll have all > three of "i386", "x86_64" and "x86". Is that useful? How will that work? Hmm, yes, on closer look this is a big mess as it is. We've been using generic terms for covering multiple architectures :-( 'x86' for both i386 and x86_64, 'sparc' for sparc and sparc64, etc. If we try to fix that we'll be entering a world of backcompat hurt :-( Since your schema is likely to end up just being a file in docs/specs, rather than directly part of our existnig qapi schema, I suggest we just ignore whats there. Just define an arch enum in your spec which is right, and let someone else worry about fixing the mess > Second, assuming I add constants for the ~10 (?) softmmu arches, can I > still use @CpuInfoOther as the type for the corresponding new members in > @CpuInfo? What C code changes will be necessary? Yes, we could still use the CpuInfoOther struct, since struct names are invisible to consumers, but as above, lets ignore the mess 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
Hi, > Since your schema is likely to end up just being a file in docs/specs, > rather than directly part of our existnig qapi schema, I suggest we just > ignore whats there. Just define an arch enum in your spec which is right, > and let someone else worry about fixing the mess I think it would be useful to have this as part of the schema. Should be easy then to write up a small utility then which takes a json file as input and translates that into qemu command line options. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Apr 20, 2018 at 11:46:24AM +0200, Gerd Hoffmann wrote: > Hi, > > > Since your schema is likely to end up just being a file in docs/specs, > > rather than directly part of our existnig qapi schema, I suggest we just > > ignore whats there. Just define an arch enum in your spec which is right, > > and let someone else worry about fixing the mess > > I think it would be useful to have this as part of the schema. Should > be easy then to write up a small utility then which takes a json file as > input and translates that into qemu command line options. Currently we have two distinct QAPI schemas, one covering the system emulator for QMP and subset of CLI args, and one covering the guest agent. This new schema isn't mapping to anything in QMP / CLI right now though. It can be used to decide how to generate CLI args, but it isn't describing those CLI args. So to me this is a 3rd distinct schema. 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
> > > Since your schema is likely to end up just being a file in docs/specs, > > I think it would be useful to have this as part of the schema. Should > > be easy then to write up a small utility then which takes a json file as > > input and translates that into qemu command line options. > > Currently we have two distinct QAPI schemas, one covering the system > emulator for QMP and subset of CLI args, and one covering the guest > agent. Yep, we can make it a third one, fine with me. But it likewise wouldn't end up in docs/ then. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/20/18 12:41, Gerd Hoffmann wrote: >>>> Since your schema is likely to end up just being a file in docs/specs, > >>> I think it would be useful to have this as part of the schema. Should >>> be easy then to write up a small utility then which takes a json file as >>> input and translates that into qemu command line options. >> >> Currently we have two distinct QAPI schemas, one covering the system >> emulator for QMP and subset of CLI args, and one covering the guest >> agent. > > Yep, we can make it a third one, fine with me. > But it likewise wouldn't end up in docs/ then. Let's please focus on the schema itself for now, with the stupid little test command included. Once the schema enjoys universal popularity, I'll be glad to pass it on to QMP wizards -- because, if we ultimately wanted to keep the schema code-enabled, then Markus's earlier suggestions apply too, regarding QMP test cases. I'll admit I haven't dared look at those yet (in particular because Paolo originally suggested to move this under docs/interop). Looking at my todo list, I'd really like to focus on the schema itself. Thanks Laszlo
On 04/20/18 11:34, Daniel P. Berrangé wrote: > On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote: >> On 04/19/18 11:12, Daniel P. Berrangé wrote: >>> On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: >>>> On 04/19/18 09:56, Daniel P. Berrangé wrote: >>>>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: >>>>>> Laszlo Ersek <lersek@redhat.com> writes: >>>>>> >>>>>>> On 04/18/18 10:47, Markus Armbruster wrote: >>>>>>>> Laszlo Ersek <lersek@redhat.com> writes: >>>>>> Replacing CpuInfoArch by such an enum will change the discriminator >>>>>> value from "other" to the real architecture, with the obvious >>>>>> compatibility concerns. But we've accepted similar changes twice >>>>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. >>>>>> >>>>>> "other" was a bad idea. Hindsight 20/20. >>>>>> >>>>>> Getting rid of it in one go rather than piecemeal seems like the least >>>>>> bad way out. Too late for 2.12, though. Eric, what do you think? >>>>> >>>>> Given the context in which this "other" value is used, I think it is >>>>> reasonable to kill it and put a full arch list in there. >>>>> >>>>> No app is likely to be accessing the struct under "other" because it >>>>> is just an empty placeholder. >>>> >>>> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the >>>> potential to confuse QMP clients that didn't expect "s390", but >>>> otherwise it didn't mess with preexistent enum values / structures. >>> >>> NB, qemu-system-s390x would previously have returned "other" in >>> this field, and now it returns "s390". So while it didn't >>> remove "other" from the list of things that could potentially >>> exist, it did change what the s390x binary will actually report. >>> >>>> The same applies to commit 25fa194b7b1, just with "riscv" / >>>> "CpuInfoRISCV" substituted. >>>> >>>> Removing "other" might confuse QMP clients that expect it, except >>>> (according to Daniel) no such client exists, probably. >>> >>> When I say removing "other", I imply that we add an explicit arch >>> for all those which we currently are missing. IOW, all qemu-system-XXX >>> binaries which currently report "other" would change to report their >>> respective "XXX" values. >>> >>> So in this way, it is exactly the same as what we did when we >>> introduced "s390" as an option. >>> >>> The only difference is that once we have every binary reporting the >>> correct arch, we can now also remove "other" from the schema itself >>> as it will then be unused. >> >> Can we please translate this into more actionable items for me, because >> I'm getting confused :) >> >> First, if I add "i386" and "x86_64" to the enum list, we'll have all >> three of "i386", "x86_64" and "x86". Is that useful? How will that work? > > Hmm, yes, on closer look this is a big mess as it is. We've been using > generic terms for covering multiple architectures :-( 'x86' for both > i386 and x86_64, 'sparc' for sparc and sparc64, etc. If we try to fix > that we'll be entering a world of backcompat hurt :-( > > Since your schema is likely to end up just being a file in docs/specs, > rather than directly part of our existnig qapi schema, I suggest we just > ignore whats there. Just define an arch enum in your spec which is right, > and let someone else worry about fixing the mess I can't tell you how much I love this idea. :) Thanks! Laszlo > >> Second, assuming I add constants for the ~10 (?) softmmu arches, can I >> still use @CpuInfoOther as the type for the corresponding new members in >> @CpuInfo? What C code changes will be necessary? > > Yes, we could still use the CpuInfoOther struct, since struct names are > invisible to consumers, but as above, lets ignore the mess > > Regards, > Daniel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Apr 20, 2018 at 10:34:57AM +0100, Daniel P. Berrangé wrote: > On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote: > > On 04/19/18 11:12, Daniel P. Berrangé wrote: > > > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: > > >> On 04/19/18 09:56, Daniel P. Berrangé wrote: > > >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: > > >>>> Laszlo Ersek <lersek@redhat.com> writes: > > >>>> > > >>>>> On 04/18/18 10:47, Markus Armbruster wrote: > > >>>>>> Laszlo Ersek <lersek@redhat.com> writes: > > >>>> Replacing CpuInfoArch by such an enum will change the discriminator > > >>>> value from "other" to the real architecture, with the obvious > > >>>> compatibility concerns. But we've accepted similar changes twice > > >>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. > > >>>> > > >>>> "other" was a bad idea. Hindsight 20/20. > > >>>> > > >>>> Getting rid of it in one go rather than piecemeal seems like the least > > >>>> bad way out. Too late for 2.12, though. Eric, what do you think? > > >>> > > >>> Given the context in which this "other" value is used, I think it is > > >>> reasonable to kill it and put a full arch list in there. > > >>> > > >>> No app is likely to be accessing the struct under "other" because it > > >>> is just an empty placeholder. > > >> > > >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the > > >> potential to confuse QMP clients that didn't expect "s390", but > > >> otherwise it didn't mess with preexistent enum values / structures. > > > > > > NB, qemu-system-s390x would previously have returned "other" in > > > this field, and now it returns "s390". So while it didn't > > > remove "other" from the list of things that could potentially > > > exist, it did change what the s390x binary will actually report. > > > > > >> The same applies to commit 25fa194b7b1, just with "riscv" / > > >> "CpuInfoRISCV" substituted. > > >> > > >> Removing "other" might confuse QMP clients that expect it, except > > >> (according to Daniel) no such client exists, probably. > > > > > > When I say removing "other", I imply that we add an explicit arch > > > for all those which we currently are missing. IOW, all qemu-system-XXX > > > binaries which currently report "other" would change to report their > > > respective "XXX" values. > > > > > > So in this way, it is exactly the same as what we did when we > > > introduced "s390" as an option. > > > > > > The only difference is that once we have every binary reporting the > > > correct arch, we can now also remove "other" from the schema itself > > > as it will then be unused. > > > > Can we please translate this into more actionable items for me, because > > I'm getting confused :) > > > > First, if I add "i386" and "x86_64" to the enum list, we'll have all > > three of "i386", "x86_64" and "x86". Is that useful? How will that work? > > Hmm, yes, on closer look this is a big mess as it is. We've been using > generic terms for covering multiple architectures :-( 'x86' for both > i386 and x86_64, 'sparc' for sparc and sparc64, etc. If we try to fix > that we'll be entering a world of backcompat hurt :-( > > Since your schema is likely to end up just being a file in docs/specs, > rather than directly part of our existnig qapi schema, I suggest we just > ignore whats there. Just define an arch enum in your spec which is right, > and let someone else worry about fixing the mess Trouble is, for these "biarch" cases, I'm not sure it's always clear what the right value for a firmware is. While whether a userspace binary is i386 or x86_64 is clear and well-defined, a firmware could well be responsible for switching the CPU from its reset mode into the more modern 64-bit mode, and would therefore have at least some code in both archs. > > Second, assuming I add constants for the ~10 (?) softmmu arches, can I > > still use @CpuInfoOther as the type for the corresponding new members in > > @CpuInfo? What C code changes will be necessary? > > Yes, we could still use the CpuInfoOther struct, since struct names are > invisible to consumers, but as above, lets ignore the mess > > Regards, > Daniel -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Apr 23, 2018 at 10:10:10AM +1000, David Gibson wrote: > On Fri, Apr 20, 2018 at 10:34:57AM +0100, Daniel P. Berrangé wrote: > > On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote: > > > On 04/19/18 11:12, Daniel P. Berrangé wrote: > > > > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: > > > >> On 04/19/18 09:56, Daniel P. Berrangé wrote: > > > >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: > > > >>>> Laszlo Ersek <lersek@redhat.com> writes: > > > >>>> > > > >>>>> On 04/18/18 10:47, Markus Armbruster wrote: > > > >>>>>> Laszlo Ersek <lersek@redhat.com> writes: > > > >>>> Replacing CpuInfoArch by such an enum will change the discriminator > > > >>>> value from "other" to the real architecture, with the obvious > > > >>>> compatibility concerns. But we've accepted similar changes twice > > > >>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0. > > > >>>> > > > >>>> "other" was a bad idea. Hindsight 20/20. > > > >>>> > > > >>>> Getting rid of it in one go rather than piecemeal seems like the least > > > >>>> bad way out. Too late for 2.12, though. Eric, what do you think? > > > >>> > > > >>> Given the context in which this "other" value is used, I think it is > > > >>> reasonable to kill it and put a full arch list in there. > > > >>> > > > >>> No app is likely to be accessing the struct under "other" because it > > > >>> is just an empty placeholder. > > > >> > > > >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the > > > >> potential to confuse QMP clients that didn't expect "s390", but > > > >> otherwise it didn't mess with preexistent enum values / structures. > > > > > > > > NB, qemu-system-s390x would previously have returned "other" in > > > > this field, and now it returns "s390". So while it didn't > > > > remove "other" from the list of things that could potentially > > > > exist, it did change what the s390x binary will actually report. > > > > > > > >> The same applies to commit 25fa194b7b1, just with "riscv" / > > > >> "CpuInfoRISCV" substituted. > > > >> > > > >> Removing "other" might confuse QMP clients that expect it, except > > > >> (according to Daniel) no such client exists, probably. > > > > > > > > When I say removing "other", I imply that we add an explicit arch > > > > for all those which we currently are missing. IOW, all qemu-system-XXX > > > > binaries which currently report "other" would change to report their > > > > respective "XXX" values. > > > > > > > > So in this way, it is exactly the same as what we did when we > > > > introduced "s390" as an option. > > > > > > > > The only difference is that once we have every binary reporting the > > > > correct arch, we can now also remove "other" from the schema itself > > > > as it will then be unused. > > > > > > Can we please translate this into more actionable items for me, because > > > I'm getting confused :) > > > > > > First, if I add "i386" and "x86_64" to the enum list, we'll have all > > > three of "i386", "x86_64" and "x86". Is that useful? How will that work? > > > > Hmm, yes, on closer look this is a big mess as it is. We've been using > > generic terms for covering multiple architectures :-( 'x86' for both > > i386 and x86_64, 'sparc' for sparc and sparc64, etc. If we try to fix > > that we'll be entering a world of backcompat hurt :-( > > > > Since your schema is likely to end up just being a file in docs/specs, > > rather than directly part of our existnig qapi schema, I suggest we just > > ignore whats there. Just define an arch enum in your spec which is right, > > and let someone else worry about fixing the mess > > Trouble is, for these "biarch" cases, I'm not sure it's always clear > what the right value for a firmware is. While whether a userspace > binary is i386 or x86_64 is clear and well-defined, a firmware could > well be responsible for switching the CPU from its reset mode into the > more modern 64-bit mode, and would therefore have at least some code > in both archs. In the context of these firmware metadata files, the architecture is essentially the QEMU target binary architecture name, as it has an associated list of machine types from that binary. So 'i386' refers to qemu-system-i386, and the fact that qemu-system-x86_64 can run 'i386' is not applicable - you'd use the 'x86_64' firmware metadata rules for that. 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
Laszlo Ersek <lersek@redhat.com> writes:
> On 04/19/18 09:56, Daniel P. Berrangé wrote:
>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
>>> Laszlo Ersek <lersek@redhat.com> writes:
>>>
>>>> On 04/18/18 10:47, Markus Armbruster wrote:
>>>>> Laszlo Ersek <lersek@redhat.com> writes:
>>>>
>>>>>> +##
>>>>>> +# @FirmwareArchitecture:
>>>>>> +#
>>>>>> +# Defines the target architectures (emulator binaries) that firmware may
>>>>>> +# execute on.
>>>>>> +#
>>>>>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
>>>>>> +#
>>>>>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
>>>>>> +#
>>>>>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
>>>>>> +#
>>>>>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
>>>>>> +#
>>>>>> +# Since: 2.13
>>>>>> +##
>>>>>> +{ 'enum' : 'FirmwareArchitecture',
>>>>>> + 'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>>>>>
>>>>> Partial dupe of CpuInfoArch from misc.json. Neither covers all our
>>>>> target architectures. Should we have one that does in common.json
>>>>> instead?
>>>>
>>>> If we had one there, I'd use it here :)
>>>>
>>>> For collecting the arch identifiers, is it OK to run "./configure
>>>> --help", and look for the "*-softmmu" options under
>>>> "--target-list=LIST"? Because that's what I need here; the qemu-system-*
>>>> emulators.
>>>
>>> configure gets its list like this:
>>>
>>> default_target_list=""
>>>
>>> mak_wilds=""
>>>
>>> if [ "$softmmu" = "yes" ]; then
>>> mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
>>> fi
>>> if [ "$linux_user" = "yes" ]; then
>>> mak_wilds="${mak_wilds} $source_path/default-configs/*-linux-user.mak"
>>> fi
>>> if [ "$bsd_user" = "yes" ]; then
>>> mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
>>> fi
>>>
>>> for config in $mak_wilds; do
>>> default_target_list="${default_target_list} $(basename "$config" .mak)"
>>> done
>>>
>>> Since we use QMP only in system emulation, a QAPI enum limited to the
>>> system emulation targets makes sense.
>>>
>>> Replacing CpuInfoArch by such an enum will change the discriminator
>>> value from "other" to the real architecture, with the obvious
>>> compatibility concerns. But we've accepted similar changes twice
>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
>>>
>>> "other" was a bad idea. Hindsight 20/20.
>>>
>>> Getting rid of it in one go rather than piecemeal seems like the least
>>> bad way out. Too late for 2.12, though. Eric, what do you think?
>>
>> Given the context in which this "other" value is used, I think it is
>> reasonable to kill it and put a full arch list in there.
>>
>> No app is likely to be accessing the struct under "other" because it
>> is just an empty placeholder.
>
> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
> potential to confuse QMP clients that didn't expect "s390", but
> otherwise it didn't mess with preexistent enum values / structures.
>
> The same applies to commit 25fa194b7b1, just with "riscv" /
> "CpuInfoRISCV" substituted.
>
> Removing "other" might confuse QMP clients that expect it, except
> (according to Daniel) no such client exists, probably.
It's a done deal anyway; we're not going to hold 2.12 to avoid this QMP
compatibility break.
> However, replacing the current list of CpuInfoArch constants with the
> system emulation target list would be more intrusive than all three of
> the above. In particular there is no "x86" target; only i386 and x86_64
> targets exist. For the firmware schema, this distinction is important,
> but it would break QMP clients that expect "x86" (and such clients must
> certainly exist).
You're right. Another nice mess.
> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
>
> That is, at least the following constants in CpuInfoArch have no
> corresponding (identical) mapping -- I'll state to the right of the
> arrow the emulation targets which I know or presume to be associated
> with the CpuInfoArch constant:
> - x86 -> i386, x86_64
> - sparc -> sparc, sparc64
> - ppc -> ppc, ppc64, ppcemb
> - mips -> mips, mips64, mips64el, mipsel
> - s390 -> s390x
> - riscv -> riscv32, riscv64
>
> The only constant that seems to have a 1:1 mapping is "tricore", but I
> could perfectly well be thinking even that just because I have no clue
> about "tricore".
>
> So, I don't think CpuInfoArch can be updated so that it both remains
> compatible with current QMP clients, and serves "firmware.json". In the
> firmware schema we don't just need CPU architecture, but actual emulator
> names (and board / machine types).
The completely orthodox fix for CpuInfo would be:
* Keep @arch unchanged. In particular, keep "other" for all targets
other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'.
* Add a new member @target of new enum type Target, whose values match
configures idea of targets exactly.
* Make the new member the union tag.
It's too late for complete orthodoxy; we already changed @arch.
A common enum type Target makes sense anyway, I think.
Using it in CpuInfo as described above may make sense, too. Could be
left to a follow-up patch.
> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
> that remotely matches is the @TargetInfo structure, in which the @arch
> field is a string, coming with the example "x86_64". The example also
> names "i386" separately.
Well spotted.
TargetInfo member @arch is set to TARGET_NAME, which matches configure's
idea of the target. If we add enum Target, we should change @arch's
type from str to Target.
> So what might make sense is to introduce a separate enum in
> "qapi/misc.json" with all the softmmu targets I listed above, and change
> the type of @TargetInfo.@arch to that enum.
I arrived at this conclusion, too.
> In parallel,
> qmp_query_target() would have to be updated to look up the enum value
> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
> for collecting the relevant arches here: @TargetInfo is only used in the
> "query-target" QMP command, and Markus said above that QMP is only used
> with system emulation.)
>
> Should I do this?
Yes, please.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/20/18 14:53, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: [snip] >> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386, >> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel, >> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4, >> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb. >> >> That is, at least the following constants in CpuInfoArch have no >> corresponding (identical) mapping -- I'll state to the right of the >> arrow the emulation targets which I know or presume to be associated >> with the CpuInfoArch constant: >> - x86 -> i386, x86_64 >> - sparc -> sparc, sparc64 >> - ppc -> ppc, ppc64, ppcemb >> - mips -> mips, mips64, mips64el, mipsel >> - s390 -> s390x >> - riscv -> riscv32, riscv64 >> >> The only constant that seems to have a 1:1 mapping is "tricore", but I >> could perfectly well be thinking even that just because I have no clue >> about "tricore". >> >> So, I don't think CpuInfoArch can be updated so that it both remains >> compatible with current QMP clients, and serves "firmware.json". In the >> firmware schema we don't just need CPU architecture, but actual emulator >> names (and board / machine types). > > The completely orthodox fix for CpuInfo would be: > > * Keep @arch unchanged. In particular, keep "other" for all targets > other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'. > > * Add a new member @target of new enum type Target, whose values match > configures idea of targets exactly. > > * Make the new member the union tag. > > It's too late for complete orthodoxy; we already changed @arch. > > A common enum type Target makes sense anyway, I think. > > Using it in CpuInfo as described above may make sense, too. Could be > left to a follow-up patch. > >> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing >> that remotely matches is the @TargetInfo structure, in which the @arch >> field is a string, coming with the example "x86_64". The example also >> names "i386" separately. > > Well spotted. > > TargetInfo member @arch is set to TARGET_NAME, which matches configure's > idea of the target. If we add enum Target, we should change @arch's > type from str to Target. > >> So what might make sense is to introduce a separate enum in >> "qapi/misc.json" with all the softmmu targets I listed above, and change >> the type of @TargetInfo.@arch to that enum. > > I arrived at this conclusion, too. > >> In parallel, >> qmp_query_target() would have to be updated to look up the enum value >> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators >> for collecting the relevant arches here: @TargetInfo is only used in the >> "query-target" QMP command, and Markus said above that QMP is only used >> with system emulation.) >> >> Should I do this? > > Yes, please. OK, so here's my understanding: (1) I'll introduce a new @Target enum in misc.json. I'll inherit / include it in firmware.json. This is compatible with what Dan said. (2) I'll change @TargetInfo.@arch to the new type. I believe this will break the compilation of qmp_query_target(); I'll hack on that until it builds again, with the lookup. :) (3) Regarding the addition of @target to CpuInfo (accompanying @arch) doesn't look hard; what *does* look hard is changing the union discriminator from @arch to @target. @target has many more values, and I would have to map all of them to the (fewer) @arch values that currently do *not* select @CpuInfoOther. Here's an example: - Both @i386 and @x86_64 from @target mean @x86 in @arch, - because @x86 currently selects @CpuInfoX86, not @CpuInfoOther, both @i386 and @x86_64 must be assigned @CpuInfoX86. This depends on the knowledge that "x86" actually *means* "i386 plus x86_64", and I totally don't have that knowledge for the rest of the arches / targets. So, the modification of @CpuInfo I would indeed like to pass off to someone else. (Well, if all the architecture maintainers follow up and tell me what emulators exactly fall under the umbrella of each individual @arch value, I can post the patch.) BTW... I wonder how compatibility would be affected if we just added @target to @CpuInfo, even without making it the new discriminator. ... Anyway, I think I've gotten a huge amount of useful and actionable feedback; thanks everyone, let me work on RFCv3 and post it soon. :) Thanks! Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Laszlo Ersek <lersek@redhat.com> writes:
> On 04/20/18 14:53, Markus Armbruster wrote:
>> Laszlo Ersek <lersek@redhat.com> writes:
>
> [snip]
>
>>> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
>>> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
>>> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
>>> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
>>>
>>> That is, at least the following constants in CpuInfoArch have no
>>> corresponding (identical) mapping -- I'll state to the right of the
>>> arrow the emulation targets which I know or presume to be associated
>>> with the CpuInfoArch constant:
>>> - x86 -> i386, x86_64
>>> - sparc -> sparc, sparc64
>>> - ppc -> ppc, ppc64, ppcemb
>>> - mips -> mips, mips64, mips64el, mipsel
>>> - s390 -> s390x
>>> - riscv -> riscv32, riscv64
>>>
>>> The only constant that seems to have a 1:1 mapping is "tricore", but I
>>> could perfectly well be thinking even that just because I have no clue
>>> about "tricore".
>>>
>>> So, I don't think CpuInfoArch can be updated so that it both remains
>>> compatible with current QMP clients, and serves "firmware.json". In the
>>> firmware schema we don't just need CPU architecture, but actual emulator
>>> names (and board / machine types).
>>
>> The completely orthodox fix for CpuInfo would be:
>>
>> * Keep @arch unchanged. In particular, keep "other" for all targets
>> other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'.
>>
>> * Add a new member @target of new enum type Target, whose values match
>> configures idea of targets exactly.
>>
>> * Make the new member the union tag.
>>
>> It's too late for complete orthodoxy; we already changed @arch.
>>
>> A common enum type Target makes sense anyway, I think.
>>
>> Using it in CpuInfo as described above may make sense, too. Could be
>> left to a follow-up patch.
>>
>>> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
>>> that remotely matches is the @TargetInfo structure, in which the @arch
>>> field is a string, coming with the example "x86_64". The example also
>>> names "i386" separately.
>>
>> Well spotted.
>>
>> TargetInfo member @arch is set to TARGET_NAME, which matches configure's
>> idea of the target. If we add enum Target, we should change @arch's
>> type from str to Target.
>>
>>> So what might make sense is to introduce a separate enum in
>>> "qapi/misc.json" with all the softmmu targets I listed above, and change
>>> the type of @TargetInfo.@arch to that enum.
>>
>> I arrived at this conclusion, too.
>>
>>> In parallel,
>>> qmp_query_target() would have to be updated to look up the enum value
>>> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
>>> for collecting the relevant arches here: @TargetInfo is only used in the
>>> "query-target" QMP command, and Markus said above that QMP is only used
>>> with system emulation.)
>>>
>>> Should I do this?
>>
>> Yes, please.
>
> OK, so here's my understanding:
>
> (1) I'll introduce a new @Target enum in misc.json. I'll inherit /
> include it in firmware.json. This is compatible with what Dan said.
>
> (2) I'll change @TargetInfo.@arch to the new type. I believe this will
> break the compilation of qmp_query_target(); I'll hack on that until it
> builds again, with the lookup. :)
>
> (3) Regarding the addition of @target to CpuInfo (accompanying @arch)
> doesn't look hard; what *does* look hard is changing the union
> discriminator from @arch to @target. @target has many more values, and I
> would have to map all of them to the (fewer) @arch values that currently
> do *not* select @CpuInfoOther. Here's an example:
>
> - Both @i386 and @x86_64 from @target mean @x86 in @arch,
> - because @x86 currently selects @CpuInfoX86, not @CpuInfoOther,
> both @i386 and @x86_64 must be assigned @CpuInfoX86.
>
> This depends on the knowledge that "x86" actually *means* "i386 plus
> x86_64", and I totally don't have that knowledge for the rest of the
> arches / targets.
Start with qmp_query_cpus()'s code to initialize @arch:
#if defined(TARGET_I386)
info->value->arch = CPU_INFO_ARCH_X86;
info->value->u.x86.pc = env->eip + env->segs[R_CS].base;
#elif defined(TARGET_PPC)
info->value->arch = CPU_INFO_ARCH_PPC;
info->value->u.ppc.nip = env->nip;
#elif defined(TARGET_SPARC)
info->value->arch = CPU_INFO_ARCH_SPARC;
info->value->u.q_sparc.pc = env->pc;
info->value->u.q_sparc.npc = env->npc;
#elif defined(TARGET_MIPS)
info->value->arch = CPU_INFO_ARCH_MIPS;
info->value->u.q_mips.PC = env->active_tc.PC;
#elif defined(TARGET_TRICORE)
info->value->arch = CPU_INFO_ARCH_TRICORE;
info->value->u.tricore.PC = env->PC;
#elif defined(TARGET_S390X)
info->value->arch = CPU_INFO_ARCH_S390;
info->value->u.s390.cpu_state = env->cpu_state;
#elif defined(TARGET_RISCV)
info->value->arch = CPU_INFO_ARCH_RISCV;
info->value->u.riscv.pc = env->pc;
#else
info->value->arch = CPU_INFO_ARCH_OTHER;
#endif
The TARGET_FOO come from config-target.h, generated by
scripts/create_config from config-target.mak. It derives FOO is from
config-target.mak's TARGET_BASE_ARCH. config-target.mak is in turn
generated by configure. It computes TARGET_BASE_ARCH from target_name.
And that's your mapping.
> So, the modification of @CpuInfo I would indeed like to pass off to
> someone else. (Well, if all the architecture maintainers follow up and
> tell me what emulators exactly fall under the umbrella of each
> individual @arch value, I can post the patch.) BTW... I wonder how
> compatibility would be affected if we just added @target to @CpuInfo,
> even without making it the new discriminator.
Doesn't work for "completely orthodox", because then @arch "other"
comprises totally different targets. But we're not doing that.
We might have to switch the union tag eventually, but if there's no hard
reason to switch it now, feel free to leave it for later.
A comment explaining this mess would be nice, though.
> ... Anyway, I think I've gotten a huge amount of useful and actionable
> feedback; thanks everyone, let me work on RFCv3 and post it soon. :)
I'm not demanding you do this work. If you can do it, wonderful! But
if all you can do is introduce the new enum Target and leave the CpuInfo
mess alone, that's okay. Just add a suitable TODO then.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/20/18 18:37, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: > >> On 04/20/18 14:53, Markus Armbruster wrote: >>> Laszlo Ersek <lersek@redhat.com> writes: >> >> [snip] >> >>>> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386, >>>> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel, >>>> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4, >>>> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb. >>>> >>>> That is, at least the following constants in CpuInfoArch have no >>>> corresponding (identical) mapping -- I'll state to the right of the >>>> arrow the emulation targets which I know or presume to be associated >>>> with the CpuInfoArch constant: >>>> - x86 -> i386, x86_64 >>>> - sparc -> sparc, sparc64 >>>> - ppc -> ppc, ppc64, ppcemb >>>> - mips -> mips, mips64, mips64el, mipsel >>>> - s390 -> s390x >>>> - riscv -> riscv32, riscv64 >>>> >>>> The only constant that seems to have a 1:1 mapping is "tricore", but I >>>> could perfectly well be thinking even that just because I have no clue >>>> about "tricore". >>>> >>>> So, I don't think CpuInfoArch can be updated so that it both remains >>>> compatible with current QMP clients, and serves "firmware.json". In the >>>> firmware schema we don't just need CPU architecture, but actual emulator >>>> names (and board / machine types). >>> >>> The completely orthodox fix for CpuInfo would be: >>> >>> * Keep @arch unchanged. In particular, keep "other" for all targets >>> other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'. >>> >>> * Add a new member @target of new enum type Target, whose values match >>> configures idea of targets exactly. >>> >>> * Make the new member the union tag. >>> >>> It's too late for complete orthodoxy; we already changed @arch. >>> >>> A common enum type Target makes sense anyway, I think. >>> >>> Using it in CpuInfo as described above may make sense, too. Could be >>> left to a follow-up patch. >>> >>>> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing >>>> that remotely matches is the @TargetInfo structure, in which the @arch >>>> field is a string, coming with the example "x86_64". The example also >>>> names "i386" separately. >>> >>> Well spotted. >>> >>> TargetInfo member @arch is set to TARGET_NAME, which matches configure's >>> idea of the target. If we add enum Target, we should change @arch's >>> type from str to Target. >>> >>>> So what might make sense is to introduce a separate enum in >>>> "qapi/misc.json" with all the softmmu targets I listed above, and change >>>> the type of @TargetInfo.@arch to that enum. >>> >>> I arrived at this conclusion, too. >>> >>>> In parallel, >>>> qmp_query_target() would have to be updated to look up the enum value >>>> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators >>>> for collecting the relevant arches here: @TargetInfo is only used in the >>>> "query-target" QMP command, and Markus said above that QMP is only used >>>> with system emulation.) >>>> >>>> Should I do this? >>> >>> Yes, please. >> >> OK, so here's my understanding: >> >> (1) I'll introduce a new @Target enum in misc.json. I'll inherit / >> include it in firmware.json. This is compatible with what Dan said. >> >> (2) I'll change @TargetInfo.@arch to the new type. I believe this will >> break the compilation of qmp_query_target(); I'll hack on that until it >> builds again, with the lookup. :) >> >> (3) Regarding the addition of @target to CpuInfo (accompanying @arch) >> doesn't look hard; what *does* look hard is changing the union >> discriminator from @arch to @target. @target has many more values, and I >> would have to map all of them to the (fewer) @arch values that currently >> do *not* select @CpuInfoOther. Here's an example: >> >> - Both @i386 and @x86_64 from @target mean @x86 in @arch, >> - because @x86 currently selects @CpuInfoX86, not @CpuInfoOther, >> both @i386 and @x86_64 must be assigned @CpuInfoX86. >> >> This depends on the knowledge that "x86" actually *means* "i386 plus >> x86_64", and I totally don't have that knowledge for the rest of the >> arches / targets. > > Start with qmp_query_cpus()'s code to initialize @arch: > > #if defined(TARGET_I386) > info->value->arch = CPU_INFO_ARCH_X86; > info->value->u.x86.pc = env->eip + env->segs[R_CS].base; > #elif defined(TARGET_PPC) > info->value->arch = CPU_INFO_ARCH_PPC; > info->value->u.ppc.nip = env->nip; > #elif defined(TARGET_SPARC) > info->value->arch = CPU_INFO_ARCH_SPARC; > info->value->u.q_sparc.pc = env->pc; > info->value->u.q_sparc.npc = env->npc; > #elif defined(TARGET_MIPS) > info->value->arch = CPU_INFO_ARCH_MIPS; > info->value->u.q_mips.PC = env->active_tc.PC; > #elif defined(TARGET_TRICORE) > info->value->arch = CPU_INFO_ARCH_TRICORE; > info->value->u.tricore.PC = env->PC; > #elif defined(TARGET_S390X) > info->value->arch = CPU_INFO_ARCH_S390; > info->value->u.s390.cpu_state = env->cpu_state; > #elif defined(TARGET_RISCV) > info->value->arch = CPU_INFO_ARCH_RISCV; > info->value->u.riscv.pc = env->pc; > #else > info->value->arch = CPU_INFO_ARCH_OTHER; > #endif > > The TARGET_FOO come from config-target.h, generated by > scripts/create_config from config-target.mak. It derives FOO is from > config-target.mak's TARGET_BASE_ARCH. config-target.mak is in turn > generated by configure. It computes TARGET_BASE_ARCH from target_name. > And that's your mapping. > >> So, the modification of @CpuInfo I would indeed like to pass off to >> someone else. (Well, if all the architecture maintainers follow up and >> tell me what emulators exactly fall under the umbrella of each >> individual @arch value, I can post the patch.) BTW... I wonder how >> compatibility would be affected if we just added @target to @CpuInfo, >> even without making it the new discriminator. > > Doesn't work for "completely orthodox", because then @arch "other" > comprises totally different targets. But we're not doing that. > > We might have to switch the union tag eventually, but if there's no hard > reason to switch it now, feel free to leave it for later. > > A comment explaining this mess would be nice, though. > >> ... Anyway, I think I've gotten a huge amount of useful and actionable >> feedback; thanks everyone, let me work on RFCv3 and post it soon. :) > > I'm not demanding you do this work. If you can do it, wonderful! But > if all you can do is introduce the new enum Target and leave the CpuInfo > mess alone, that's okay. Just add a suitable TODO then. > I went offline to work on RFCv3 before seeing this email, and now I'm seeing it only after I've posted RFCv3. I'll have to digest it some more. I'll try to do something about the comments at least. Thanks! Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 18/04/2018 00:40, Laszlo Ersek wrote:
> +#
> +# Lists firmware types commonly used with QEMU virtual machines.
> +#
> +# @bios: The firmware was built from the SeaBIOS project.
> +#
> +# @slof: The firmware was built from the Slimline Open Firmware project.
> +#
> +# @uboot: The firmware was built from the U-Boot project.
> +#
> +# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareType',
> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
A very basic question (so not likely a suggestion for change). Why
wouldn't these be features too? For example a UEFI with CSM could
expose both uefi and bios, a u-boot with UEFI support could expose both
uboot and uefi, etc.
Perhaps there are two dimensions, the FirmwareType tells you how to
configure it and the FirmwareFeature tells you the APIs it exposes to
the guest?
Paolo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 11:43, Paolo Bonzini wrote:
> On 18/04/2018 00:40, Laszlo Ersek wrote:
>> +#
>> +# Lists firmware types commonly used with QEMU virtual machines.
>> +#
>> +# @bios: The firmware was built from the SeaBIOS project.
>> +#
>> +# @slof: The firmware was built from the Slimline Open Firmware project.
>> +#
>> +# @uboot: The firmware was built from the U-Boot project.
>> +#
>> +# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum' : 'FirmwareType',
>> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>
> A very basic question (so not likely a suggestion for change). Why
> wouldn't these be features too? For example a UEFI with CSM could
> expose both uefi and bios, a u-boot with UEFI support could expose both
> uboot and uefi, etc.
Good point. I considered "type" to be a given, from the initial
brainstorming, but if Dan is OK with your suggestion, I can turn these
into features as well.
> Perhaps there are two dimensions, the FirmwareType tells you how to
> configure it and the FirmwareFeature tells you the APIs it exposes to
> the guest?
I don't know enough firmware types to answer this :) I believe I agree
about the FirmwareFeature statement (if we also include "platform
requirements" there), but I have no clue about any generalizations for
firmware configuration.
Thanks,
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 18, 2018 at 01:52:19PM +0200, Laszlo Ersek wrote:
> On 04/18/18 11:43, Paolo Bonzini wrote:
> > On 18/04/2018 00:40, Laszlo Ersek wrote:
> >> +#
> >> +# Lists firmware types commonly used with QEMU virtual machines.
> >> +#
> >> +# @bios: The firmware was built from the SeaBIOS project.
> >> +#
> >> +# @slof: The firmware was built from the Slimline Open Firmware project.
> >> +#
> >> +# @uboot: The firmware was built from the U-Boot project.
> >> +#
> >> +# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
> >> +#
> >> +# Since: 2.13
> >> +##
> >> +{ 'enum' : 'FirmwareType',
> >> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> >
> > A very basic question (so not likely a suggestion for change). Why
> > wouldn't these be features too? For example a UEFI with CSM could
> > expose both uefi and bios, a u-boot with UEFI support could expose both
> > uboot and uefi, etc.
>
> Good point. I considered "type" to be a given, from the initial
> brainstorming, but if Dan is OK with your suggestion, I can turn these
> into features as well.
>
> > Perhaps there are two dimensions, the FirmwareType tells you how to
> > configure it and the FirmwareFeature tells you the APIs it exposes to
> > the guest?
>
> I don't know enough firmware types to answer this :) I believe I agree
> about the FirmwareFeature statement (if we also include "platform
> requirements" there), but I have no clue about any generalizations for
> firmware configuration.
IIUC Paolo is basically suggesting
{
"description": "OVMF firmware"
"type": "uefi",
"features": [
"uefi",
"bios"
],
}
where 'bios' is only listed if CSM is enabled in the OVMF build. I tend
to think that is redundant and we could just do
{
"description": "OVMF firmware"
"features": [
"uefi",
"bios"
],
}
And declare the order of "features" list is significant. ie
"features": [
"uefi",
"bios"
],
means a UEFI firmware which has back compat for BIOS (ie OVMF with CSM)
while
"features": [
"bios"
"uefi",
],
means a traditional BIOS firmware with compat for UEFI (thinking uboot
being able to include uefi support in this case)
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
Hi, > "features": [ > "bios" > "uefi", > ], > > means a traditional BIOS firmware with compat for UEFI (thinking uboot > being able to include uefi support in this case) Well, uboot is it's own world, I would write that as [ "uboot", "uefi" ]. I like the idea to list all supported interfaces in order of preference. cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/18 14:03, Daniel P. Berrangé wrote:
> On Wed, Apr 18, 2018 at 01:52:19PM +0200, Laszlo Ersek wrote:
>> On 04/18/18 11:43, Paolo Bonzini wrote:
>>> On 18/04/2018 00:40, Laszlo Ersek wrote:
>>>> +#
>>>> +# Lists firmware types commonly used with QEMU virtual machines.
>>>> +#
>>>> +# @bios: The firmware was built from the SeaBIOS project.
>>>> +#
>>>> +# @slof: The firmware was built from the Slimline Open Firmware project.
>>>> +#
>>>> +# @uboot: The firmware was built from the U-Boot project.
>>>> +#
>>>> +# @uefi: The firmware was built from the edk2 (EFI Development Kit II) project.
>>>> +#
>>>> +# Since: 2.13
>>>> +##
>>>> +{ 'enum' : 'FirmwareType',
>>>> + 'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
>>>
>>> A very basic question (so not likely a suggestion for change). Why
>>> wouldn't these be features too? For example a UEFI with CSM could
>>> expose both uefi and bios, a u-boot with UEFI support could expose both
>>> uboot and uefi, etc.
>>
>> Good point. I considered "type" to be a given, from the initial
>> brainstorming, but if Dan is OK with your suggestion, I can turn these
>> into features as well.
>>
>>> Perhaps there are two dimensions, the FirmwareType tells you how to
>>> configure it and the FirmwareFeature tells you the APIs it exposes to
>>> the guest?
>>
>> I don't know enough firmware types to answer this :) I believe I agree
>> about the FirmwareFeature statement (if we also include "platform
>> requirements" there), but I have no clue about any generalizations for
>> firmware configuration.
>
> IIUC Paolo is basically suggesting
>
> {
> "description": "OVMF firmware"
> "type": "uefi",
> "features": [
> "uefi",
> "bios"
> ],
> }
>
> where 'bios' is only listed if CSM is enabled in the OVMF build. I tend
> to think that is redundant and we could just do
>
>
> {
> "description": "OVMF firmware"
> "features": [
> "uefi",
> "bios"
> ],
> }
Actually, this is how I interpreted Paolo's idea at once. I agree the
"type" member can be dropped.
>
> And declare the order of "features" list is significant. ie
>
> "features": [
> "uefi",
> "bios"
> ],
>
>
> means a UEFI firmware which has back compat for BIOS (ie OVMF with CSM)
> while
>
> "features": [
> "bios"
> "uefi",
> ],
>
> means a traditional BIOS firmware with compat for UEFI (thinking uboot
> being able to include uefi support in this case)
Is it guaranteed that lists in JSON keep the order of the elements?
Because, dictionaries definitely don't promise any ordering between the
keys.
Thanks,
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/18/2018 07:40 AM, Laszlo Ersek wrote: > Is it guaranteed that lists in JSON keep the order of the elements? > Because, dictionaries definitely don't promise any ordering between the > keys. Yes, JSON lists preserve order (and we have to explicitly document cases where order within a list is not significant). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 04/18/18 17:17, Eric Blake wrote: > On 04/18/2018 07:40 AM, Laszlo Ersek wrote: > >> Is it guaranteed that lists in JSON keep the order of the elements? >> Because, dictionaries definitely don't promise any ordering between the >> keys. > > Yes, JSON lists preserve order (and we have to explicitly document cases > where order within a list is not significant). > Thanks. After digesting the comments on @type for a while longer, from Dan, Gerd and Paolo, I think we *should* keep @type, but rather than having it be a simple enum, let's make it a list of enums, where order matters. Keep @features separate, and document that order does not matter there. Here's why: my brain is crashing trying to come up with a human-parseable explanation why for *some* entries in @features, their relative order is important, and why for some others, it isn't. Discerning the subset for which order matters, from any specific grab-bag of @features, will be no fun for the human user. Given that the "ordered features" idea was brought to life solely because a firmware can provide multiple interfaces (with a strong preference order between them), I guess we might as well be honest about that, and update @type accordingly. Thoughts? :) Thanks, Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Apr 18, 2018 at 05:27:32PM +0200, Laszlo Ersek wrote: > On 04/18/18 17:17, Eric Blake wrote: > > On 04/18/2018 07:40 AM, Laszlo Ersek wrote: > > > >> Is it guaranteed that lists in JSON keep the order of the elements? > >> Because, dictionaries definitely don't promise any ordering between the > >> keys. > > > > Yes, JSON lists preserve order (and we have to explicitly document cases > > where order within a list is not significant). > > > > Thanks. > > After digesting the comments on @type for a while longer, from Dan, Gerd > and Paolo, I think we *should* keep @type, but rather than having it be > a simple enum, let's make it a list of enums, where order matters. Keep > @features separate, and document that order does not matter there. > > Here's why: my brain is crashing trying to come up with a > human-parseable explanation why for *some* entries in @features, their > relative order is important, and why for some others, it isn't. > Discerning the subset for which order matters, from any specific > grab-bag of @features, will be no fun for the human user. > > Given that the "ordered features" idea was brought to life solely > because a firmware can provide multiple interfaces (with a strong > preference order between them), I guess we might as well be honest about > that, and update @type accordingly. > > Thoughts? :) Keeping type separate from features is fine by me. 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
On 04/18/18 17:27, Laszlo Ersek wrote: > On 04/18/18 17:17, Eric Blake wrote: >> On 04/18/2018 07:40 AM, Laszlo Ersek wrote: >> >>> Is it guaranteed that lists in JSON keep the order of the elements? >>> Because, dictionaries definitely don't promise any ordering between the >>> keys. >> >> Yes, JSON lists preserve order (and we have to explicitly document cases >> where order within a list is not significant). >> > > Thanks. > > After digesting the comments on @type for a while longer, from Dan, Gerd > and Paolo, I think we *should* keep @type, but rather than having it be > a simple enum, let's make it a list of enums, where order matters. Keep > @features separate, and document that order does not matter there. > > Here's why: my brain is crashing trying to come up with a > human-parseable explanation why for *some* entries in @features, their > relative order is important, and why for some others, it isn't. > Discerning the subset for which order matters, from any specific > grab-bag of @features, will be no fun for the human user. > > Given that the "ordered features" idea was brought to life solely > because a firmware can provide multiple interfaces (with a strong > preference order between them), I guess we might as well be honest about > that, and update @type accordingly. > > Thoughts? :) I mean I realize I just reinvented Paolo's point from <http://mid.mail-archive.com/c5e3f942-2c0e-9809-32ae-df97eaf2b7cd@redhat.com> :) I don't intend to claim the idea, I just needed some time to understand it. Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 18.04.2018 00:40, Laszlo Ersek wrote:
> Add a schema that describes the different uses and properties of virtual
> machine firmware.
>
> Each firmware executable installed on a host system should come with at
> least one JSON file that conforms to this schema. Each file informs the
> management applications about the firmware's properties and one possible
> use case / feature set.
>
> 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 description 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 for a
> certain use case or feature set.
[...]
> +##
> +# @FirmwareMappingFlash:
> +#
> +# Describes loading and mapping properties for the firmware executable and its
> +# accompanying NVRAM file, when @FirmwareDevice is @flash.
> +#
> +# @executable: Identifies the firmware executable. The firmware executable may
> +# be shared by multiple virtual machine definitions. The
> +# corresponding QEMU command line option is "-drive
> +# if=pflash,unit=0,readonly=on,file=@executable.@pathname,format=@executable.@format".
> +#
> +# @nvram_template: Identifies the NVRAM template compatible with @executable.
> +# Management software instantiates an individual copy -- a
> +# specific NVRAM file -- from @nvram_template.@pathname for
> +# each new virtual machine definition created.
> +# @nvram_template.@pathname itself is never mapped into
> +# virtual machines, only individual copies of it are. An NVRAM
> +# file is typically used for persistently storing the
> +# non-volatile UEFI variables of a virtual machine definition.
> +# The corresponding QEMU command line option is "-drive
> +# if=pflash,unit=1,readonly=off,file=PATHNAME_OF_PRIVATE_NVRAM_FILE,format=@nvram_template.@format".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingFlash',
> + 'data' : { 'executable' : 'FirmwareFlashFile',
> + 'nvram_template' : 'FirmwareFlashFile' } }
I think it's more common to use "-" than "_" in these json files, so
maybe better use "nvram-template" instead of "nvram_template" ?
Thomas
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.