Repository: lima-vm/lima
Pull request #2061 (2023-12-11 13:04:23)

qemu, aarch64: use EDK2 from "Firmware/edk2 20231213 patches" (needed for QEMU v8.2.0)
The "Firmware/edk2 20231213" patches <https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01694.html>
are necessary to boot most aarch64 images (except Debian, which does not use UEFI shim).

The patches are proposed for the QEMU v8.2.0 milestone, but likely to be postponed to v8.2.1.
Until the patches get accepted in the QEMU upstream, Lima fetches the patched
edk2 binary from <https://gitlab.com/kraxel/qemu/-/tags/firmware%2Fedk2-20231213-pull-request>.

AkihiroSuda: QEMU 8.2.0 is likely to be released with an incompatible version of EDK2, on 2023-12-12 or 2023-12-19:
https://gitlab.com/qemu-project/qemu/-/issues/1990#note_1688360743
https://wiki.qemu.org/Planning/8.2

We have to release a new version Lima with this PR before that.
cc @lima-vm/maintainers 

jandubois: I'm somewhat confused how these patches are related to the M3 issues on QEMU.

Are the patches necessary for most images to work at all on Apple chips, include M1 and M2, and without them QEMU 8.2.0 is broken for all distros except Debian?

But QEMU 8.1.0 is still good even without the firmware update?

And will the new firmware also fix the problem that even older QEMU versions don't work on M3? Or is that a completely separate issue?

And finally (although maybe not interesting): will the new firmware fix the M3 problem on older QEMU versions, like 8.0.0 or 8.1.0? Or does the new firmware require the new QEMU version?
AkihiroSuda: 
> I'm somewhat confused how these patches are related to the M3 issues on QEMU.

The story is quite complicated.


