[PULL 0/7] Firmware 20241216 patches

[PULL 0/7] Firmware 20241216 patches

[Gerd Hoffmann]

The following changes since commit ca80a5d026a280762e0772615f1988db542b3ade:

  Merge tag 'hw-misc-20241214' of https://github.com/philmd/qemu into staging (2024-12-14 08:42:53 -0500)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git tags/firmware-20241216-pull-request

for you to fetch changes up to 0f5715e4b5706b31b3550d8e6b88871e029c7823:

  roms: re-add edk2-basetools target (2024-12-16 07:31:28 +0100)

----------------------------------------------------------------
x86/loader: fix efi binary loading
x86/loader: support secure boot with direct kernel load
firmware: json descriptor updates
roms: re-add edk2-basetools target

----------------------------------------------------------------

Gerd Hoffmann (5):
  x86/loader: only patch linux kernels
  x86/loader: read complete kernel
  x86/loader: expose unpatched kernel
  x86/loader: add -shim option
  roms: re-add edk2-basetools target

Heinrich Schuchardt (1):
  pc-bios: add missing riscv64 descriptor

Pratik R. Sampat (1):
  pc-bios: Add amd-sev-es to edk2 json

 include/hw/boards.h                      |  1 +
 hw/core/machine.c                        | 20 +++++++++++++++
 hw/i386/x86-common.c                     | 32 +++++++++++++++++++-----
 system/vl.c                              |  9 +++++++
 pc-bios/descriptors/60-edk2-riscv64.json | 31 +++++++++++++++++++++++
 pc-bios/descriptors/60-edk2-x86_64.json  |  1 +
 pc-bios/descriptors/meson.build          |  3 ++-
 qemu-options.hx                          |  7 ++++++
 roms/Makefile                            |  5 ++++
 9 files changed, 102 insertions(+), 7 deletions(-)
 create mode 100644 pc-bios/descriptors/60-edk2-riscv64.json

-- 
2.47.1

[PULL 1/7] x86/loader: only patch linux kernels

[Gerd Hoffmann]

If the binary loaded via -kernel is *not* a linux kernel (in which
case protocol == 0), do not patch the linux kernel header fields.

It's (a) pointless and (b) might break binaries by random patching
and (c) changes the binary hash which in turn breaks secure boot
verification.

Background: OVMF happily loads and runs not only linux kernels but
any efi binary via direct kernel boot.

