Merge pull request #12750 from mihalicyn/force_seabios_for_csm

Force SeaBIOS instead of OVMF-based firmware & some firmware lookup logic changes

doc/.wordlist.txt

@@ -311,3 +311,5 @@ ZFS
 zpool
 zpools
 qdisc
+firmware
+SeaBIOS

doc/environment.md

@@ -36,6 +36,7 @@ Name                            | Description
 `LXD_LXC_TEMPLATE_CONFIG`       | Path to the LXC template configuration directory
 `LXD_SECURITY_APPARMOR`         | If set to `false`, forces AppArmor off
 `LXD_UNPRIVILEGED_ONLY`         | If set to `true`, enforces that only unprivileged containers can be created. Note that any privileged containers that have been created before setting LXD_UNPRIVILEGED_ONLY will continue to be privileged. To use this option effectively it should be set when the LXD daemon is first set up.
-`LXD_OVMF_PATH`                 | Path to an OVMF build including `OVMF_CODE.fd` and `OVMF_VARS.ms.fd`
+`LXD_OVMF_PATH`                 | Path to an OVMF build including `OVMF_CODE.fd` and `OVMF_VARS.ms.fd` (deprecated, please use `LXD_QEMU_FW_PATH` instead)
+`LXD_QEMU_FW_PATH`              | Path (or `:` separated list of paths) to firmware (OVMF, SeaBIOS) to be used by QEMU
 `LXD_IDMAPPED_MOUNTS_DISABLE`   | Disable idmapped mounts support (useful when testing traditional UID shifting)
 `LXD_DEVMONITOR_DIR`            | Path to be monitored by the device monitor. This is primarily for testing.

lxd/apparmor/instance.go

@@ -182,12 +182,7 @@ func instanceProfile(sysOS *sys.OS, inst instance) (string, error) {
 			return "", err
 		}
 
-		ovmfPath := "/usr/share/OVMF"
-		if os.Getenv("LXD_OVMF_PATH") != "" {
-			ovmfPath = os.Getenv("LXD_OVMF_PATH")
-		}
-
-		ovmfPath, err = filepath.EvalSymlinks(ovmfPath)
+		qemuFwPathsArr, err := util.GetQemuFwPaths()
 		if err != nil {
 			return "", err
 		}
