qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: qemu-devel@nongnu.org
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	"Xiaoyao Li" <xiaoyao.li@intel.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	"Laszlo Ersek" <lersek@redhat.com>,
	linux-efi@vger.kernel.org
Subject: [PATCH v3] hw/i386: place setup_data at fixed place in memory
Date: Fri,  5 Aug 2022 01:04:11 +0200	[thread overview]
Message-ID: <20220804230411.17720-1-Jason@zx2c4.com> (raw)
In-Reply-To: <YuxOgtykRQb1HU3e@zx2c4.com>

The boot parameter header refers to setup_data at an absolute address,
and each setup_data refers to the next setup_data at an absolute address
too. Currently QEMU simply puts the setup_datas right after the kernel
image, and since the kernel_image is loaded at prot_addr -- a fixed
address knowable to QEMU apriori -- the setup_data absolute address
winds up being just `prot_addr + a_fixed_offset_into_kernel_image`.

This mostly works fine, so long as the kernel image really is loaded at
prot_addr. However, OVMF doesn't load the kernel at prot_addr, and
generally EFI doesn't give a good way of predicting where it's going to
load the kernel. So when it loads it at some address != prot_addr, the
absolute addresses in setup_data now point somewhere bogus, causing
crashes when EFI stub tries to follow the next link.

Fix this by placing setup_data at some fixed place in memory, not as
part of the kernel image, and then pointing the setup_data absolute
address to that fixed place in memory. This way, even if OVMF or other
chains relocate the kernel image, the boot parameter still points to the
correct absolute address.

For this, an unused part of the hardware mapped area is used, which
isn't used by anything else.

