From 263e0ff554c4c68962fb689dec541dabf5cc71c9 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Wed, 17 Aug 2016 14:27:24 +0100 Subject: [PATCH] v2v: Use OVMF secure boot file (RHBZ#1367615). From RHEL 7.3, Red Hat have decided to only compile the secure boot version of OVMF on x86-64, with flags -D SECURE_BOOT_ENABLE -D SMM_REQUIRE. The filename has also changed to reflect this - it is now /usr/share/OVMF/OVMF_CODE.secboot.fd. The old file /usr/share/OVMF/OVMF_CODE.fd is no longer shipped. However switching to using this variant of OVMF for UEFI guests is not just a matter of changing the filename. The new OVMF variant won't run unless we also change: - The qemu machine model, from the default ("pc" == "pc-i440fx-rhel7.3.0" or later) to Q35 ("q35" == "pc-q35-rhel7.3.0" or later). - Add under . - Set . NB: On RHEL the changes requires qemu-kvm-rhev. It is no longer possible to convert UEFI guests using the basic qemu-kvm. Thanks: Laszlo Ersek, Ming Xie. (cherry picked from commit ba5c5a8713c4418c861edecacff761f4d3720508) --- generator/uefi.ml | 24 ++++++++++++++++++------ src/appliance.c | 6 ++++-- src/guestfs-internal-frontend.h | 3 ++- src/guestfs-internal.h | 2 +- src/launch-direct.c | 15 ++++++++++++++- src/launch-libvirt.c | 16 +++++++++++++++- v2v/output_libvirt.ml | 40 +++++++++++++++++++++++++++++----------- v2v/output_qemu.ml | 13 ++++++++++--- v2v/virt-v2v.pod | 2 ++ 9 files changed, 95 insertions(+), 26 deletions(-) diff --git a/generator/uefi.ml b/generator/uefi.ml index 3b371b8..661c1d7 100644 --- a/generator/uefi.ml +++ b/generator/uefi.ml @@ -35,6 +35,16 @@ let firmware = [ "/usr/share/OVMF/OVMF_VARS.fd", []; + (* From RHEL 7.3, only secure boot variants of UEFI are shipped. + * This requires additional qemu options, see RHBZ#1367615 for + * details. + *) + "x86_64", + "/usr/share/OVMF/OVMF_CODE.secboot.fd", + None, + "/usr/share/OVMF/OVMF_VARS.fd", + [ "UEFI_FLAG_SECURE_BOOT_REQUIRED" ]; + "aarch64", "/usr/share/AAVMF/AAVMF_CODE.fd", None, @@ -63,14 +73,13 @@ let generate_uefi_c () = pr "guestfs_int_uefi_%s_firmware[] = {\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { \"%s\",\n" (c_quote code); (match code_debug with | None -> pr " NULL,\n" | Some code_debug -> pr " \"%s\",\n" (c_quote code_debug) ); pr " \"%s\",\n" (c_quote vars); - pr " 0\n"; + pr " %s\n" (if flags <> [] then String.concat "|" flags else "0"); pr " },\n"; ) firmware; pr "};\n"; @@ -84,8 +93,10 @@ type uefi_firmware = { code : string; code_debug : string option; vars : string; - flags : unit list; + flags : uefi_flags; } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; List.iter ( fun arch -> @@ -95,14 +106,13 @@ type uefi_firmware = { pr "let uefi_%s_firmware = [\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { code = %S;\n" code; (match code_debug with | None -> pr " code_debug = None;\n" | Some code_debug -> pr " code_debug = Some %S;\n" code_debug ); pr " vars = %S;\n" vars; - pr " flags = [];\n"; + pr " flags = [%s];\n" (String.concat "; " flags); pr " };\n"; ) firmware; pr "]\n"; @@ -118,8 +128,10 @@ type uefi_firmware = { code : string; (** code file *) code_debug : string option; (** code debug file *) vars : string; (** vars template file *) - flags : unit list; (** flags *) + flags : uefi_flags; (** flags *) } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; diff --git a/src/appliance.c b/src/appliance.c index 6409cfb..a3af164 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -427,10 +427,10 @@ dir_contains_files (const char *dir, ...) * cause appliance building to fail (no UEFI firmware is not an * error). * - * XXX See also v2v/utils.ml:find_uefi_firmware + * See also v2v/utils.ml:find_uefi_firmware */ int -guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) +guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags) { #ifdef __aarch64__ size_t i; @@ -468,6 +468,7 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Caller frees. */ *code = safe_strdup (g, codefile); *vars = varst; + *flags = guestfs_int_aavmf_firmware[i].flags; return 0; } } @@ -475,5 +476,6 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Not found. */ *code = *vars = NULL; + *flags = 0; return 0; } diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index 1dba172..ebf8063 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -123,7 +123,8 @@ struct uefi_firmware { const char *code; /* code file (NULL = end of list) */ const char *code_debug; /* code file with debugging msgs (may be NULL)*/ const char *vars; /* vars template file */ - int flags; /* flags */ + int flags; /* various flags, see below */ +#define UEFI_FLAG_SECURE_BOOT_REQUIRED 1 /* secure boot (see RHBZ#1367615) */ }; extern struct uefi_firmware guestfs_int_ovmf_i386_firmware[]; extern struct uefi_firmware guestfs_int_ovmf_x86_64_firmware[]; diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 44dae32..2002253 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -756,7 +756,7 @@ extern const char *guestfs_int_drive_protocol_to_string (enum drive_protocol pro /* appliance.c */ extern int guestfs_int_build_appliance (guestfs_h *g, char **kernel, char **dtb, char **initrd, char **appliance); -extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars); +extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags); /* launch.c */ extern int64_t guestfs_int_timeval_diff (const struct timeval *x, const struct timeval *y); diff --git a/src/launch-direct.c b/src/launch-direct.c index 1081445..426bb60 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -242,6 +242,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) CLEANUP_FREE char *uefi_code = NULL, *uefi_vars = NULL; CLEANUP_FREE char *kernel = NULL, *dtb = NULL, *initrd = NULL, *appliance = NULL; + int uefi_flags; int has_appliance_drive; CLEANUP_FREE char *appliance_dev = NULL; uint32_t size; @@ -434,8 +435,20 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) } /* UEFI (firmware) if required. */ - if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars, &uefi_flags) == -1) goto cleanup0; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the qemu command line. + * See RHBZ#1367615 for details. As the guestfs_int_get_uefi + * function is only implemented for aarch64, and UEFI secure boot + * is some way off on aarch64 (2017/2018), we only need to worry + * about this later. + */ + error (g, "internal error: direct backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup0; + } if (uefi_code) { ADD_CMDLINE ("-drive"); ADD_CMDLINE_PRINTF ("if=pflash,format=raw,file=%s,readonly", uefi_code); diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 4c29409..505f3f0 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -312,6 +312,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) int r; uint32_t size; CLEANUP_FREE void *buf = NULL; + int uefi_flags; params.current_proc_is_root = geteuid () == 0; @@ -404,8 +405,21 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) goto cleanup; /* UEFI code and variables, on architectures where that is required. */ - if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars, + &uefi_flags) == -1) goto cleanup; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the libvirt XML. See + * RHBZ#1367615 for details. As the guestfs_int_get_uefi function + * is only implemented for aarch64, and UEFI secure boot is some + * way off on aarch64 (2017/2018), we only need to worry about + * this later. + */ + error (g, "internal error: libvirt backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup; + } /* Misc backend settings. */ guestfs_push_error_handler (g, NULL, NULL); diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml index df0963e..ed430a2 100644 --- a/v2v/output_libvirt.ml +++ b/v2v/output_libvirt.ml @@ -71,6 +71,16 @@ let target_features_of_capabilities_doc doc arch = let create_libvirt_xml ?pool source target_buses guestcaps target_features target_firmware = + let uefi_firmware = + match target_firmware with + | TargetBIOS -> None + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in + let memory_k = source.s_memory /^ 1024L in (* We have the machine features of the guest when it was on the @@ -106,24 +116,32 @@ let create_libvirt_xml ?pool source target_buses guestcaps StringSet.inter(*section*) force_features target_features in let features = StringSet.union features force_features in + (* Add feature if UEFI requires it. Note that libvirt + * capabilities doesn't list this feature even if it is supported + * by qemu, so we have to blindly add it, which might cause libvirt + * to fail. (XXX) + *) + let features = + if secure_boot_required then StringSet.add "smm" features else features in + let features = List.sort compare (StringSet.elements features) in (* The section subelements. *) let os_section = + let machine = if secure_boot_required then [ "machine", "q35" ] else [] in + let loader = - match target_firmware with - | TargetBIOS -> [] - | TargetUEFI -> - (* danpb is proposing that libvirt supports , - * (https://bugzilla.redhat.com/show_bug.cgi?id=1217444#c6) but - * until that day we have to use a bunch of heuristics. XXX - *) - let { Uefi.code = code; vars = vars_template } = - find_uefi_firmware guestcaps.gcaps_arch in - [ e "loader" ["readonly", "yes"; "type", "pflash"] [ PCData code ]; + match uefi_firmware with + | None -> [] + | Some { Uefi.code = code; vars = vars_template } -> + let secure = + if secure_boot_required then [ "secure", "yes" ] else [] in + [ e "loader" (["readonly", "yes"; "type", "pflash"] @ secure) + [ PCData code ]; e "nvram" ["template", vars_template] [] ] in - (e "type" ["arch", guestcaps.gcaps_arch] [PCData "hvm"]) :: loader in + (e "type" (["arch", guestcaps.gcaps_arch] @ machine) [PCData "hvm"]) + :: loader in (* The devices. *) let devices = ref [] in diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml index 2a944d7..55620d1 100644 --- a/v2v/output_qemu.ml +++ b/v2v/output_qemu.ml @@ -57,8 +57,12 @@ object let uefi_firmware = match target_firmware with | TargetBIOS -> None - | TargetUEFI -> - Some (find_uefi_firmware guestcaps.gcaps_arch) in + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in let chan = open_out file in @@ -79,11 +83,14 @@ object fpf "/usr/libexec/qemu-kvm"; fpf "%s-no-user-config -nodefaults" nl; fpf "%s-name %s" nl (quote source.s_name); - fpf "%s-machine accel=kvm:tcg" nl; + fpf "%s-machine %saccel=kvm:tcg" nl + (if secure_boot_required then "q35,smm=on," else ""); (match uefi_firmware with | None -> () | Some { Uefi.code = code } -> + if secure_boot_required then + fpf "%s-global driver=cfi.pflash01,property=secure,value=on" nl; fpf "%s-drive if=pflash,format=raw,file=%s,readonly" nl (quote code); fpf "%s-drive if=pflash,format=raw,file=\"$uefi_vars\"" nl ); diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod index 3e24025..32085c1 100644 --- a/v2v/virt-v2v.pod +++ b/v2v/virt-v2v.pod @@ -783,6 +783,8 @@ automatically, but note that the same version of OVMF must be installed on the conversion host as is installed on the target hypervisor, else you will have to adjust paths in the metadata. +On RHEL E 7.3, only qemu-kvm-rhev (not qemu-kvm) is supported. + =item UEFI on OpenStack Not supported. -- 2.7.4