@@ -209,7 +204,7 @@ func instanceProfile(sysOS *sys.OS, inst instance) (string, error) {
 			"rootPath":    rootPath,
 			"snap":        shared.InSnap(),
 			"userns":      sysOS.RunningInUserNS,
-			"ovmfPath":    ovmfPath,
+			"qemuFwPaths": qemuFwPathsArr,
 		})
 		if err != nil {
 			return "", err

lxd/apparmor/instance_qemu.go

@@ -39,8 +39,6 @@ profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) {
   /sys/module/vhost/**                      r,
   /tmp/lxd_sev_*                            r,
   /{,usr/}bin/qemu-system-*                 mrix,
-  {{ .ovmfPath }}/OVMF_CODE.fd              kr,
-  {{ .ovmfPath }}/OVMF_CODE.*.fd            kr,
   /usr/share/qemu/**                        kr,
   /usr/share/seabios/**                     kr,
   owner @{PROC}/@{pid}/cpuset               r,
@@ -94,6 +92,15 @@ profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) {
 {{- end }}
 {{- end }}
 
+{{if .qemuFwPaths -}}
+  # Entries from LXD_OVMF_PATH or LXD_QEMU_FW_PATH
+{{range $index, $element := .qemuFwPaths}}
+  {{$element}}/OVMF_CODE.fd   kr,
+  {{$element}}/OVMF_CODE.*.fd kr,
+  {{$element}}/*bios*.bin     kr,
+{{- end }}
+{{- end }}
+
 {{- if .raw }}
 
   ### Configuration: raw.apparmor

lxd/instance/drivers/driver_qemu.go

@@ -103,31 +103,32 @@ const qemuBlockDevIDPrefix = "lxd_"
 // qemuMigrationNBDExportName is the name of the disk device export by the migration NBD server.
 const qemuMigrationNBDExportName = "lxd_root"
 
-// OVMF firmwares.
-type ovmfFirmware struct {
+// VM firmwares.
+type vmFirmware struct {
 	code string
 	vars string
 }
 
 // Debug version of the "default" firmware.
-var ovmfDebugFirmware = "OVMF_CODE.4MB.debug.fd"
+var vmDebugFirmware = "OVMF_CODE.4MB.debug.fd"
 
-var ovmfGenericFirmwares = []ovmfFirmware{
+var vmGenericFirmwares = []vmFirmware{
 	{code: "OVMF_CODE.4MB.fd", vars: "OVMF_VARS.4MB.fd"},
 	{code: "OVMF_CODE.2MB.fd", vars: "OVMF_VARS.2MB.fd"},
 	{code: "OVMF_CODE.fd", vars: "OVMF_VARS.fd"},
 	{code: "OVMF_CODE.fd", vars: "qemu.nvram"},
 }
 
-var ovmfSecurebootFirmwares = []ovmfFirmware{
+var vmSecurebootFirmwares = []vmFirmware{
 	{code: "OVMF_CODE.4MB.fd", vars: "OVMF_VARS.4MB.ms.fd"},
 	{code: "OVMF_CODE.2MB.fd", vars: "OVMF_VARS.2MB.ms.fd"},
 	{code: "OVMF_CODE.fd", vars: "OVMF_VARS.ms.fd"},
 	{code: "OVMF_CODE.fd", vars: "qemu.nvram"},
 }
 
-var ovmfCSMFirmwares = []ovmfFirmware{
-	{code: "seabios.bin", vars: "seabios.bin"},
+// Only valid for x86_64.
+var vmLegacyFirmwares = []vmFirmware{
+	{code: "bios-256k.bin", vars: "bios-256k.bin"},
 	{code: "OVMF_CODE.4MB.CSM.fd", vars: "OVMF_VARS.4MB.CSM.fd"},
 	{code: "OVMF_CODE.2MB.CSM.fd", vars: "OVMF_VARS.2MB.CSM.fd"},
 	{code: "OVMF_CODE.CSM.fd", vars: "OVMF_VARS.CSM.fd"},
@@ -784,12 +785,27 @@ func (d *qemu) Rebuild(img *api.Image, op *operations.Operation) error {
 	return d.rebuildCommon(d, img, op)
 }
 
-func (d *qemu) ovmfPath() string {
-	if os.Getenv("LXD_OVMF_PATH") != "" {
-		return os.Getenv("LXD_OVMF_PATH")
+func (*qemu) fwPath(filename string) string {
+	qemuFwPathsArr, err := util.GetQemuFwPaths()
+	if err != nil {
+		return ""
+	}
+
+	// GetQemuFwPaths resolves symlinks for us, but we still need EvalSymlinks() in here,
+	// because filename itself can be a symlink.
+	for _, path := range qemuFwPathsArr {
+		filePath := filepath.Join(path, filename)
+		filePath, err := filepath.EvalSymlinks(filePath)
+		if err != nil {
+			continue
+		}
+
+		if shared.PathExists(filePath) {
+			return filePath
+		}
 	}
 
-	return "/usr/share/OVMF"
+	return ""
 }
 
 // killQemuProcess kills specified process. Optimistically attempts to wait for the process to fully exit, but does
@@ -1104,9 +1120,21 @@ func (d *qemu) start(stateful bool, op *operationlock.InstanceOperation) error {
 		return fmt.Errorf("The image used by this instance is incompatible with secureboot. Please set security.secureboot=false on the instance")
 	}
 
-	// Ensure secureboot is turned off when CSM is on
-	if shared.IsTrue(d.expandedConfig["security.csm"]) && shared.IsTrueOrEmpty(d.expandedConfig["security.secureboot"]) {
-		return fmt.Errorf("Secure boot can't be enabled while CSM is turned on. Please set security.secureboot=false on the instance")
+	if shared.IsTrue(d.expandedConfig["security.csm"]) {
+		// Ensure CSM is turned off for all arches except x86_64
+		if d.architecture != osarch.ARCH_64BIT_INTEL_X86 {
+			return fmt.Errorf("CSM can be enabled for x86_64 architecture only. Please set security.csm=false on the instance")
+		}
+
+		// Having boot.debug_edk2 enabled contradicts with enabling CSM
+		if shared.IsTrue(d.localConfig["boot.debug_edk2"]) {
+			return fmt.Errorf("CSM can not be enabled together with boot.debug_edk2. Please set one of them to false")
+		}
+
+		// Ensure secureboot is turned off when CSM is on
+		if shared.IsTrueOrEmpty(d.expandedConfig["security.secureboot"]) {
+			return fmt.Errorf("Secure boot can't be enabled while CSM is turned on. Please set security.secureboot=false on the instance")
+		}
 	}
 
 	// Setup a new operation if needed.
@@ -1231,7 +1259,7 @@ func (d *qemu) start(stateful bool, op *operationlock.InstanceOperation) error {
 		return err
 	}
 
-	// Copy OVMF settings firmware to nvram file if needed.
+	// Copy VM firmware settings firmware to nvram file if needed.
 	// This firmware file can be modified by the VM so it must be copied from the defaults.
 	if d.architectureSupportsUEFI(d.architecture) && (!shared.PathExists(d.nvramPath()) || shared.IsTrue(d.localConfig["volatile.apply_nvram"])) {
 		err = d.setupNvram()
@@ -1466,11 +1494,6 @@ func (d *qemu) start(stateful bool, op *operationlock.InstanceOperation) error {
 		qemuCmd = append(qemuCmd, "-debugcon", "file:"+d.EDK2LogFilePath(), "-global", "isa-debugcon.iobase=0x402")
 	}
 
-	// This feature specific to the snap-shipped CSM edk2 version, because we have a custom patch to make it work.
-	if shared.InSnap() && shared.IsTrue(d.expandedConfig["security.csm"]) {
-		qemuCmd = append(qemuCmd, "-fw_cfg", "name=opt/com.canonical.lxd/force_csm,string=yes")
-	}
-
 	// If stateful, restore now.
 	if stateful {
 		if !d.stateful {
@@ -1954,7 +1977,7 @@ func (d *qemu) setupNvram() error {
 	d.logger.Debug("Generating NVRAM")
 
 	// Cleanup existing variables.
-	for _, firmwares := range [][]ovmfFirmware{ovmfGenericFirmwares, ovmfSecurebootFirmwares, ovmfCSMFirmwares} {
+	for _, firmwares := range [][]vmFirmware{vmGenericFirmwares, vmSecurebootFirmwares, vmLegacyFirmwares} {
 		for _, firmware := range firmwares {
 			err := os.Remove(filepath.Join(d.Path(), firmware.vars))
 			if err != nil && !os.IsNotExist(err) {
@@ -1964,45 +1987,41 @@ func (d *qemu) setupNvram() error {
 	}
 
 	// Determine expected firmware.
-	firmwares := ovmfGenericFirmwares
+	firmwares := vmGenericFirmwares
 	if shared.IsTrue(d.expandedConfig["security.csm"]) {
-		firmwares = ovmfCSMFirmwares
+		firmwares = vmLegacyFirmwares
 	} else if shared.IsTrueOrEmpty(d.expandedConfig["security.secureboot"]) {
-		firmwares = ovmfSecurebootFirmwares
+		firmwares = vmSecurebootFirmwares
 	}
 
 	// Find the template file.
-	var ovmfVarsPath string
-	var ovmfVarsName string
+	var vmfVarsPath string
+	var vmfVarsName string
 	for _, firmware := range firmwares {
-		varsPath := filepath.Join(d.ovmfPath(), firmware.vars)
-		varsPath, err = filepath.EvalSymlinks(varsPath)
-		if err != nil {
-			continue
-		}
+		varsPath := d.fwPath(firmware.vars)
 
-		if shared.PathExists(varsPath) {
-			ovmfVarsPath = varsPath
-			ovmfVarsName = firmware.vars
+		if varsPath != "" {
+			vmfVarsPath = varsPath
+			vmfVarsName = firmware.vars
 			break
 		}
 	}
 
-	if ovmfVarsPath == "" {
-		return fmt.Errorf("Couldn't find one of the required UEFI firmware files: %+v", firmwares)
+	if vmfVarsPath == "" {
+		return fmt.Errorf("Couldn't find one of the required firmware files: %+v", firmwares)
 	}
 
 	// Copy the template.
-	err = shared.FileCopy(ovmfVarsPath, filepath.Join(d.Path(), ovmfVarsName))
+	err = shared.FileCopy(vmfVarsPath, filepath.Join(d.Path(), vmfVarsName))
 	if err != nil {
 		return err
 	}
 
 	// Generate a symlink if needed.
-	// This is so qemu.nvram can always be assumed to be the OVMF vars file.
+	// This is so qemu.nvram can always be assumed to be the VM firmware vars file.
 	// The real file name is then used to determine what firmware must be selected.
 	if !shared.PathExists(d.nvramPath()) {
-		err = os.Symlink(ovmfVarsName, d.nvramPath())
+		err = os.Symlink(vmfVarsName, d.nvramPath())
 		if err != nil {
 			return err
 		}
@@ -3041,43 +3060,51 @@ func (d *qemu) generateQemuConfigFile(cpuInfo *cpuTopology, mountInfo *storagePo
 		}
 
 		// Determine expected firmware.
-		firmwares := ovmfGenericFirmwares
+		firmwares := vmGenericFirmwares
 		if shared.IsTrue(d.expandedConfig["security.csm"]) {
-			firmwares = ovmfCSMFirmwares
+			firmwares = vmLegacyFirmwares
 		} else if shared.IsTrueOrEmpty(d.expandedConfig["security.secureboot"]) {
-			firmwares = ovmfSecurebootFirmwares
+			firmwares = vmSecurebootFirmwares
 		}
 
-		var ovmfCode string
+		var vmfCode string
 		for _, firmware := range firmwares {
 			if shared.PathExists(filepath.Join(d.Path(), firmware.vars)) {
-				ovmfCode = firmware.code
+				vmfCode = firmware.code
 				break
 			}
 		}
 
-		if ovmfCode == "" {
+		if vmfCode == "" {
 			return "", nil, fmt.Errorf("Unable to locate matching firmware: %+v", firmwares)
 		}
 
 		// As 2MB firmware was deprecated in the LXD snap we have to regenerate NVRAM for VMs which used the 2MB one.
-		if shared.InSnap() && !strings.Contains(ovmfCode, "4MB") {
+		// As EDK2-based CSM firmwares were deprecated in the LXD snap we want to force VMs to start using SeaBIOS directly.
+		isOVMF2MB := (strings.Contains(vmfCode, "OVMF") && !strings.Contains(vmfCode, "4MB"))
+		isOVMFCSM := (strings.Contains(vmfCode, "OVMF") && strings.Contains(vmfCode, "CSM"))
+		if shared.InSnap() && (isOVMF2MB || isOVMFCSM) {
 			err = d.setupNvram()
 			if err != nil {
 				return "", nil, err
 			}
 
-			// force to use a 4MB firmware
-			ovmfCode = firmwares[0].code
+			// force to use a top-priority firmware
+			vmfCode = firmwares[0].code
 		}
 
 		// Use debug version of firmware. (Only works for "default" (4MB, no CSM) firmware flavor)
-		if shared.IsTrue(d.localConfig["boot.debug_edk2"]) && ovmfCode == ovmfGenericFirmwares[0].code {
-			ovmfCode = ovmfDebugFirmware
+		if shared.IsTrue(d.localConfig["boot.debug_edk2"]) && vmfCode == vmGenericFirmwares[0].code {
+			vmfCode = vmDebugFirmware
+		}
+
+		fwPath := d.fwPath(vmfCode)
+		if fwPath == "" {
+			return "", nil, fmt.Errorf("Unable to locate the file for firmware %q", vmfCode)
 		}
 
 		driveFirmwareOpts := qemuDriveFirmwareOpts{
-			roPath:    filepath.Join(d.ovmfPath(), ovmfCode),
+			roPath:    fwPath,
 			nvramPath: fmt.Sprintf("/dev/fd/%d", d.addFileDescriptor(fdFiles, nvRAMFile)),
 		}
 
@@ -8346,13 +8373,18 @@ func (d *qemu) checkFeatures(hostArch int, qemuPath string) (map[string]any, err
 	}
 
 	if d.architectureSupportsUEFI(hostArch) {
-		ovmfCode := "OVMF_CODE.fd"
+		vmfCode := "OVMF_CODE.fd"
 
 		if shared.InSnap() {
-			ovmfCode = ovmfGenericFirmwares[0].code
+			vmfCode = vmGenericFirmwares[0].code
+		}
+
+		fwPath := d.fwPath(vmfCode)
+		if fwPath == "" {
+			return nil, fmt.Errorf("Unable to locate the file for firmware %q", vmfCode)
 		}
 
-		qemuArgs = append(qemuArgs, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", filepath.Join(d.ovmfPath(), ovmfCode)))
+		qemuArgs = append(qemuArgs, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", fwPath))
 	}
 
 	var stderr bytes.Buffer

lxd/util/sys.go

@@ -3,7 +3,9 @@
 package util
 
 import (
+	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"golang.org/x/sys/unix"
@@ -63,3 +65,40 @@ func ReplaceDaemon() error {
 
 	return nil
 }
+
+// GetQemuFwPaths returns a list of directory paths to search for QEMU firmware files.
+func GetQemuFwPaths() ([]string, error) {
+	var qemuFwPaths []string
+
+	for _, v := range []string{"LXD_QEMU_FW_PATH", "LXD_OVMF_PATH"} {
+		searchPaths := os.Getenv(v)
+		if searchPaths == "" {
+			continue
+		}
+
+		qemuFwPaths = append(qemuFwPaths, strings.Split(searchPaths, ":")...)
+	}
+
+	// Append default paths after ones extracted from env vars so they take precedence.
+	qemuFwPaths = append(qemuFwPaths, "/usr/share/OVMF", "/usr/share/seabios")
+
+	count := 0
+	for i, path := range qemuFwPaths {
+		var err error
+		resolvedPath, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			// don't fail, just skip as some search paths can be optional
+			continue
+		}
+
+		count++
+		qemuFwPaths[i] = resolvedPath
+	}
+
+	// We want to have at least one valid path to search for firmware.
+	if count == 0 {
+		return nil, fmt.Errorf("Failed to find a valid search path for firmware")
+	}
+
+	return qemuFwPaths, nil
+}