Fixes: 3cbeb52467 ("hw/i386: add device tree support")
Reported-by: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: linux-efi@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/i386/x86.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 050eedc0c8..3affef3277 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -773,10 +773,10 @@ void x86_load_linux(X86MachineState *x86ms,
     bool linuxboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
     uint16_t protocol;
     int setup_size, kernel_size, cmdline_size;
-    int dtb_size, setup_data_offset;
+    int dtb_size, setup_data_item_len, setup_data_total_len = 0;
     uint32_t initrd_max;
-    uint8_t header[8192], *setup, *kernel;
-    hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0, first_setup_data = 0;
+    uint8_t header[8192], *setup, *kernel, *setup_datas = NULL;
+    hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0, first_setup_data = 0, setup_data_base;
     FILE *f;
     char *vmode;
     MachineState *machine = MACHINE(x86ms);
@@ -899,6 +899,8 @@ void x86_load_linux(X86MachineState *x86ms,
         cmdline_addr = 0x20000;
         prot_addr    = 0x100000;
     }
+    /* Nothing else uses this part of the hardware mapped region */
+    setup_data_base = 0xfffff - 0x1000;
 
     /* highest address for loading the initrd */
     if (protocol >= 0x20c &&
@@ -1062,34 +1064,35 @@ void x86_load_linux(X86MachineState *x86ms,
             exit(1);
         }
 
-        setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
-        kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
-        kernel = g_realloc(kernel, kernel_size);
-
-
-        setup_data = (struct setup_data *)(kernel + setup_data_offset);
+        setup_data_item_len = sizeof(struct setup_data) + dtb_size;
+        setup_datas = g_realloc(setup_datas, setup_data_total_len + setup_data_item_len);
+        setup_data = (struct setup_data *)(setup_datas + setup_data_total_len);
         setup_data->next = cpu_to_le64(first_setup_data);
-        first_setup_data = prot_addr + setup_data_offset;
+        first_setup_data = setup_data_base + setup_data_total_len;
+        setup_data_total_len += setup_data_item_len;
         setup_data->type = cpu_to_le32(SETUP_DTB);
         setup_data->len = cpu_to_le32(dtb_size);
-
         load_image_size(dtb_filename, setup_data->data, dtb_size);
     }
 
     if (!legacy_no_rng_seed) {
-        setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
-        kernel_size = setup_data_offset + sizeof(struct setup_data) + RNG_SEED_LENGTH;
-        kernel = g_realloc(kernel, kernel_size);
-        setup_data = (struct setup_data *)(kernel + setup_data_offset);
+        setup_data_item_len = sizeof(struct setup_data) + RNG_SEED_LENGTH;
+        setup_datas = g_realloc(setup_datas, setup_data_total_len + setup_data_item_len);
+        setup_data = (struct setup_data *)(setup_datas + setup_data_total_len);
         setup_data->next = cpu_to_le64(first_setup_data);
-        first_setup_data = prot_addr + setup_data_offset;
+        first_setup_data = setup_data_base + setup_data_total_len;
+        setup_data_total_len += setup_data_item_len;
         setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
         setup_data->len = cpu_to_le32(RNG_SEED_LENGTH);
         qemu_guest_getrandom_nofail(setup_data->data, RNG_SEED_LENGTH);
     }
 
-    /* Offset 0x250 is a pointer to the first setup_data link. */
-    stq_p(header + 0x250, first_setup_data);
+    if (first_setup_data && !sev_enabled()) {
+            /* Offset 0x250 is a pointer to the first setup_data link. */
+            stq_p(header + 0x250, first_setup_data);
+            rom_add_blob("setup_data", setup_datas, setup_data_total_len, setup_data_total_len,
+                         setup_data_base, NULL, NULL, NULL, NULL, false);
+    }
 
     /*
      * If we're starting an encrypted VM, it will be OVMF based, which uses the
-- 
2.35.1



  reply	other threads:[~2022-08-04 23:05 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03 17:02 [PATCH RFC v1] hw/i386: place setup_data at fixed place in memory Jason A. Donenfeld
2022-08-03 22:25 ` Michael S. Tsirkin
2022-08-03 22:50   ` Jason A. Donenfeld
2022-08-04  0:39     ` Jason A. Donenfeld
2022-08-04  0:44       ` [PATCH v2] " Jason A. Donenfeld
2022-08-04  7:03         ` Michael S. Tsirkin
2022-08-04  8:58           ` Laszlo Ersek
2022-08-04  9:25             ` Daniel P. Berrangé
2022-08-04 10:26               ` Ard Biesheuvel
2022-08-04 11:31                 ` Laszlo Ersek
2022-08-04 12:11                   ` Jason A. Donenfeld
2022-08-04 12:47                     ` Jason A. Donenfeld
2022-08-04 13:16                       ` Laszlo Ersek
2022-08-04 13:25                         ` Jason A. Donenfeld
2022-08-04 13:29                         ` Laszlo Ersek
2022-08-04 12:03               ` Jason A. Donenfeld
2022-08-04 12:11                 ` Daniel P. Berrangé
2022-08-04 12:16                   ` Ard Biesheuvel
2022-08-04 12:17                     ` Jason A. Donenfeld
2022-08-04 12:28                       ` Jason A. Donenfeld
2022-08-04 13:25                     ` Laszlo Ersek
2022-08-04 13:28                       ` Jason A. Donenfeld
2022-08-04 13:56                         ` Laszlo Ersek
2022-08-04 14:03                           ` Daniel P. Berrangé
2022-08-04 14:04                           ` Laszlo Ersek
2022-08-04 22:56                           ` Jason A. Donenfeld
2022-08-04 23:04                             ` Jason A. Donenfeld [this message]
2022-08-05  8:10                               ` [PATCH v3] " Paolo Bonzini
2022-08-05 11:08                                 ` Ard Biesheuvel
2022-08-05 17:29                                   ` Paolo Bonzini
2022-08-05 17:56                                     ` Ard Biesheuvel
2022-08-09  9:17                                     ` Michael S. Tsirkin
2022-08-09 14:19                                       ` Paolo Bonzini
2022-08-05 12:47                                 ` Jason A. Donenfeld
2022-08-05 13:34                                   ` Laszlo Ersek
2022-08-09 12:17                                   ` Jason A. Donenfeld
2022-08-09 14:07                                     ` Michael S. Tsirkin
2022-08-09 14:15                                       ` Daniel P. Berrangé
2022-08-05  6:26                             ` [PATCH v2] " Laszlo Ersek
2022-08-16  8:55                         ` Gerd Hoffmann
2022-08-18 15:38                           ` Jason A. Donenfeld
2022-08-19  6:40                             ` Gerd Hoffmann
2022-08-19  7:16                               ` Ard Biesheuvel
2022-08-04 12:54         ` Daniel P. Berrangé
2022-08-04 13:07           ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220804230411.17720-1-Jason@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=ardb@kernel.org \
    --cc=berrange@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).