[libvirt] [PATCH] qemu: use 'bochs' video type by default for UEFI domains

[Peter Krempa]

On Wed, Aug 28, 2019 at 16:52:39 -0500, Jonathon Jongsma wrote:
> The 'bochs' video device doesn't have any legacy vga emulation so the
> attack surface is much lower. It works with OVMF, so UEFI guests should
> not see any functional difference to VGA.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1707119
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---

Historically we did not allow change of behaviour when filling in
defaults unless the old configuration stopped to be supported by qemu
and we could detect it. I don't thik this case can be excused.

> NOTE:
> You may run into an error when trying to use the bochs video device. For
> example:
> 
>     error: internal error: process exited while connecting to monitor:
>     2019-08-28T21:32:20.134546Z qemu-system-x86_64: -device
>     bochs-display,id=video0,vgamem=16384k,bus=pcie.0,addr=0x1: failed to find
>     romfile "vgabios-bochs-display.bin"
> 
> This should be solved in e.g. Fedora 31 with newer releases of seabios/qemu. As
> a temporary workaround, you can symlink the appropriate vgabios file under
> /usr/share/qemu/.

Similarly I don't think this is acceptable.

>  src/qemu/qemu_domain.c                        | 19 +++++----
>  src/qemu/qemu_domain.h                        |  1 +
>  .../video-default-nouefi.x86_64-latest.args   | 36 +++++++++++++++++
>  .../qemuxml2argvdata/video-default-nouefi.xml | 20 ++++++++++
>  .../video-default-uefi.x86_64-latest.args     | 40 +++++++++++++++++++
>  tests/qemuxml2argvdata/video-default-uefi.xml | 22 ++++++++++
>  tests/qemuxml2argvtest.c                      |  2 +
>  7 files changed, 133 insertions(+), 7 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/video-default-nouefi.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/video-default-nouefi.xml
>  create mode 100644 tests/qemuxml2argvdata/video-default-uefi.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/video-default-uefi.xml
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 4998474dc9..7ecb89ac84 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4584,6 +4584,14 @@ qemuDomainValidateCpuCount(const virDomainDef *def,
>  }
>  
>  
> +static bool
> +qemuDomainDefIsUEFI(const virDomainDef *def)
> +{
> +    return ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI ||
> +             (def->os.loader && def->os.loader->type ==
> +              VIR_DOMAIN_LOADER_TYPE_PFLASH)));
> +}
> +
>  static int
>  qemuDomainDefValidate(const virDomainDef *def,
>                        virCapsPtr caps ATTRIBUTE_UNUSED,
> @@ -4606,10 +4614,7 @@ qemuDomainDefValidate(const virDomainDef *def,
>      }
>  
>      /* On x86, UEFI requires ACPI */
> -    if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI ||
> -         (def->os.loader &&
> -          def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH)) &&
> -        ARCH_IS_X86(def->os.arch) &&
> +    if (qemuDomainDefIsUEFI(def) && ARCH_IS_X86(def->os.arch) &&
>          def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("UEFI requires ACPI on this architecture"));
> @@ -4619,9 +4624,7 @@ qemuDomainDefValidate(const virDomainDef *def,
>      /* On aarch64, ACPI requires UEFI */
>      if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON &&
>          def->os.arch == VIR_ARCH_AARCH64 &&
> -        (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
> -         (!def->os.loader ||
> -          def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH))) {
> +        !qemuDomainDefIsUEFI(def)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("ACPI requires UEFI on this architecture"));
>          goto cleanup;

The three hunks above are refactors not functionally connected with the
behaviour change later, thus they should be in a separate patch.


> @@ -7452,6 +7455,8 @@ qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video,
>                   qemuDomainIsRISCVVirt(def) ||
>                   ARCH_IS_S390(def->os.arch))
>              video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
> +        else if (qemuDomainDefIsUEFI(def))
> +            video->type = VIR_DOMAIN_VIDEO_TYPE_BOCHS;

'bochs-display' seems to be supported starting qemu 3.0.0, but
pflash/ovmf starting qemu 1.7, so this can result in invalid
configuration for older qemu versions.

>          else
>              video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
>      }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190829/88987745/attachment-0001.sig>