https://github.com/tianocore/edk2/pull/4418 (`ArmPkg/ArmMmuLib AARCH64: Add missing ISB after page table update`, merged on 2023-05-23) is expected to fix the issue for M3, but this revision is practically unusable for Lima (and many others) on any chip, because https://github.com/tianocore/edk2/pull/4150 (`Pull arm memory attr table`, merged on 2023-03-17) added the implementation of `EFI_MEMORY_ATTRIBUTE_PROTOCOL`, which makes almost all existing Linux distro images (except Debian, which does not use UEFI shim) unbootable, due to a broken support for `EFI_MEMORY_ATTRIBUTE_PROTOCOL` on the UEFI shim side.
(The UEFI shim issue was fixed in https://github.com/rhboot/shim/pull/539 , but the fix is only present in the main branch of the shim and not cherry-picked by any well-known distro.)

The QEMU patches (https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01694.html) bumps up EDK2
to include https://github.com/tianocore/edk2/pull/5104 (`ArmVirtPkg: Allow EFI memory attributes protocol to be disabled`, merged on 2023-12-12) and enables the `PcdUninstallMemAttrProtocol` build option to uninstall `EFI_MEMORY_ATTRIBUTE_PROTOCOL` so as to rescue us.

This revision should work on any chip, but I only tested it on M2 Pro.


> Are the patches necessary for most images to work at all on Apple chips, include M1 and M2, and without them QEMU 8.2.0 is broken for all distros except Debian?

Yes, necessary. Without the patches, QEMU v8.2.0-rc4 cannot boot most images on any chip.

> But QEMU 8.1.0 is still good even without the firmware update?

Yes, still good, except on M3.

> And will the new firmware also fix the problem that even older QEMU versions don't work on M3? Or is that a completely separate issue?

Separate.
- 2023-03-17 (https://github.com/tianocore/edk2/pull/4150): broke compatibility with existing releases of UFI shim
- 2023-05-23 (https://github.com/tianocore/edk2/pull/4418): fixed M3 compatibility
- 2023-12-12 (https://github.com/tianocore/edk2/pull/5104): provided a workaround for the UEFI shim issue

> And finally (although maybe not interesting): will the new firmware fix the M3 problem on older QEMU versions, like 8.0.0 or 8.1.0? Or does the new firmware require the new QEMU version?

The former, IIUC
(I don't have M3 to verify)


jandubois: > The former, IIUC

So (given that Rancher Desktop bundles QEMU 8.0.0 with the aarch64 version right now, and doesn't care about which version from homebrew is installed), we should be able to just replace the firmware in 8.0.0 and things should continue to work on M1 and M2, and start working for M3? That sounds great, if true.

> (I don't have M3 to verify)

Yeah, me neither. Making it the second Apple related issue I cannot reproduce and verify (the `mcxalr` issue is also still unresolved with our latest attempts). Maybe I can find someone who bought an M3 as their personal machine...
AkihiroSuda: > we should be able to just replace the firmware in 8.0.0 and things should continue to work on M1 and M2, and start working for M3? That sounds great, if true.

I believe so.

AkihiroSuda: The patches https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01694.html aren't likely going to be included in QEMU v8.2.0, so I'll merge this PR and release Lima v0.19.1 today.



Patch:
diff --git a/examples/default.yaml b/examples/default.yaml
index 9654fbd1..8df843bb 100644
--- a/examples/default.yaml
+++ b/examples/default.yaml
@@ -263,6 +263,14 @@ firmware:
   # Use legacy BIOS instead of UEFI. Ignored for aarch64.
   # 🟢 Builtin default: false
   legacyBIOS: null
+#  # Override UEFI images
+#  # 🟢 Builtin default: uses VM's default UEFI, except for qemu + aarch64.
+#  # See <https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01694.html>
+#  images:
+#  - location: "~/Downloads/edk2-aarch64-code.fd.gz"
+#    arch: "aarch64"
+#    digest: "sha256:..."
+#    vmType: "qemu"
 
 audio:
   # EXPERIMENTAL
diff --git a/pkg/limayaml/defaults.go b/pkg/limayaml/defaults.go
index 654b0d2d..8e076a38 100644
--- a/pkg/limayaml/defaults.go
+++ b/pkg/limayaml/defaults.go
@@ -112,6 +112,37 @@ func defaultGuestInstallPrefix() string {
 	return "/usr/local"
 }
 
+func defaultFirmwareImages() []FileWithVMType {
+	return []FileWithVMType{
+		/*
+			The "Firmware/edk2 20231213" patches <https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg01694.html>
+			are necessary to boot most aarch64 images (except Debian, which does not use UEFI shim).
+
+			The patches are proposed for the QEMU v8.2.0 milestone, but likely to be postponed to v8.2.1.
+			Until the patches get accepted in the QEMU upstream, Lima fetches the patched edk2 binary from
+			<https://gitlab.com/kraxel/qemu/-/tags/firmware%2Fedk2-20231213-pull-request>.
+		*/
+		{
+			File: File{
+				Location: "https://gitlab.com/kraxel/qemu/-/raw/704f7cad5105246822686f65765ab92045f71a3b/pc-bios/edk2-aarch64-code.fd.bz2",
+				Arch:     AARCH64,
+				Digest:   "sha256:a5fc228623891297f2d82e22ea56ec57cde93fea5ec01abf543e4ed5cacaf277",
+			},
+			VMType: QEMU,
+		},
+		// Mirror
+		{
+			File: File{
+				Location: "https://github.com/AkihiroSuda/qemu/raw/704f7cad5105246822686f65765ab92045f71a3b/pc-bios/edk2-aarch64-code.fd.bz2",
+				Arch:     AARCH64,
+				Digest:   "sha256:a5fc228623891297f2d82e22ea56ec57cde93fea5ec01abf543e4ed5cacaf277",
+			},
+			VMType: QEMU,
+		},
+		// TODO: what about ARMv7?
+	}
+}
+
 // FillDefault updates undefined fields in y with defaults from d (or built-in default), and overwrites with values from o.
 // Both d and o may be empty.
 //
@@ -283,6 +314,17 @@ func FillDefault(y, d, o *LimaYAML, filePath string) {
 		y.Firmware.LegacyBIOS = ptr.Of(false)
 	}
 
+	y.Firmware.Images = append(append(o.Firmware.Images, y.Firmware.Images...), d.Firmware.Images...)
+	if len(y.Firmware.Images) == 0 {
+		y.Firmware.Images = defaultFirmwareImages()
+	}
+	for i := range y.Firmware.Images {
+		f := &y.Firmware.Images[i]
+		if f.Arch == "" {
+			f.Arch = *y.Arch
+		}
+	}
+
 	if y.SSH.LocalPort == nil {
 		y.SSH.LocalPort = d.SSH.LocalPort
 	}
diff --git a/pkg/limayaml/limayaml.go b/pkg/limayaml/limayaml.go
index 2998a08b..ad927834 100644
--- a/pkg/limayaml/limayaml.go
+++ b/pkg/limayaml/limayaml.go
@@ -77,6 +77,11 @@ type File struct {
 	Digest   digest.Digest `yaml:"digest,omitempty" json:"digest,omitempty"`
 }
 
+type FileWithVMType struct {
+	File   `yaml:",inline"`
+	VMType VMType `yaml:"vmType,omitempty" json:"vmType,omitempty"`
+}
+
 type Kernel struct {
 	File    `yaml:",inline"`
 	Cmdline string `yaml:"cmdline,omitempty" json:"cmdline,omitempty"`
@@ -142,6 +147,10 @@ type Firmware struct {
 	// LegacyBIOS disables UEFI if set.
 	// LegacyBIOS is ignored for aarch64.
 	LegacyBIOS *bool `yaml:"legacyBIOS,omitempty" json:"legacyBIOS,omitempty"`
+
+	// Images specify UEFI images (edk2-aarch64-code.fd.gz).
+	// Defaults to built-in UEFI.
+	Images []FileWithVMType `yaml:"images,omitempty" json:"images,omitempty"`
 }
 
 type Audio struct {
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
index 4c0f4451..0e66bea9 100644
--- a/pkg/qemu/qemu.go
+++ b/pkg/qemu/qemu.go
@@ -569,12 +569,31 @@ func Cmdline(cfg Config) (exe string, args []string, err error) {
 		logrus.Warnf("field `firmware.legacyBIOS` is not supported for architecture %q, ignoring", *y.Arch)
 		legacyBIOS = false
 	}
-	if !legacyBIOS && *y.Arch != limayaml.RISCV64 {
-		firmware, err := getFirmware(exe, *y.Arch)
-		if err != nil {
-			return "", nil, err
+	if !legacyBIOS {
+		var firmware string
+		for _, f := range y.Firmware.Images {
+			switch f.VMType {
+			case "", limayaml.QEMU:
+				if f.Arch == *y.Arch {
+					firmwareCandidate := filepath.Join(cfg.InstanceDir, filenames.QemuEfiCodeFD)
+					if _, err = fileutils.DownloadFile(firmwareCandidate, f.File, true, "UEFI code", *y.Arch); err != nil {
+						logrus.WithError(err).Warnf("failed to download %q", f.Location)
+						continue
+					}
+					firmware = firmwareCandidate
+					break
+				}
+			}
+		}
+		if firmware == "" && *y.Arch != limayaml.RISCV64 {
+			firmware, err = getFirmware(exe, *y.Arch)
+			if err != nil {
+				return "", nil, err
+			}
+		}
+		if firmware != "" {
+			args = append(args, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", firmware))
 		}
-		args = append(args, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", firmware))
 	}
 
 	// Disk
diff --git a/pkg/store/filenames/filenames.go b/pkg/store/filenames/filenames.go
index c630fb53..466d552d 100644
--- a/pkg/store/filenames/filenames.go
+++ b/pkg/store/filenames/filenames.go
@@ -53,7 +53,8 @@ const (
 	HostAgentStdoutLog = "ha.stdout.log"
 	HostAgentStderrLog = "ha.stderr.log"
 	VzIdentifier       = "vz-identifier"
-	VzEfi              = "vz-efi"
+	VzEfi              = "vz-efi"           // efi variable store
+	QemuEfiCodeFD      = "qemu-efi-code.fd" // efi code; not always created
 
 	// SocketDir is the default location for forwarded sockets with a relative paths in HostSocket
 	SocketDir = "sock"
diff --git a/pkg/vz/vz_driver_darwin.go b/pkg/vz/vz_driver_darwin.go
index 50ba0b1d..33be0f68 100644
--- a/pkg/vz/vz_driver_darwin.go
+++ b/pkg/vz/vz_driver_darwin.go
@@ -78,6 +78,14 @@ func (l *LimaVzDriver) Validate() error {
 	if *l.Yaml.Firmware.LegacyBIOS {
 		return fmt.Errorf("`firmware.legacyBIOS` configuration is not supported for VZ driver")
 	}
+	for _, f := range l.Yaml.Firmware.Images {
+		switch f.VMType {
+		case "", limayaml.VZ:
+			if f.Arch == *l.Yaml.Arch {
+				return fmt.Errorf("`firmware.images` configuration is not supported for VZ driver")
+			}
+		}
+	}
 	if unknown := reflectutil.UnknownNonEmptyFields(l.Yaml, knownYamlProperties...); len(unknown) > 0 {
 		logrus.Warnf("vmType %s: ignoring %+v", *l.Yaml.VMType, unknown)
 	}
diff --git a/website/content/en/docs/dev/Internals/_index.md b/website/content/en/docs/dev/Internals/_index.md
index add318d2..b0f738e5 100644
--- a/website/content/en/docs/dev/Internals/_index.md
+++ b/website/content/en/docs/dev/Internals/_index.md
@@ -48,6 +48,7 @@ kernel:
 QEMU:
 - `qemu.pid`: QEMU PID
 - `qmp.sock`: QMP socket
+- `qemu-efi-code.fd`: QEMU UEFI code (not always present)
 
 VZ:
 - `vz.pid`: VZ PID


Test patch:
diff --git a/pkg/limayaml/defaults_test.go b/pkg/limayaml/defaults_test.go
index e8b64027..9c5a26db 100644
--- a/pkg/limayaml/defaults_test.go
+++ b/pkg/limayaml/defaults_test.go
@@ -181,6 +181,27 @@ func TestFillDefault(t *testing.T) {
 				"-----BEGIN CERTIFICATE-----\nYOUR-ORGS-TRUSTED-CA-CERT\n-----END CERTIFICATE-----\n",
 			},
 		},
+		Firmware: Firmware{
+			LegacyBIOS: ptr.Of(false),
+			Images: []FileWithVMType{
+				{
+					File: File{
+						Location: "https://gitlab.com/kraxel/qemu/-/raw/704f7cad5105246822686f65765ab92045f71a3b/pc-bios/edk2-aarch64-code.fd.bz2",
+						Arch:     AARCH64,
+						Digest:   "sha256:a5fc228623891297f2d82e22ea56ec57cde93fea5ec01abf543e4ed5cacaf277",
+					},
+					VMType: QEMU,
+				},
+				{
+					File: File{
+						Location: "https://github.com/AkihiroSuda/qemu/raw/704f7cad5105246822686f65765ab92045f71a3b/pc-bios/edk2-aarch64-code.fd.bz2",
+						Arch:     AARCH64,
+						Digest:   "sha256:a5fc228623891297f2d82e22ea56ec57cde93fea5ec01abf543e4ed5cacaf277",
+					},
+					VMType: QEMU,
+				},
+			},
+		},
 	}
 
 	expect := builtin
@@ -252,6 +273,8 @@ func TestFillDefault(t *testing.T) {
 		},
 	}
 
+	expect.Firmware = y.Firmware
+
 	expect.Rosetta = Rosetta{
 		Enabled: ptr.Of(false),
 		BinFmt:  ptr.Of(false),
@@ -299,6 +322,14 @@ func TestFillDefault(t *testing.T) {
 		},
 		Firmware: Firmware{
 			LegacyBIOS: ptr.Of(true),
+			Images: []FileWithVMType{
+				{
+					File: File{
+						Location: "/dummy",
+						Arch:     X8664,
+					},
+				},
+			},
 		},
 		Audio: Audio{
 			Device: ptr.Of("coreaudio"),
@@ -427,6 +458,7 @@ func TestFillDefault(t *testing.T) {
 	expect.CopyToHost = append(append([]CopyToHost{}, y.CopyToHost...), d.CopyToHost...)
 	expect.Containerd.Archives = append(append([]File{}, y.Containerd.Archives...), d.Containerd.Archives...)
 	expect.AdditionalDisks = append(append([]Disk{}, y.AdditionalDisks...), d.AdditionalDisks...)
+	expect.Firmware.Images = append(append([]FileWithVMType{}, y.Firmware.Images...), d.Firmware.Images...)
 
 	// Mounts and Networks start with lowest priority first, so higher priority entries can overwrite
 	expect.Mounts = append(append([]Mount{}, d.Mounts...), y.Mounts...)
@@ -580,6 +612,7 @@ func TestFillDefault(t *testing.T) {
 	expect.CopyToHost = append(append(o.CopyToHost, y.CopyToHost...), d.CopyToHost...)
 	expect.Containerd.Archives = append(append(o.Containerd.Archives, y.Containerd.Archives...), d.Containerd.Archives...)
 	expect.AdditionalDisks = append(append(o.AdditionalDisks, y.AdditionalDisks...), d.AdditionalDisks...)
+	expect.Firmware.Images = append(append(o.Firmware.Images, y.Firmware.Images...), d.Firmware.Images...)
 
 	expect.HostResolver.Hosts["default"] = d.HostResolver.Hosts["default"]
 	expect.HostResolver.Hosts["MY.Host"] = d.HostResolver.Hosts["host.lima.internal"]