WIP: Add efi and secureboot support #12

jovial · 2018-10-05T15:35:04Z

secure boot support is currently limited to redhat
derived distros as the ubuntu packages don't provide
UefiShell.iso.
Although the upgrading of qemu packages is more suited
to the ansible-role-libvirt-host role, we need to know
the location of the UEFI firmware images to setup the
virtual machines. As we somtimes require custom packages
this makes it hard to know upfront what the path will
be.

jovial · 2018-10-05T15:38:37Z

May need to move some of this code in ansible-role-libvirt-host. How can we workaround the problem that the EFI firmware path may change if we install a different package. One solution would be to copy these files to a standard location.

- secure boot support is currently limited to redhat derived distros as the ubuntu packages don't provide UefiShell.iso. - Although the upgrading of qemu packages is more suited to the ansible-role-libvirt-host role, we need to know the location of the UEFI firmware images to setup the virtual machines. As we somtimes require custom packages this makes it hard to know upfront what the path will be.

Before this change you would get: K [stackhpc.libvirt-vm : Ensure the VM is undefined] *********************** * An exception occurred during task execution. To see the full traceback, use -vv v. The error was: libvirtError: Requested operation is not valid: cannot delete inactive domain with nvram fatal: [localhost]: FAILED! => {"changed": false, "msg": "Requested operation i s not valid: cannot delete inactive domain with nvram"}

markgoddard · 2018-10-09T12:59:21Z

tasks/destroy-vm.yml

+      # virsh cli directly. It may be better to detect if dumpxml
+      # actually contains an nvram element rather than relying on
+      # boot_firmware having the correct value.
+      command: virsh -c qemu:///system undefine{% if boot_firmware == 'efi' %} --nvram{% endif %} {{ vm.name }}


