On Thu, Mar 18, 2021 at 01:26:45PM +0100, Pavel Hrdina wrote:
> When the firmware auto-selection was introduced it always picked first
> usable firmware based on the JSON descriptions on the host. It is
> possible to add/remove/change the JSON files but it will always be for
> the whole host.
>
> This patch introduces support for configuring the auto-selection per VM
> by adding users an option to limit what features they would like to have
> available in the firmware.
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
[...]
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index c101d5a1f1..dd063b0794 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -155,6 +155,37 @@ harddisk, cdrom, network) determining where to obtain/find the boot image.
> the host native arch will be chosen. For the ``test``, ``ESX`` and ``VMWare``
> hypervisor drivers, however, the ``i686`` arch will always be chosen even on
> an ``x86_64`` host. :since:`Since 0.0.1`
> +``firmware``
> + :since:`Since 7.2.0 QEMU/KVM only`
> +
> + When used together with ``firmware`` attribute of ``os`` element the ``type``
> + attribute must have the same value.
> +
> + List of mandatory attributes:
> +
> + - ``type`` (accepted values are ``bios`` and ``efi``) same as the ``firmware``
> + attribute of ``os`` element.
> +
> + When using firmware auto-selection there are different features enabled in
> + the firmwares.
Nit: I'd recast it as: "When using firmware auto-selection, different
features are enabled in any given firmware binary."
> + The list of features can be used to limit what firmware should
> + be automatically selected for the VM. The list of features can be specified
> + using zero or more ``feature`` elements. Libvirt will take into consideration
> + only the listed features and ignore the rest when selecting the firmware.
> +
> + ``feature``
Should we also list a couple of example features? E.g. "amd-sev" (on
supported hardware), "acpi-s3", "secure-boot".
I notice that "enrolled-keys" is the only feature advertized by one of
the firmware descriptor files (40-edk2-ovmf-x64-sb-enrolled.json), which
is used as a mandatory XML attribute for the ``feature`` element.
I'm silently wondering there's a possibility for confusion: "Hmm,
'enrolled-keys' is a possible feature, but it is a mandatory attribute
of ``feature`` element."
> + The list of mandatory attributes:
> +
> + - ``enabled`` (accepted values are ``yes`` and ``no``) is used to tell libvirt
> + if the feature must be enabled or not in the automatically selected firmware
> +
> + - ``name`` the name of the feature, the list of the features:
> +
> + - ``enrolled-keys`` whether the selected nvram template has default
Nit: s/nvram/NVRAM/ (Or ``nvram``, to use rST verbatim; as it's an XML
element.)
> + certificate enrolled. Firmware with Secure Boot feature but without
> + enrolled keys will successfully boot non-signed binaries as well.
> + Valid only for firmwares with Secure Boot feature.
Nice; it spells out the granular possibilities.
> + - ``secure-boot`` whether the firmware implements UEFI Secure boot feature.
> ``loader``
Here too (to match with the other formatdomain.rst): "whether the
firmware is _capable_ of UEFI Secure Boot feature."
I'm not qualified to comment on the rest of the patch, as I don't dwell
on this code; but, FWIW:
Acked-by: Kashyap Chamarthy <kchamart@xxxxxxxxxx>
[...]
--
/kashyap