Note: Breaking the secure boot verification is a problem for linux
kernels too, but fixed that is left for another day ...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-ID: <20240905141211.1253307-3-kraxel@redhat.com>
---
 hw/i386/x86-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index dc031af66217..dadc9d99e78f 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -945,7 +945,7 @@ void x86_load_linux(X86MachineState *x86ms,
      * kernel on the other side of the fw_cfg interface matches the hash of the
      * file the user passed in.
      */
-    if (!sev_enabled()) {
+    if (!sev_enabled() && protocol > 0) {
         memcpy(setup, header, MIN(sizeof(header), setup_size));
     }
 
-- 
2.47.1

[PULL 2/7] x86/loader: read complete kernel

[Gerd Hoffmann]

Load the complete kernel (including setup) into memory.  Excluding the
setup is handled later when adding the FW_CFG_KERNEL_SIZE and
FW_CFG_KERNEL_DATA entries.

This is a preparation for the next patch which adds a new fw_cfg file
containing the complete, unpatched kernel.  No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-ID: <20240905141211.1253307-4-kraxel@redhat.com>
---
 hw/i386/x86-common.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index dadc9d99e78f..28341b42d949 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -895,7 +895,6 @@ void x86_load_linux(X86MachineState *x86ms,
         fprintf(stderr, "qemu: invalid kernel header\n");
         exit(1);
     }
-    kernel_size -= setup_size;
 
     setup  = g_malloc(setup_size);
     kernel = g_malloc(kernel_size);
@@ -904,6 +903,7 @@ void x86_load_linux(X86MachineState *x86ms,
         fprintf(stderr, "fread() failed\n");
         exit(1);
     }
+    fseek(f, 0, SEEK_SET);
     if (fread(kernel, 1, kernel_size, f) != kernel_size) {
         fprintf(stderr, "fread() failed\n");
         exit(1);
@@ -950,10 +950,11 @@ void x86_load_linux(X86MachineState *x86ms,
     }
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
-    sev_load_ctx.kernel_data = (char *)kernel;
-    sev_load_ctx.kernel_size = kernel_size;
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size - setup_size);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
+                     kernel + setup_size, kernel_size - setup_size);
+    sev_load_ctx.kernel_data = (char *)kernel + setup_size;
+    sev_load_ctx.kernel_size = kernel_size - setup_size;
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
     fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
-- 
2.47.1

[PULL 3/7] x86/loader: expose unpatched kernel

[Gerd Hoffmann]

Add a new "etc/boot/kernel" fw_cfg file, containing the kernel without
the setup header patches.  Intended use is booting in UEFI with secure
boot enabled, where the setup header patching breaks secure boot
verification.

Needs OVMF changes too to be actually useful.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-ID: <20240905141211.1253307-5-kraxel@redhat.com>
---
 hw/i386/x86-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index 28341b42d949..1cef3045ad83 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -962,6 +962,9 @@ void x86_load_linux(X86MachineState *x86ms,
     sev_load_ctx.setup_data = (char *)setup;
     sev_load_ctx.setup_size = setup_size;
 
+    /* kernel without setup header patches */
+    fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
+
     if (sev_enabled()) {
         sev_add_kernel_loader_hashes(&sev_load_ctx, &error_fatal);
     }
-- 
2.47.1

Re: [PULL 3/7] x86/loader: expose unpatched kernel

[Daniel P. Berrangé]

On Mon, Dec 16, 2024 at 11:50:49AM +0100, Gerd Hoffmann wrote:
> Add a new "etc/boot/kernel" fw_cfg file, containing the kernel without
> the setup header patches.  Intended use is booting in UEFI with secure
> boot enabled, where the setup header patching breaks secure boot
> verification.
> 
> Needs OVMF changes too to be actually useful.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Message-ID: <20240905141211.1253307-5-kraxel@redhat.com>
> ---
>  hw/i386/x86-common.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> index 28341b42d949..1cef3045ad83 100644
> --- a/hw/i386/x86-common.c
> +++ b/hw/i386/x86-common.c
> @@ -962,6 +962,9 @@ void x86_load_linux(X86MachineState *x86ms,
>      sev_load_ctx.setup_data = (char *)setup;
>      sev_load_ctx.setup_size = setup_size;
>  
> +    /* kernel without setup header patches */
> +    fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
> +

How concerned should we be about the memory duplication overhead
from loading the kernel image twice ?

A bare modular kernel is 16MB, a non-modular one would be bigger
perhaps 10's of MB, a UKI meanwhile could easily be 100's of MB
in size, and >=1GB is not entirely insane to contemplate with a
UKI depending on how much is put into the embedded initrd.

I don't think the memory for fw_cfg is counted against the guest
RAM, right?

Is there anyway the duplicate kernel gets erased from memory once
the firmware is done, otherwise we live with this extra host RAM
overhead forever ?

>      if (sev_enabled()) {
>          sev_add_kernel_loader_hashes(&sev_load_ctx, &error_fatal);
>      }

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 3/7] x86/loader: expose unpatched kernel

[Gerd Hoffmann]

On Tue, Dec 17, 2024 at 02:15:15PM +0000, Daniel P. Berrangé wrote:
> On Mon, Dec 16, 2024 at 11:50:49AM +0100, Gerd Hoffmann wrote:
> > Add a new "etc/boot/kernel" fw_cfg file, containing the kernel without
> > the setup header patches.  Intended use is booting in UEFI with secure
> > boot enabled, where the setup header patching breaks secure boot
> > verification.
> > 
> > Needs OVMF changes too to be actually useful.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > Message-ID: <20240905141211.1253307-5-kraxel@redhat.com>
> > ---
> >  hw/i386/x86-common.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> > index 28341b42d949..1cef3045ad83 100644
> > --- a/hw/i386/x86-common.c
> > +++ b/hw/i386/x86-common.c
> > @@ -962,6 +962,9 @@ void x86_load_linux(X86MachineState *x86ms,
> >      sev_load_ctx.setup_data = (char *)setup;
> >      sev_load_ctx.setup_size = setup_size;
> >  
> > +    /* kernel without setup header patches */
> > +    fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
> > +
> 
> How concerned should we be about the memory duplication overhead
> from loading the kernel image twice ?

It's not loaded twice, see 214191f6b574 ("x86/loader: read complete
kernel"), both fw_cfg entries point to the same memory block.

take care,
  Gerd

Re: [PULL 3/7] x86/loader: expose unpatched kernel

[Daniel P. Berrangé]

On Tue, Dec 17, 2024 at 03:26:35PM +0100, Gerd Hoffmann wrote:
> On Tue, Dec 17, 2024 at 02:15:15PM +0000, Daniel P. Berrangé wrote:
> > On Mon, Dec 16, 2024 at 11:50:49AM +0100, Gerd Hoffmann wrote:
> > > Add a new "etc/boot/kernel" fw_cfg file, containing the kernel without
> > > the setup header patches.  Intended use is booting in UEFI with secure
> > > boot enabled, where the setup header patching breaks secure boot
> > > verification.
> > > 
> > > Needs OVMF changes too to be actually useful.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > Message-ID: <20240905141211.1253307-5-kraxel@redhat.com>
> > > ---
> > >  hw/i386/x86-common.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> > > index 28341b42d949..1cef3045ad83 100644
> > > --- a/hw/i386/x86-common.c
> > > +++ b/hw/i386/x86-common.c
> > > @@ -962,6 +962,9 @@ void x86_load_linux(X86MachineState *x86ms,
> > >      sev_load_ctx.setup_data = (char *)setup;
> > >      sev_load_ctx.setup_size = setup_size;
> > >  
> > > +    /* kernel without setup header patches */
> > > +    fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
> > > +
> > 
> > How concerned should we be about the memory duplication overhead
> > from loading the kernel image twice ?
> 
> It's not loaded twice, see 214191f6b574 ("x86/loader: read complete
> kernel"), both fw_cfg entries point to the same memory block.

Ah, I see now, that's subtle :-)


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PULL 4/7] x86/loader: add -shim option

[Gerd Hoffmann]

Add new -shim command line option, wire up for the x86 loader.
When specified load shim into the new "etc/boot/shim" fw_cfg file.

Needs OVMF changes too to be actually useful.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-ID: <20240905141211.1253307-6-kraxel@redhat.com>
---
 include/hw/boards.h  |  1 +
 hw/core/machine.c    | 20 ++++++++++++++++++++
 hw/i386/x86-common.c | 16 ++++++++++++++++
 system/vl.c          |  9 +++++++++
 qemu-options.hx      |  7 +++++++
 5 files changed, 53 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 7456889c37eb..5723ee76bdea 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -431,6 +431,7 @@ struct MachineState {
     BootConfiguration boot_config;
     char *kernel_filename;
     char *kernel_cmdline;
+    char *shim_filename;
     char *initrd_filename;
     const char *cpu_type;
     AccelState *accelerator;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index e6900b43efa2..d970f753e370 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -305,6 +305,21 @@ static void machine_set_kernel(Object *obj, const char *value, Error **errp)
     ms->kernel_filename = g_strdup(value);
 }
 
+static char *machine_get_shim(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return g_strdup(ms->shim_filename);
+}
+
+static void machine_set_shim(Object *obj, const char *value, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    g_free(ms->shim_filename);
+    ms->shim_filename = g_strdup(value);
+}
+
 static char *machine_get_initrd(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -1082,6 +1097,11 @@ static void machine_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "kernel",
         "Linux kernel image file");
 
+    object_class_property_add_str(oc, "shim",
+        machine_get_shim, machine_set_shim);
+    object_class_property_set_description(oc, "shim",
+        "shim.efi file");
+
     object_class_property_add_str(oc, "initrd",
         machine_get_initrd, machine_set_initrd);
     object_class_property_set_description(oc, "initrd",
diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index 1cef3045ad83..3f7818269234 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -965,6 +965,22 @@ void x86_load_linux(X86MachineState *x86ms,
     /* kernel without setup header patches */
     fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
 
+    if (machine->shim_filename) {
+        GMappedFile *mapped_file;
+        GError *gerr = NULL;
+
+        mapped_file = g_mapped_file_new(machine->shim_filename, false, &gerr);
+        if (!mapped_file) {
+            fprintf(stderr, "qemu: error reading shim %s: %s\n",
+                    machine->shim_filename, gerr->message);
+            exit(1);
+        }
+
+        fw_cfg_add_file(fw_cfg, "etc/boot/shim",
+                        g_mapped_file_get_contents(mapped_file),
+                        g_mapped_file_get_length(mapped_file));
+    }
+
     if (sev_enabled()) {
         sev_add_kernel_loader_hashes(&sev_load_ctx, &error_fatal);
     }
diff --git a/system/vl.c b/system/vl.c
index 4a370da624a7..09202b57e73b 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2427,6 +2427,7 @@ static void configure_accelerators(const char *progname)
 static void qemu_validate_options(const QDict *machine_opts)
 {
     const char *kernel_filename = qdict_get_try_str(machine_opts, "kernel");
+    const char *shim_filename = qdict_get_try_str(machine_opts, "shim");
     const char *initrd_filename = qdict_get_try_str(machine_opts, "initrd");
     const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append");
 
@@ -2436,6 +2437,11 @@ static void qemu_validate_options(const QDict *machine_opts)
             exit(1);
         }
 
+        if (shim_filename != NULL) {
+            error_report("-shim only allowed with -kernel option");
+            exit(1);
+        }
+
         if (initrd_filename != NULL) {
             error_report("-initrd only allowed with -kernel option");
             exit(1);
@@ -2912,6 +2918,9 @@ void qemu_init(int argc, char **argv)
             case QEMU_OPTION_kernel:
                 qdict_put_str(machine_opts_dict, "kernel", optarg);
                 break;
+            case QEMU_OPTION_shim:
+                qdict_put_str(machine_opts_dict, "shim", optarg);
+                break;
             case QEMU_OPTION_initrd:
                 qdict_put_str(machine_opts_dict, "initrd", optarg);
                 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index dacc9790a4b8..cc694d3b890c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4145,6 +4145,13 @@ SRST
     or in multiboot format.
 ERST
 
+DEF("shim", HAS_ARG, QEMU_OPTION_shim, \
+    "-shim shim.efi use 'shim.efi' to boot the kernel\n", QEMU_ARCH_ALL)
+SRST
+``-shim shim.efi``
+    Use 'shim.efi' to boot the kernel
+ERST
+
 DEF("append", HAS_ARG, QEMU_OPTION_append, \
     "-append cmdline use 'cmdline' as kernel command line\n", QEMU_ARCH_ALL)
 SRST
-- 
2.47.1

[PULL 5/7] pc-bios: Add amd-sev-es to edk2 json

[Gerd Hoffmann]

From: "Pratik R. Sampat" <pratikrajesh.sampat@amd.com>

With the default BIOS being compatible with amd-sev-es add the feature
to the json to indicate it's support

Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
Message-ID: <20241118161405.208437-1-pratikrajesh.sampat@amd.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 pc-bios/descriptors/60-edk2-x86_64.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pc-bios/descriptors/60-edk2-x86_64.json b/pc-bios/descriptors/60-edk2-x86_64.json
index 968cb65cf973..4599c63f14ec 100644
--- a/pc-bios/descriptors/60-edk2-x86_64.json
+++ b/pc-bios/descriptors/60-edk2-x86_64.json
@@ -26,6 +26,7 @@
     "features": [
         "acpi-s3",
         "amd-sev",
+        "amd-sev-es",
         "verbose-dynamic"
     ],
     "tags": [
-- 
2.47.1

[PULL 6/7] pc-bios: add missing riscv64 descriptor

[Gerd Hoffmann]

From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Without descriptor libvirt cannot discover the EDK II binaries via
the qemu:///system connection.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Message-ID: <20241212090059.94167-1-heinrich.schuchardt@canonical.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 pc-bios/descriptors/60-edk2-riscv64.json | 31 ++++++++++++++++++++++++
 pc-bios/descriptors/meson.build          |  3 ++-
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/descriptors/60-edk2-riscv64.json

diff --git a/pc-bios/descriptors/60-edk2-riscv64.json b/pc-bios/descriptors/60-edk2-riscv64.json
new file mode 100644
index 000000000000..14811ca307f6
--- /dev/null
+++ b/pc-bios/descriptors/60-edk2-riscv64.json
@@ -0,0 +1,31 @@
+{
+    "description": "UEFI firmware for riscv64",
+    "interface-types": [
+        "uefi"
+    ],
+    "mapping": {
+        "device": "flash",
+        "executable": {
+            "filename": "@DATADIR@/edk2-riscv-code.fd",
+            "format": "raw"
+        },
+        "nvram-template": {
+            "filename": "@DATADIR@/edk2-riscv-vars.fd",
+            "format": "raw"
+        }
+    },
+    "targets": [
+        {
+            "architecture": "riscv64",
+            "machines": [
+                "virt*"
+            ]
+        }
+    ],
+    "features": [
+
+    ],
+    "tags": [
+
+    ]
+}
diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
index afb5a959ccf4..cdd0be01a356 100644
--- a/pc-bios/descriptors/meson.build
+++ b/pc-bios/descriptors/meson.build
@@ -6,7 +6,8 @@ if unpack_edk2_blobs and get_option('install_blobs')
     '60-edk2-arm.json',
     '60-edk2-i386.json',
     '60-edk2-x86_64.json',
-    '60-edk2-loongarch64.json'
+    '60-edk2-loongarch64.json',
+    '60-edk2-riscv64.json'
   ]
     configure_file(input: files(f),
                    output: f,
-- 
2.47.1

[PULL 7/7] roms: re-add edk2-basetools target

[Gerd Hoffmann]

Needed to build ipxe nic roms.

Reported-by: Liu Jaloo <liu.jaloo@gmail.com>
Fixes: 22e11539e167 ("edk2: replace build scripts")
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-ID: <20241212084408.1390728-1-kraxel@redhat.com>
---
 roms/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/roms/Makefile b/roms/Makefile
index dfed2b216a1e..31e4b97c983b 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -157,6 +157,11 @@ edk2-version: edk2
 		touch $@; \
 	fi
 
+edk2-basetools: edk2-version
+	$(PYTHON) edk2-build.py --config edk2-build.config \
+		--silent --no-logs \
+		--match none # build only basetools
+
 efi: edk2-version
 	$(PYTHON) edk2-build.py --config edk2-build.config \
 		--version-override "$(EDK2_STABLE)$(FIRMWARE_EXTRAVERSION)" \
-- 
2.47.1

Re: [PULL 0/7] Firmware 20241216 patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]