This is a bit clunky :( Looks like it's necessary though

markgoddard · 2018-10-09T13:01:16Z

tasks/setup.yml

+        - "{{ ansible_distribution }}.yml"
+        - "{{ ansible_os_family }}.yml"
+      skip: true
+  tags: vars


Probably nicer to do this in main.yml.

I wasn't sure if we should make it so the playbooks can be run separately.

Fair point.

markgoddard · 2018-10-09T13:01:46Z

tasks/setup.yml

+  # The core team had a a change of heart and it is actually being preserved:
+  # https://github.com/ansible/ansible/pull/43798
+  yum_repository: "{{ item }}"
+  loop: "{{ libvirt_vm_custom_yum_repos | default({}) }}"


nit: default should be a list

markgoddard · 2018-10-09T13:02:48Z

tasks/setup.yml

@@ -0,0 +1,31 @@
+---


This stuff probably belongs in libvirt-host.

markgoddard · 2018-10-09T13:04:43Z

tasks/prepare_secure_boot.yml

@@ -0,0 +1,59 @@
+---


micronit: other task file names use dashes

This file could do with a comment to explain what's going on.

markgoddard · 2018-10-09T13:10:30Z

tasks/prepare_secure_boot.yml

+    path: "{{ ovmf_enrolled_variables_path }}"
+  register: generated_ovmf
+
+- name: Run OVMF vars generator


Might be good if this role was just provided with a path to this script, which could be installed/cloned by libvirt-host.

markgoddard · 2018-10-09T13:12:36Z

vars/Debian.yml

+  - ovmf
+
+# List of extra packages to install
+libvirt_vm_extra_packages: "{{ [] + (libvirt_vm_extra_packages_efi if libvirt_vm_enable_efi_support else []) | unique }}"


Do you need the [] + here?

markgoddard · 2018-10-09T13:13:29Z

vars/RedHat.yml

+
+# Packages that are only necessary if you require EFI support
+libvirt_vm_extra_packages_efi:
+  - edk2.git-ovmf-x64 # Official OVMF package doesn't boot (CentOS 7.5)


Interesting OVMF was just proposed for addition to the nova containers in kolla: https://review.openstack.org/608579

markgoddard · 2018-10-09T13:14:59Z

tasks/vm.yml

+  include_tasks: prepare_secure_boot.yml
+  when:
+    - boot_firmware == "efi"
+    - libvirt_vm_ovmf_uefi_shell_iso_path is defined


Not supported on Debian?

The Debian packages don't contain the UEFI shell ISO which contains the EnrollDefaultKeys.efi binary. WE would either need to build this from source or obtain it by some other means.

markgoddard · 2018-10-09T13:15:52Z

vars/RedHat.yml

+# Packages that are only necessary if you require EFI support
+libvirt_vm_extra_packages_efi:
+  - edk2.git-ovmf-x64 # Official OVMF package doesn't boot (CentOS 7.5)
+  - qemu-kvm-ev # Need smm support for secure boot


Does this require hardware virtualisation?

I believe SMM emulation, which is needed for secure boot, requires either KVM or TCG acceleration to be enabled. So, by enabling TCG we should be able to use secure-boot without KVM - but I will need to test this to be sure.

From the OVMF readme, https://github.com/tianocore/edk2/tree/073891a3e74059e996258e32b56b3f0770c6fe55/OvmfPkg: * The minimum required QEMU machine type is "pc-q35-2.5".

* It seems you cannot specify multiple <loader> elements, instead you must use an nvram element * Ensure the qemu user can read the generated file. This is done by generating the file in a temporary location before transferring ownership to the libvirt_vm_log_owner owner.

This has more general applicability than just setting the owner of the log files.

This matches the existing files.

See: stackhpc/ansible-role-libvirt-host#7

This requires upstream support in stackhpc.libvirt-host and stackhpc.libvirt-vm, see: - stackhpc/ansible-role-libvirt-vm#12 - stackhpc/ansible-role-libvirt-host#7

…ture The qemu-kvm binary only supports the host architecture. We must therefore use the qemu-system-<architecture> binaries when the target architecture is not the same as the host architecture. Although it may be possible that the host CPU may support acceleration for a foreign architecture, I do not currently know of any chips that support this functionality. For this reason, we default to software emulation.

It seems that the virt module does not take into account changes to the templated vm.xml.j2 file. As such, changes to variables used within this template are not propagated to libvirt i.e the output of virsh dumpxml remains the same before and after the application of define.

This is a requirement for smm and secureboot, see: https://github.com/tianocore/edk2/tree/master/OvmfPkg and can be used with and without kvm.

SMM is requirement for secure boot, see: https://github.com/tianocore/edk2/blob/a8b5750901faa63ff5570634851e648b8e335e5a/OvmfPkg/README

This is so we can use a newer version of qemu if secure boot is enabled. The version shipped in CentOS 7.5 does not support SMM which is a requirement of secure boot.

SMM is a requirement to use the needs-SMM variant and qemu will hang on boot if SMM is disabled.

markgoddard · 2018-11-01T16:48:10Z

tasks/autodetect.yml

-          when: item.stat.exists
+    - name: Ensure old value stored in detected_libvirt_vm_emulator is cleared
+      set_fact:
+        detected_libvirt_vm_emulator: none


Does this not set it to the string 'none'? You can set a variable to none via:

detected_libvirt_vm_emulator:

markgoddard · 2018-11-02T09:16:41Z

tasks/vm.yml

+  # not creating any newv VMs.
+  when: >-
+    (libvirt_vms | selectattr('state', 'defined')
+       | selectattr('state', 'equalto', 'absent') | list) != libvirt_vms


Don't we want to do this unconditionally?

markgoddard · 2018-11-02T09:24:58Z

vars/RedHat.yml

-libvirt_vm_log_owner: qemu
+# This controls ownership of files used by the libvirt qemu process,
+# e.g the serial console logs in console_log_path.
+libvirt_vm_qemu_user: qemu


Better check this isn't referenced anywhere by consumers of this role.

jovial requested a review from markgoddard October 5, 2018 15:36

jovial force-pushed the feature/efi branch from a75cfb1 to e4e732b Compare October 5, 2018 15:41

markgoddard reviewed Oct 9, 2018

View reviewed changes

jovial added 9 commits October 22, 2018 11:18

SMM requires q35 machine type

a90cf1b

From the OVMF readme, https://github.com/tianocore/edk2/tree/073891a3e74059e996258e32b56b3f0770c6fe55/OvmfPkg: * The minimum required QEMU machine type is "pc-q35-2.5".

Rename libvirt_vm_log_owner to libvirt_vm_qemu_user

3a2f2a8

This has more general applicability than just setting the owner of the log files.

Use dashes instead of underscores for word separator

808573e

This matches the existing files.

Default value should be an empty list

0e9b51f

Add flag to explicitly enable SMM

bade36f

Add comment explaining usage of OVMF generator

1957070

Appending to an empty list is unnecessary

e96b7aa

Move package installation to stackhpc.libvirt-host role

d960ec0

See: stackhpc/ansible-role-libvirt-host#7

jovial added a commit to jovial/tenks that referenced this pull request Oct 22, 2018

Add support for EFI boot mode

0d40357

This requires upstream support in stackhpc.libvirt-host and stackhpc.libvirt-vm, see: - stackhpc/ansible-role-libvirt-vm#12 - stackhpc/ansible-role-libvirt-host#7

jovial mentioned this pull request Oct 22, 2018

Add support for EFI boot mode stackhpc/tenks#29

Open

jovial added 8 commits October 23, 2018 09:12

Remove trailing whitespace

fdbdbe2

Add option to disable secure boot

8da41b5

Use q35 machine type for EFI based booting

547a03d

This is a requirement for smm and secureboot, see: https://github.com/tianocore/edk2/tree/master/OvmfPkg and can be used with and without kvm.

SMM defaults to enabled when secure boot is enabled

5bb890d

SMM is requirement for secure boot, see: https://github.com/tianocore/edk2/blob/a8b5750901faa63ff5570634851e648b8e335e5a/OvmfPkg/README

Auto detection now relies on virtualisation features

923bf75

This is so we can use a newer version of qemu if secure boot is enabled. The version shipped in CentOS 7.5 does not support SMM which is a requirement of secure boot.

Use pure efi version of OVMF if SMM is disabled.

93a9ed7

SMM is a requirement to use the needs-SMM variant and qemu will hang on boot if SMM is disabled.

markgoddard reviewed Nov 2, 2018

View reviewed changes

markgoddard mentioned this pull request Feb 2, 2022

Add efi support #83

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add efi and secureboot support #12

WIP: Add efi and secureboot support #12

jovial commented Oct 5, 2018

jovial commented Oct 5, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

jovial Oct 22, 2018

markgoddard Oct 31, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

markgoddard Oct 9, 2018

jovial Oct 22, 2018

markgoddard Oct 9, 2018

jovial Oct 22, 2018

markgoddard Nov 1, 2018

markgoddard Nov 2, 2018

markgoddard Nov 2, 2018

WIP: Add efi and secureboot support #12

Are you sure you want to change the base?

WIP: Add efi and secureboot support #12

Conversation

jovial commented Oct 5, 2018

jovial commented Oct 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment