* [PATCH RFC 1/7] Revert "x86: don't let decompressed kernel image clobber setup_data"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-14 16:36 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 2/7] Revert "x86: do not re-randomize RNG seed on snapshot load" Michael S. Tsirkin
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Sergio Lopez,
Marcel Apfelbaum, Eduardo Habkost
This reverts commit eac7a7791bb6d719233deed750034042318ffd56.
Fixes: eac7a7791b ("x86: don't let decompressed kernel image clobber setup_data")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/i386/microvm.h | 5 ++--
include/hw/nvram/fw_cfg.h | 9 -------
hw/i386/microvm.c | 15 ++++-------
hw/i386/x86.c | 52 ++++++++++++++++++---------------------
hw/nvram/fw_cfg.c | 9 -------
5 files changed, 31 insertions(+), 59 deletions(-)
diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index e8af61f194..fad97a891d 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -50,9 +50,8 @@
*/
/* Platform virtio definitions */
-#define VIRTIO_MMIO_BASE 0xfeb00000
-#define VIRTIO_CMDLINE_MAXLEN 64
-#define VIRTIO_CMDLINE_TOTAL_MAX_LEN ((VIRTIO_CMDLINE_MAXLEN + 1) * 16)
+#define VIRTIO_MMIO_BASE 0xfeb00000
+#define VIRTIO_CMDLINE_MAXLEN 64
#define GED_MMIO_BASE 0xfea00000
#define GED_MMIO_BASE_MEMHP (GED_MMIO_BASE + 0x100)
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 990dcdbb2e..2e503904dc 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -139,15 +139,6 @@ void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
void *data, size_t len,
bool read_only);
-/**
- * fw_cfg_read_bytes_ptr:
- * @s: fw_cfg device being modified
- * @key: selector key value for new fw_cfg item
- *
- * Reads an existing fw_cfg data pointer.
- */
-void *fw_cfg_read_bytes_ptr(FWCfgState *s, uint16_t key);
-
/**
* fw_cfg_add_string:
* @s: fw_cfg device being modified
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 29f30dd6d3..170a331e3f 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -378,8 +378,7 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
MicrovmMachineState *mms = MICROVM_MACHINE(machine);
BusState *bus;
BusChild *kid;
- char *cmdline, *existing_cmdline;
- size_t len;
+ char *cmdline;
/*
* Find MMIO transports with attached devices, and add them to the kernel
@@ -388,8 +387,7 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
* Yes, this is a hack, but one that heavily improves the UX without
* introducing any significant issues.
*/
- existing_cmdline = fw_cfg_read_bytes_ptr(x86ms->fw_cfg, FW_CFG_CMDLINE_DATA);
- cmdline = g_strdup(existing_cmdline);
+ cmdline = g_strdup(machine->kernel_cmdline);
bus = sysbus_get_default();
QTAILQ_FOREACH(kid, &bus->children, sibling) {
DeviceState *dev = kid->child;
@@ -413,12 +411,9 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
}
}
- len = strlen(cmdline);
- if (len > VIRTIO_CMDLINE_TOTAL_MAX_LEN + strlen(existing_cmdline)) {
- fprintf(stderr, "qemu: virtio mmio cmdline too large, skipping\n");
- } else {
- memcpy(existing_cmdline, cmdline, len + 1);
- }
+ fw_cfg_modify_i32(x86ms->fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(cmdline) + 1);
+ fw_cfg_modify_string(x86ms->fw_cfg, FW_CFG_CMDLINE_DATA, cmdline);
+
g_free(cmdline);
}
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index eaff4227bd..78cc131926 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -50,7 +50,6 @@
#include "hw/intc/i8259.h"
#include "hw/rtc/mc146818rtc.h"
#include "target/i386/sev.h"
-#include "hw/i386/microvm.h"
#include "hw/acpi/cpu_hotplug.h"
#include "hw/irq.h"
@@ -814,18 +813,12 @@ void x86_load_linux(X86MachineState *x86ms,
const char *kernel_filename = machine->kernel_filename;
const char *initrd_filename = machine->initrd_filename;
const char *dtb_filename = machine->dtb;
- char *kernel_cmdline;
+ const char *kernel_cmdline = machine->kernel_cmdline;
SevKernelLoaderContext sev_load_ctx = {};
enum { RNG_SEED_LENGTH = 32 };
- /*
- * Add the NUL terminator, some padding for the microvm cmdline fiddling
- * hack, and then align to 16 bytes as a paranoia measure
- */
- cmdline_size = (strlen(machine->kernel_cmdline) + 1 +
- VIRTIO_CMDLINE_TOTAL_MAX_LEN + 16) & ~15;
- /* Make a copy, since we might append arbitrary bytes to it later. */
- kernel_cmdline = g_strndup(machine->kernel_cmdline, cmdline_size);
+ /* Align to 16 bytes as a paranoia measure */
+ cmdline_size = (strlen(kernel_cmdline) + 16) & ~15;
/* load the kernel header */
f = fopen(kernel_filename, "rb");
@@ -966,6 +959,12 @@ void x86_load_linux(X86MachineState *x86ms,
initrd_max = x86ms->below_4g_mem_size - acpi_data_size - 1;
}
+ fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
+ fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline) + 1);
+ fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
+ sev_load_ctx.cmdline_data = (char *)kernel_cmdline;
+ sev_load_ctx.cmdline_size = strlen(kernel_cmdline) + 1;
+
if (protocol >= 0x202) {
stl_p(header + 0x228, cmdline_addr);
} else {
@@ -1092,24 +1091,27 @@ void x86_load_linux(X86MachineState *x86ms,
exit(1);
}
- setup_data_offset = cmdline_size;
- cmdline_size += sizeof(SetupData) + dtb_size;
- kernel_cmdline = g_realloc(kernel_cmdline, cmdline_size);
- setup_data = (void *)kernel_cmdline + setup_data_offset;
+ setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
+ kernel_size = setup_data_offset + sizeof(SetupData) + dtb_size;
+ kernel = g_realloc(kernel, kernel_size);
+
+
+ setup_data = (SetupData *)(kernel + setup_data_offset);
setup_data->next = cpu_to_le64(first_setup_data);
- first_setup_data = cmdline_addr + setup_data_offset;
+ first_setup_data = prot_addr + setup_data_offset;
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 && protocol >= 0x209) {
- setup_data_offset = cmdline_size;
- cmdline_size += sizeof(SetupData) + RNG_SEED_LENGTH;
- kernel_cmdline = g_realloc(kernel_cmdline, cmdline_size);
- setup_data = (void *)kernel_cmdline + setup_data_offset;
+ if (!legacy_no_rng_seed) {
+ setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
+ kernel_size = setup_data_offset + sizeof(SetupData) + RNG_SEED_LENGTH;
+ kernel = g_realloc(kernel, kernel_size);
+ setup_data = (SetupData *)(kernel + setup_data_offset);
setup_data->next = cpu_to_le64(first_setup_data);
- first_setup_data = cmdline_addr + setup_data_offset;
+ first_setup_data = prot_addr + setup_data_offset;
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);
@@ -1120,12 +1122,6 @@ void x86_load_linux(X86MachineState *x86ms,
fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
}
- fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
- fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, cmdline_size);
- fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline, cmdline_size);
- sev_load_ctx.cmdline_data = (char *)kernel_cmdline;
- sev_load_ctx.cmdline_size = cmdline_size;
-
fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
sev_load_ctx.kernel_data = (char *)kernel;
@@ -1138,7 +1134,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() && first_setup_data) {
+ if (!sev_enabled()) {
SetupDataFixup *fixup = g_malloc(sizeof(*fixup));
memcpy(setup, header, MIN(sizeof(header), setup_size));
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 432754eda4..a00881bc64 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -741,15 +741,6 @@ void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
fw_cfg_add_bytes_callback(s, key, NULL, NULL, NULL, data, len, true);
}
-void *fw_cfg_read_bytes_ptr(FWCfgState *s, uint16_t key)
-{
- int arch = !!(key & FW_CFG_ARCH_LOCAL);
-
- key &= FW_CFG_ENTRY_MASK;
- assert(key < fw_cfg_max_entry(s));
- return s->entries[arch][key].data;
-}
-
void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value)
{
size_t sz = strlen(value) + 1;
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 1/7] Revert "x86: don't let decompressed kernel image clobber setup_data"
2023-02-08 21:12 ` [PATCH RFC 1/7] Revert "x86: don't let decompressed kernel image clobber setup_data" Michael S. Tsirkin
@ 2023-02-14 16:36 ` Daniel P. Berrangé
0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Sergio Lopez,
Marcel Apfelbaum, Eduardo Habkost
On Wed, Feb 08, 2023 at 04:12:27PM -0500, Michael S. Tsirkin wrote:
> This reverts commit eac7a7791bb6d719233deed750034042318ffd56.
>
> Fixes: eac7a7791b ("x86: don't let decompressed kernel image clobber setup_data")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/hw/i386/microvm.h | 5 ++--
> include/hw/nvram/fw_cfg.h | 9 -------
> hw/i386/microvm.c | 15 ++++-------
> hw/i386/x86.c | 52 ++++++++++++++++++---------------------
> hw/nvram/fw_cfg.c | 9 -------
> 5 files changed, 31 insertions(+), 59 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 2/7] Revert "x86: do not re-randomize RNG seed on snapshot load"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
2023-02-08 21:12 ` [PATCH RFC 1/7] Revert "x86: don't let decompressed kernel image clobber setup_data" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-14 16:36 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 3/7] Revert "x86: re-initialize RNG seed when selecting kernel" Michael S. Tsirkin
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
This reverts commit 14b29fea742034186403914b4d013d0e83f19e78.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes: 14b29fea74 ("x86: do not re-randomize RNG seed on snapshot load")
---
hw/i386/x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 78cc131926..7984f65352 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1115,7 +1115,7 @@ void x86_load_linux(X86MachineState *x86ms,
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);
- qemu_register_reset_nosnapshotload(reset_rng_seed, setup_data);
+ qemu_register_reset(reset_rng_seed, setup_data);
fw_cfg_add_bytes_callback(fw_cfg, FW_CFG_KERNEL_DATA, reset_rng_seed, NULL,
setup_data, kernel, kernel_size, true);
} else {
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 2/7] Revert "x86: do not re-randomize RNG seed on snapshot load"
2023-02-08 21:12 ` [PATCH RFC 2/7] Revert "x86: do not re-randomize RNG seed on snapshot load" Michael S. Tsirkin
@ 2023-02-14 16:36 ` Daniel P. Berrangé
0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
On Wed, Feb 08, 2023 at 04:12:31PM -0500, Michael S. Tsirkin wrote:
> This reverts commit 14b29fea742034186403914b4d013d0e83f19e78.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Fixes: 14b29fea74 ("x86: do not re-randomize RNG seed on snapshot load")
> ---
> hw/i386/x86.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 3/7] Revert "x86: re-initialize RNG seed when selecting kernel"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
2023-02-08 21:12 ` [PATCH RFC 1/7] Revert "x86: don't let decompressed kernel image clobber setup_data" Michael S. Tsirkin
2023-02-08 21:12 ` [PATCH RFC 2/7] Revert "x86: do not re-randomize RNG seed on snapshot load" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-14 16:37 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 4/7] Revert "x86: reinitialize RNG seed on system reboot" Michael S. Tsirkin
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
This reverts commit cc63374a5a7c240b7d3be734ef589dabbefc7527.
Fixes: cc63374a5a ("x86: re-initialize RNG seed when selecting kernel")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/x86.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 7984f65352..e1a5f244a9 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1116,14 +1116,11 @@ void x86_load_linux(X86MachineState *x86ms,
setup_data->len = cpu_to_le32(RNG_SEED_LENGTH);
qemu_guest_getrandom_nofail(setup_data->data, RNG_SEED_LENGTH);
qemu_register_reset(reset_rng_seed, setup_data);
- fw_cfg_add_bytes_callback(fw_cfg, FW_CFG_KERNEL_DATA, reset_rng_seed, NULL,
- setup_data, kernel, kernel_size, true);
- } else {
- fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
}
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;
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 3/7] Revert "x86: re-initialize RNG seed when selecting kernel"
2023-02-08 21:12 ` [PATCH RFC 3/7] Revert "x86: re-initialize RNG seed when selecting kernel" Michael S. Tsirkin
@ 2023-02-14 16:37 ` Daniel P. Berrangé
0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
On Wed, Feb 08, 2023 at 04:12:37PM -0500, Michael S. Tsirkin wrote:
> This reverts commit cc63374a5a7c240b7d3be734ef589dabbefc7527.
>
> Fixes: cc63374a5a ("x86: re-initialize RNG seed when selecting kernel")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/x86.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 4/7] Revert "x86: reinitialize RNG seed on system reboot"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (2 preceding siblings ...)
2023-02-08 21:12 ` [PATCH RFC 3/7] Revert "x86: re-initialize RNG seed when selecting kernel" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-14 16:37 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct" Michael S. Tsirkin
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
This reverts commit 763a2828bf313ed55878b09759dc435355035f2e.
Fixes: 763a2828bf ("x86: reinitialize RNG seed on system reboot")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/x86.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index e1a5f244a9..32f37ab7c2 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -787,12 +787,6 @@ static void reset_setup_data(void *opaque)
stq_p(fixup->pos, fixup->orig_val);
}
-static void reset_rng_seed(void *opaque)
-{
- SetupData *setup_data = opaque;
- qemu_guest_getrandom_nofail(setup_data->data, le32_to_cpu(setup_data->len));
-}
-
void x86_load_linux(X86MachineState *x86ms,
FWCfgState *fw_cfg,
int acpi_data_size,
@@ -1115,7 +1109,6 @@ void x86_load_linux(X86MachineState *x86ms,
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);
- qemu_register_reset(reset_rng_seed, setup_data);
}
fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 4/7] Revert "x86: reinitialize RNG seed on system reboot"
2023-02-08 21:12 ` [PATCH RFC 4/7] Revert "x86: reinitialize RNG seed on system reboot" Michael S. Tsirkin
@ 2023-02-14 16:37 ` Daniel P. Berrangé
0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
On Wed, Feb 08, 2023 at 04:12:42PM -0500, Michael S. Tsirkin wrote:
> This reverts commit 763a2828bf313ed55878b09759dc435355035f2e.
>
> Fixes: 763a2828bf ("x86: reinitialize RNG seed on system reboot")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/x86.c | 7 -------
> 1 file changed, 7 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (3 preceding siblings ...)
2023-02-08 21:12 ` [PATCH RFC 4/7] Revert "x86: reinitialize RNG seed on system reboot" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-09 7:41 ` Michael S. Tsirkin
2023-02-14 16:37 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file" Michael S. Tsirkin
` (4 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
This reverts commit eebb38a5633a77f5fa79d6486d5b2fcf8fbe3c07.
Fixes: eebb38a563 ("x86: use typedef for SetupData struct")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/i386/x86.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 32f37ab7c2..76b12108b4 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -657,12 +657,12 @@ DeviceState *ioapic_init_secondary(GSIState *gsi_state)
return dev;
}
-typedef struct SetupData {
+struct setup_data {
uint64_t next;
uint32_t type;
uint32_t len;
uint8_t data[];
-} __attribute__((packed)) SetupData;
+} __attribute__((packed));
/*
@@ -803,7 +803,7 @@ void x86_load_linux(X86MachineState *x86ms,
FILE *f;
char *vmode;
MachineState *machine = MACHINE(x86ms);
- SetupData *setup_data;
+ struct setup_data *setup_data;
const char *kernel_filename = machine->kernel_filename;
const char *initrd_filename = machine->initrd_filename;
const char *dtb_filename = machine->dtb;
@@ -1086,11 +1086,11 @@ void x86_load_linux(X86MachineState *x86ms,
}
setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
- kernel_size = setup_data_offset + sizeof(SetupData) + dtb_size;
+ kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
kernel = g_realloc(kernel, kernel_size);
- setup_data = (SetupData *)(kernel + setup_data_offset);
+ setup_data = (struct setup_data *)(kernel + setup_data_offset);
setup_data->next = cpu_to_le64(first_setup_data);
first_setup_data = prot_addr + setup_data_offset;
setup_data->type = cpu_to_le32(SETUP_DTB);
@@ -1101,9 +1101,9 @@ void x86_load_linux(X86MachineState *x86ms,
if (!legacy_no_rng_seed) {
setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
- kernel_size = setup_data_offset + sizeof(SetupData) + RNG_SEED_LENGTH;
+ kernel_size = setup_data_offset + sizeof(struct setup_data) + RNG_SEED_LENGTH;
kernel = g_realloc(kernel, kernel_size);
- setup_data = (SetupData *)(kernel + setup_data_offset);
+ setup_data = (struct setup_data *)(kernel + setup_data_offset);
setup_data->next = cpu_to_le64(first_setup_data);
first_setup_data = prot_addr + setup_data_offset;
setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct"
2023-02-08 21:12 ` [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct" Michael S. Tsirkin
@ 2023-02-09 7:41 ` Michael S. Tsirkin
2023-02-14 16:37 ` Daniel P. Berrangé
1 sibling, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-09 7:41 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
On Wed, Feb 08, 2023 at 04:12:51PM -0500, Michael S. Tsirkin wrote:
> This reverts commit eebb38a5633a77f5fa79d6486d5b2fcf8fbe3c07.
>
> Fixes: eebb38a563 ("x86: use typedef for SetupData struct")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This one was actually good, I reverted so other reverts are clean.
Jason I would appreciate it if you can rebase this on top
of the revert.
> ---
> hw/i386/x86.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 32f37ab7c2..76b12108b4 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -657,12 +657,12 @@ DeviceState *ioapic_init_secondary(GSIState *gsi_state)
> return dev;
> }
>
> -typedef struct SetupData {
> +struct setup_data {
> uint64_t next;
> uint32_t type;
> uint32_t len;
> uint8_t data[];
> -} __attribute__((packed)) SetupData;
> +} __attribute__((packed));
>
>
> /*
> @@ -803,7 +803,7 @@ void x86_load_linux(X86MachineState *x86ms,
> FILE *f;
> char *vmode;
> MachineState *machine = MACHINE(x86ms);
> - SetupData *setup_data;
> + struct setup_data *setup_data;
> const char *kernel_filename = machine->kernel_filename;
> const char *initrd_filename = machine->initrd_filename;
> const char *dtb_filename = machine->dtb;
> @@ -1086,11 +1086,11 @@ void x86_load_linux(X86MachineState *x86ms,
> }
>
> setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
> - kernel_size = setup_data_offset + sizeof(SetupData) + dtb_size;
> + kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
> kernel = g_realloc(kernel, kernel_size);
>
>
> - setup_data = (SetupData *)(kernel + setup_data_offset);
> + setup_data = (struct setup_data *)(kernel + setup_data_offset);
> setup_data->next = cpu_to_le64(first_setup_data);
> first_setup_data = prot_addr + setup_data_offset;
> setup_data->type = cpu_to_le32(SETUP_DTB);
> @@ -1101,9 +1101,9 @@ void x86_load_linux(X86MachineState *x86ms,
>
> if (!legacy_no_rng_seed) {
> setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
> - kernel_size = setup_data_offset + sizeof(SetupData) + RNG_SEED_LENGTH;
> + kernel_size = setup_data_offset + sizeof(struct setup_data) + RNG_SEED_LENGTH;
> kernel = g_realloc(kernel, kernel_size);
> - setup_data = (SetupData *)(kernel + setup_data_offset);
> + setup_data = (struct setup_data *)(kernel + setup_data_offset);
> setup_data->next = cpu_to_le64(first_setup_data);
> first_setup_data = prot_addr + setup_data_offset;
> setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
> --
> MST
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct"
2023-02-08 21:12 ` [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct" Michael S. Tsirkin
2023-02-09 7:41 ` Michael S. Tsirkin
@ 2023-02-14 16:37 ` Daniel P. Berrangé
1 sibling, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Eduardo Habkost, Marcel Apfelbaum
On Wed, Feb 08, 2023 at 04:12:46PM -0500, Michael S. Tsirkin wrote:
> This reverts commit eebb38a5633a77f5fa79d6486d5b2fcf8fbe3c07.
>
> Fixes: eebb38a563 ("x86: use typedef for SetupData struct")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/x86.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (4 preceding siblings ...)
2023-02-08 21:12 ` [PATCH RFC 5/7] Revert "x86: use typedef for SetupData struct" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-09 15:52 ` Jason A. Donenfeld
2023-02-14 16:38 ` Daniel P. Berrangé
2023-02-08 21:12 ` [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry" Michael S. Tsirkin
` (3 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
This reverts commit e935b735085dfa61d8e6d276b6f9e7687796a3c7.
Fixes: e935b73508 ("x86: return modified setup_data only if read as memory, not as file")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/nvram/fw_cfg.h | 22 -------------------
hw/i386/x86.c | 46 +++++++++------------------------------
hw/nvram/fw_cfg.c | 12 +++++-----
3 files changed, 16 insertions(+), 64 deletions(-)
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 2e503904dc..c1f81a5f13 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -117,28 +117,6 @@ struct FWCfgMemState {
*/
void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
-/**
- * fw_cfg_add_bytes_callback:
- * @s: fw_cfg device being modified
- * @key: selector key value for new fw_cfg item
- * @select_cb: callback function when selecting
- * @write_cb: callback function after a write
- * @callback_opaque: argument to be passed into callback function
- * @data: pointer to start of item data
- * @len: size of item data
- * @read_only: is file read only
- *
- * Add a new fw_cfg item, available by selecting the given key, as a raw
- * "blob" of the given size. The data referenced by the starting pointer
- * is only linked, NOT copied, into the data structure of the fw_cfg device.
- */
-void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
- FWCfgCallback select_cb,
- FWCfgWriteCallback write_cb,
- void *callback_opaque,
- void *data, size_t len,
- bool read_only);
-
/**
* fw_cfg_add_string:
* @s: fw_cfg device being modified
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 76b12108b4..4831193c86 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -37,7 +37,6 @@
#include "sysemu/whpx.h"
#include "sysemu/numa.h"
#include "sysemu/replay.h"
-#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "sysemu/cpu-timers.h"
#include "sysemu/xen.h"
@@ -769,24 +768,6 @@ static bool load_elfboot(const char *kernel_filename,
return true;
}
-typedef struct SetupDataFixup {
- void *pos;
- hwaddr orig_val, new_val;
- uint32_t addr;
-} SetupDataFixup;
-
-static void fixup_setup_data(void *opaque)
-{
- SetupDataFixup *fixup = opaque;
- stq_p(fixup->pos, fixup->new_val);
-}
-
-static void reset_setup_data(void *opaque)
-{
- SetupDataFixup *fixup = opaque;
- stq_p(fixup->pos, fixup->orig_val);
-}
-
void x86_load_linux(X86MachineState *x86ms,
FWCfgState *fw_cfg,
int acpi_data_size,
@@ -1111,11 +1092,8 @@ void x86_load_linux(X86MachineState *x86ms,
qemu_guest_getrandom_nofail(setup_data->data, RNG_SEED_LENGTH);
}
- 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;
+ /* Offset 0x250 is a pointer to the first setup_data link. */
+ stq_p(header + 0x250, first_setup_data);
/*
* If we're starting an encrypted VM, it will be OVMF based, which uses the
@@ -1125,20 +1103,16 @@ void x86_load_linux(X86MachineState *x86ms,
* file the user passed in.
*/
if (!sev_enabled()) {
- SetupDataFixup *fixup = g_malloc(sizeof(*fixup));
-
memcpy(setup, header, MIN(sizeof(header), setup_size));
- /* Offset 0x250 is a pointer to the first setup_data link. */
- fixup->pos = setup + 0x250;
- fixup->orig_val = ldq_p(fixup->pos);
- fixup->new_val = first_setup_data;
- fixup->addr = cpu_to_le32(real_addr);
- fw_cfg_add_bytes_callback(fw_cfg, FW_CFG_SETUP_ADDR, fixup_setup_data, NULL,
- fixup, &fixup->addr, sizeof(fixup->addr), true);
- qemu_register_reset(reset_setup_data, fixup);
- } else {
- fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
}
+
+ 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_SETUP_ADDR, real_addr);
fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
sev_load_ctx.setup_data = (char *)setup;
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index a00881bc64..29a5bef1d5 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -693,12 +693,12 @@ static const VMStateDescription vmstate_fw_cfg = {
}
};
-void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
- FWCfgCallback select_cb,
- FWCfgWriteCallback write_cb,
- void *callback_opaque,
- void *data, size_t len,
- bool read_only)
+static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
+ FWCfgCallback select_cb,
+ FWCfgWriteCallback write_cb,
+ void *callback_opaque,
+ void *data, size_t len,
+ bool read_only)
{
int arch = !!(key & FW_CFG_ARCH_LOCAL);
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file"
2023-02-08 21:12 ` [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file" Michael S. Tsirkin
@ 2023-02-09 15:52 ` Jason A. Donenfeld
2023-02-10 11:31 ` Daniel P. Berrangé
2023-02-14 16:38 ` Daniel P. Berrangé
1 sibling, 1 reply; 25+ messages in thread
From: Jason A. Donenfeld @ 2023-02-09 15:52 UTC (permalink / raw)
Cc: qemu-devel, x86, linux-kernel, Dov Murik, Tom Lendacky,
Gerd Hoffmann, Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
On Wed, Feb 08, 2023 at 04:12:51PM -0500, Michael S. Tsirkin wrote:
> This reverts commit e935b735085dfa61d8e6d276b6f9e7687796a3c7.
>
> Fixes: e935b73508 ("x86: return modified setup_data only if read as memory, not as file")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/hw/nvram/fw_cfg.h | 22 -------------------
> hw/i386/x86.c | 46 +++++++++------------------------------
> hw/nvram/fw_cfg.c | 12 +++++-----
> 3 files changed, 16 insertions(+), 64 deletions(-)
>
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 2e503904dc..c1f81a5f13 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -117,28 +117,6 @@ struct FWCfgMemState {
> */
> void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
>
> -/**
> - * fw_cfg_add_bytes_callback:
> - * @s: fw_cfg device being modified
> - * @key: selector key value for new fw_cfg item
> - * @select_cb: callback function when selecting
> - * @write_cb: callback function after a write
> - * @callback_opaque: argument to be passed into callback function
> - * @data: pointer to start of item data
> - * @len: size of item data
> - * @read_only: is file read only
> - *
> - * Add a new fw_cfg item, available by selecting the given key, as a raw
> - * "blob" of the given size. The data referenced by the starting pointer
> - * is only linked, NOT copied, into the data structure of the fw_cfg device.
> - */
> -void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> - FWCfgCallback select_cb,
> - FWCfgWriteCallback write_cb,
> - void *callback_opaque,
> - void *data, size_t len,
> - bool read_only);
> -
> /**
> * fw_cfg_add_string:
> * @s: fw_cfg device being modified
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index a00881bc64..29a5bef1d5 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -693,12 +693,12 @@ static const VMStateDescription vmstate_fw_cfg = {
> }
> };
>
> -void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> - FWCfgCallback select_cb,
> - FWCfgWriteCallback write_cb,
> - void *callback_opaque,
> - void *data, size_t len,
> - bool read_only)
> +static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> + FWCfgCallback select_cb,
> + FWCfgWriteCallback write_cb,
> + void *callback_opaque,
> + void *data, size_t len,
> + bool read_only)
> {
> int arch = !!(key & FW_CFG_ARCH_LOCAL);
Could you leave these snippets in? This function is useful and will be
needed in the reprise.
Jason
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file"
2023-02-09 15:52 ` Jason A. Donenfeld
@ 2023-02-10 11:31 ` Daniel P. Berrangé
0 siblings, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-10 11:31 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: qemu-devel, x86, linux-kernel, Dov Murik, Tom Lendacky,
Gerd Hoffmann, Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
On Thu, Feb 09, 2023 at 04:52:32PM +0100, Jason A. Donenfeld wrote:
> On Wed, Feb 08, 2023 at 04:12:51PM -0500, Michael S. Tsirkin wrote:
> > This reverts commit e935b735085dfa61d8e6d276b6f9e7687796a3c7.
> >
> > Fixes: e935b73508 ("x86: return modified setup_data only if read as memory, not as file")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > include/hw/nvram/fw_cfg.h | 22 -------------------
> > hw/i386/x86.c | 46 +++++++++------------------------------
> > hw/nvram/fw_cfg.c | 12 +++++-----
> > 3 files changed, 16 insertions(+), 64 deletions(-)
> >
> > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> > index 2e503904dc..c1f81a5f13 100644
> > --- a/include/hw/nvram/fw_cfg.h
> > +++ b/include/hw/nvram/fw_cfg.h
> > @@ -117,28 +117,6 @@ struct FWCfgMemState {
> > */
> > void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
> >
> > -/**
> > - * fw_cfg_add_bytes_callback:
> > - * @s: fw_cfg device being modified
> > - * @key: selector key value for new fw_cfg item
> > - * @select_cb: callback function when selecting
> > - * @write_cb: callback function after a write
> > - * @callback_opaque: argument to be passed into callback function
> > - * @data: pointer to start of item data
> > - * @len: size of item data
> > - * @read_only: is file read only
> > - *
> > - * Add a new fw_cfg item, available by selecting the given key, as a raw
> > - * "blob" of the given size. The data referenced by the starting pointer
> > - * is only linked, NOT copied, into the data structure of the fw_cfg device.
> > - */
> > -void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> > - FWCfgCallback select_cb,
> > - FWCfgWriteCallback write_cb,
> > - void *callback_opaque,
> > - void *data, size_t len,
> > - bool read_only);
> > -
> > /**
> > * fw_cfg_add_string:
> > * @s: fw_cfg device being modified
> > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > index a00881bc64..29a5bef1d5 100644
> > --- a/hw/nvram/fw_cfg.c
> > +++ b/hw/nvram/fw_cfg.c
> > @@ -693,12 +693,12 @@ static const VMStateDescription vmstate_fw_cfg = {
> > }
> > };
> >
> > -void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> > - FWCfgCallback select_cb,
> > - FWCfgWriteCallback write_cb,
> > - void *callback_opaque,
> > - void *data, size_t len,
> > - bool read_only)
> > +static void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
> > + FWCfgCallback select_cb,
> > + FWCfgWriteCallback write_cb,
> > + void *callback_opaque,
> > + void *data, size_t len,
> > + bool read_only)
> > {
> > int arch = !!(key & FW_CFG_ARCH_LOCAL);
>
> Could you leave these snippets in? This function is useful and will be
> needed in the reprise.
IMHO it is better to do a full clean revert of the patches.
Switching this one function from static to public is trivial
enough that it is not burden to do in a new impl of the RNG
seed work.
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file"
2023-02-08 21:12 ` [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file" Michael S. Tsirkin
2023-02-09 15:52 ` Jason A. Donenfeld
@ 2023-02-14 16:38 ` Daniel P. Berrangé
1 sibling, 0 replies; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers,
Marcel Apfelbaum, Eduardo Habkost
On Wed, Feb 08, 2023 at 04:12:51PM -0500, Michael S. Tsirkin wrote:
> This reverts commit e935b735085dfa61d8e6d276b6f9e7687796a3c7.
>
> Fixes: e935b73508 ("x86: return modified setup_data only if read as memory, not as file")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/hw/nvram/fw_cfg.h | 22 -------------------
> hw/i386/x86.c | 46 +++++++++------------------------------
> hw/nvram/fw_cfg.c | 12 +++++-----
> 3 files changed, 16 insertions(+), 64 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry"
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (5 preceding siblings ...)
2023-02-08 21:12 ` [PATCH RFC 6/7] Revert "x86: return modified setup_data only if read as memory, not as file" Michael S. Tsirkin
@ 2023-02-08 21:12 ` Michael S. Tsirkin
2023-02-14 16:45 ` Daniel P. Berrangé
2023-02-09 6:03 ` [PATCH RFC 0/7] revert RNG seed mess Dov Murik
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-08 21:12 UTC (permalink / raw)
To: qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Sergio Lopez,
Marcel Apfelbaum, Eduardo Habkost
This reverts commit 67f7e426e53833a5db75b0d813e8d537b8a75bd2.
Fixes: 67f7e426e5 ("hw/i386: pass RNG seed via setup_data entry")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/i386/pc.h | 3 ---
include/hw/i386/x86.h | 3 +--
hw/i386/microvm.c | 2 +-
hw/i386/pc.c | 4 ++--
hw/i386/pc_piix.c | 2 --
hw/i386/pc_q35.c | 2 --
hw/i386/x86.c | 26 ++++----------------------
7 files changed, 8 insertions(+), 34 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 66e3d059ef..44b08554fa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -127,9 +127,6 @@ struct PCMachineClass {
/* create kvmclock device even when KVM PV features are not exposed */
bool kvmclock_create_always;
-
- /* skip passing an rng seed for legacy machines */
- bool legacy_no_rng_seed;
};
#define TYPE_PC_MACHINE "generic-pc-machine"
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 62fa5774f8..df82c5fd42 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -126,8 +126,7 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
void x86_load_linux(X86MachineState *x86ms,
FWCfgState *fw_cfg,
int acpi_data_size,
- bool pvh_enabled,
- bool legacy_no_rng_seed);
+ bool pvh_enabled);
bool x86_machine_is_smm_enabled(const X86MachineState *x86ms);
bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms);
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 170a331e3f..b231ceda9a 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -330,7 +330,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
rom_set_fw(fw_cfg);
if (machine->kernel_filename != NULL) {
- x86_load_linux(x86ms, fw_cfg, 0, true, false);
+ x86_load_linux(x86ms, fw_cfg, 0, true);
}
if (mms->option_roms) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e592bd969..2c5f675ba4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -804,7 +804,7 @@ void xen_load_linux(PCMachineState *pcms)
rom_set_fw(fw_cfg);
x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
- pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
+ pcmc->pvh_enabled);
for (i = 0; i < nb_option_roms; i++) {
assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
!strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
@@ -1124,7 +1124,7 @@ void pc_memory_init(PCMachineState *pcms,
if (linux_boot) {
x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
- pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
+ pcmc->pvh_enabled);
}
for (i = 0; i < nb_option_roms; i++) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..839bd65df5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -476,9 +476,7 @@ DEFINE_I440FX_MACHINE(v7_2, "pc-i440fx-7.2", NULL,
static void pc_i440fx_7_1_machine_options(MachineClass *m)
{
- PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_7_2_machine_options(m);
- pcmc->legacy_no_rng_seed = true;
compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 66cd718b70..e6e3966262 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -395,9 +395,7 @@ DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
static void pc_q35_7_1_machine_options(MachineClass *m)
{
- PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_7_2_machine_options(m);
- pcmc->legacy_no_rng_seed = true;
compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
}
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 4831193c86..80be3032cc 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -26,7 +26,6 @@
#include "qemu/cutils.h"
#include "qemu/units.h"
#include "qemu/datadir.h"
-#include "qemu/guest-random.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qapi-visit-common.h"
@@ -771,8 +770,7 @@ static bool load_elfboot(const char *kernel_filename,
void x86_load_linux(X86MachineState *x86ms,
FWCfgState *fw_cfg,
int acpi_data_size,
- bool pvh_enabled,
- bool legacy_no_rng_seed)
+ bool pvh_enabled)
{
bool linuxboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
uint16_t protocol;
@@ -780,7 +778,7 @@ void x86_load_linux(X86MachineState *x86ms,
int dtb_size, setup_data_offset;
uint32_t initrd_max;
uint8_t header[8192], *setup, *kernel;
- hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0, first_setup_data = 0;
+ hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
FILE *f;
char *vmode;
MachineState *machine = MACHINE(x86ms);
@@ -790,7 +788,6 @@ void x86_load_linux(X86MachineState *x86ms,
const char *dtb_filename = machine->dtb;
const char *kernel_cmdline = machine->kernel_cmdline;
SevKernelLoaderContext sev_load_ctx = {};
- enum { RNG_SEED_LENGTH = 32 };
/* Align to 16 bytes as a paranoia measure */
cmdline_size = (strlen(kernel_cmdline) + 16) & ~15;
@@ -1070,31 +1067,16 @@ void x86_load_linux(X86MachineState *x86ms,
kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
kernel = g_realloc(kernel, kernel_size);
+ stq_p(header + 0x250, prot_addr + setup_data_offset);
setup_data = (struct setup_data *)(kernel + setup_data_offset);
- setup_data->next = cpu_to_le64(first_setup_data);
- first_setup_data = prot_addr + setup_data_offset;
+ setup_data->next = 0;
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->next = cpu_to_le64(first_setup_data);
- first_setup_data = prot_addr + setup_data_offset;
- 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 we're starting an encrypted VM, it will be OVMF based, which uses the
* efi stub for booting and doesn't require any values to be placed in the
--
MST
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry"
2023-02-08 21:12 ` [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry" Michael S. Tsirkin
@ 2023-02-14 16:45 ` Daniel P. Berrangé
2023-02-14 18:03 ` Michael S. Tsirkin
0 siblings, 1 reply; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-14 16:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Sergio Lopez,
Marcel Apfelbaum, Eduardo Habkost
On Wed, Feb 08, 2023 at 04:12:56PM -0500, Michael S. Tsirkin wrote:
> This reverts commit 67f7e426e53833a5db75b0d813e8d537b8a75bd2.
>
> Fixes: 67f7e426e5 ("hw/i386: pass RNG seed via setup_data entry")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/hw/i386/pc.h | 3 ---
> include/hw/i386/x86.h | 3 +--
> hw/i386/microvm.c | 2 +-
> hw/i386/pc.c | 4 ++--
> hw/i386/pc_piix.c | 2 --
> hw/i386/pc_q35.c | 2 --
> hw/i386/x86.c | 26 ++++----------------------
> 7 files changed, 8 insertions(+), 34 deletions(-)
All the patches prior to this were clean reverts. This one though
does not look clean, rather it looks like reverting a combination
of 3 commits
commit ffe2d2382e5f1aae1abc4081af407905ef380311
Author: Jason A. Donenfeld <Jason@zx2c4.com>
Date: Wed Sep 21 11:31:34 2022 +0200
x86: re-enable rng seeding via SetupData
commit 3824e25db1a84fadc50b88dfbe27047aa2f7f85d
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed Aug 17 10:39:40 2022 +0200
x86: disable rng seeding via setup_data
commit 67f7e426e53833a5db75b0d813e8d537b8a75bd2
Author: Jason A. Donenfeld <Jason@zx2c4.com>
Date: Thu Jul 21 14:56:36 2022 +0200
hw/i386: pass RNG seed via setup_data entry
though, even taking the individual commits isn't a perfectly
clean revert due to other unrelated changes in machine
type versioning.
I'm not too fussed if this last one isn't a clean revert,
but lets just call out in the commit message that we're
just manually resolving conflicts and skipping those
two intermediate commits for simplicity.
Assuming an improve commit message:
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 66e3d059ef..44b08554fa 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -127,9 +127,6 @@ struct PCMachineClass {
>
> /* create kvmclock device even when KVM PV features are not exposed */
> bool kvmclock_create_always;
> -
> - /* skip passing an rng seed for legacy machines */
> - bool legacy_no_rng_seed;
> };
>
> #define TYPE_PC_MACHINE "generic-pc-machine"
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 62fa5774f8..df82c5fd42 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -126,8 +126,7 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
> void x86_load_linux(X86MachineState *x86ms,
> FWCfgState *fw_cfg,
> int acpi_data_size,
> - bool pvh_enabled,
> - bool legacy_no_rng_seed);
> + bool pvh_enabled);
>
> bool x86_machine_is_smm_enabled(const X86MachineState *x86ms);
> bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms);
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 170a331e3f..b231ceda9a 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -330,7 +330,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
> rom_set_fw(fw_cfg);
>
> if (machine->kernel_filename != NULL) {
> - x86_load_linux(x86ms, fw_cfg, 0, true, false);
> + x86_load_linux(x86ms, fw_cfg, 0, true);
> }
>
> if (mms->option_roms) {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6e592bd969..2c5f675ba4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -804,7 +804,7 @@ void xen_load_linux(PCMachineState *pcms)
> rom_set_fw(fw_cfg);
>
> x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> - pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
> + pcmc->pvh_enabled);
> for (i = 0; i < nb_option_roms; i++) {
> assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
> !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
> @@ -1124,7 +1124,7 @@ void pc_memory_init(PCMachineState *pcms,
>
> if (linux_boot) {
> x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> - pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
> + pcmc->pvh_enabled);
> }
>
> for (i = 0; i < nb_option_roms; i++) {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index df64dd8dcc..839bd65df5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -476,9 +476,7 @@ DEFINE_I440FX_MACHINE(v7_2, "pc-i440fx-7.2", NULL,
>
> static void pc_i440fx_7_1_machine_options(MachineClass *m)
> {
> - PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> pc_i440fx_7_2_machine_options(m);
> - pcmc->legacy_no_rng_seed = true;
> compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
> compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
> }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 66cd718b70..e6e3966262 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -395,9 +395,7 @@ DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
>
> static void pc_q35_7_1_machine_options(MachineClass *m)
> {
> - PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> pc_q35_7_2_machine_options(m);
> - pcmc->legacy_no_rng_seed = true;
> compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
> compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
> }
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 4831193c86..80be3032cc 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -26,7 +26,6 @@
> #include "qemu/cutils.h"
> #include "qemu/units.h"
> #include "qemu/datadir.h"
> -#include "qemu/guest-random.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qerror.h"
> #include "qapi/qapi-visit-common.h"
> @@ -771,8 +770,7 @@ static bool load_elfboot(const char *kernel_filename,
> void x86_load_linux(X86MachineState *x86ms,
> FWCfgState *fw_cfg,
> int acpi_data_size,
> - bool pvh_enabled,
> - bool legacy_no_rng_seed)
> + bool pvh_enabled)
> {
> bool linuxboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
> uint16_t protocol;
> @@ -780,7 +778,7 @@ void x86_load_linux(X86MachineState *x86ms,
> int dtb_size, setup_data_offset;
> uint32_t initrd_max;
> uint8_t header[8192], *setup, *kernel;
> - hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0, first_setup_data = 0;
> + hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> FILE *f;
> char *vmode;
> MachineState *machine = MACHINE(x86ms);
> @@ -790,7 +788,6 @@ void x86_load_linux(X86MachineState *x86ms,
> const char *dtb_filename = machine->dtb;
> const char *kernel_cmdline = machine->kernel_cmdline;
> SevKernelLoaderContext sev_load_ctx = {};
> - enum { RNG_SEED_LENGTH = 32 };
>
> /* Align to 16 bytes as a paranoia measure */
> cmdline_size = (strlen(kernel_cmdline) + 16) & ~15;
> @@ -1070,31 +1067,16 @@ void x86_load_linux(X86MachineState *x86ms,
> kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
> kernel = g_realloc(kernel, kernel_size);
>
> + stq_p(header + 0x250, prot_addr + setup_data_offset);
>
> setup_data = (struct setup_data *)(kernel + setup_data_offset);
> - setup_data->next = cpu_to_le64(first_setup_data);
> - first_setup_data = prot_addr + setup_data_offset;
> + setup_data->next = 0;
> 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->next = cpu_to_le64(first_setup_data);
> - first_setup_data = prot_addr + setup_data_offset;
> - 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 we're starting an encrypted VM, it will be OVMF based, which uses the
> * efi stub for booting and doesn't require any values to be placed in the
> --
> MST
>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry"
2023-02-14 16:45 ` Daniel P. Berrangé
@ 2023-02-14 18:03 ` Michael S. Tsirkin
0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-14 18:03 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Sergio Lopez,
Marcel Apfelbaum, Eduardo Habkost
On Tue, Feb 14, 2023 at 04:45:57PM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 08, 2023 at 04:12:56PM -0500, Michael S. Tsirkin wrote:
> > This reverts commit 67f7e426e53833a5db75b0d813e8d537b8a75bd2.
> >
> > Fixes: 67f7e426e5 ("hw/i386: pass RNG seed via setup_data entry")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > include/hw/i386/pc.h | 3 ---
> > include/hw/i386/x86.h | 3 +--
> > hw/i386/microvm.c | 2 +-
> > hw/i386/pc.c | 4 ++--
> > hw/i386/pc_piix.c | 2 --
> > hw/i386/pc_q35.c | 2 --
> > hw/i386/x86.c | 26 ++++----------------------
> > 7 files changed, 8 insertions(+), 34 deletions(-)
>
> All the patches prior to this were clean reverts. This one though
> does not look clean, rather it looks like reverting a combination
> of 3 commits
>
> commit ffe2d2382e5f1aae1abc4081af407905ef380311
> Author: Jason A. Donenfeld <Jason@zx2c4.com>
> Date: Wed Sep 21 11:31:34 2022 +0200
>
> x86: re-enable rng seeding via SetupData
>
> commit 3824e25db1a84fadc50b88dfbe27047aa2f7f85d
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date: Wed Aug 17 10:39:40 2022 +0200
>
> x86: disable rng seeding via setup_data
>
> commit 67f7e426e53833a5db75b0d813e8d537b8a75bd2
> Author: Jason A. Donenfeld <Jason@zx2c4.com>
> Date: Thu Jul 21 14:56:36 2022 +0200
>
> hw/i386: pass RNG seed via setup_data entry
>
>
>
> though, even taking the individual commits isn't a perfectly
> clean revert due to other unrelated changes in machine
> type versioning.
>
> I'm not too fussed if this last one isn't a clean revert,
> but lets just call out in the commit message that we're
> just manually resolving conflicts and skipping those
> two intermediate commits for simplicity.
>
> Assuming an improve commit message:
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Yes - what I did as part of revert is drop all mentions of
legacy_no_rng_seed manually.
I will clarify that, thanks!
>
>
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 66e3d059ef..44b08554fa 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -127,9 +127,6 @@ struct PCMachineClass {
> >
> > /* create kvmclock device even when KVM PV features are not exposed */
> > bool kvmclock_create_always;
> > -
> > - /* skip passing an rng seed for legacy machines */
> > - bool legacy_no_rng_seed;
> > };
> >
> > #define TYPE_PC_MACHINE "generic-pc-machine"
> > diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> > index 62fa5774f8..df82c5fd42 100644
> > --- a/include/hw/i386/x86.h
> > +++ b/include/hw/i386/x86.h
> > @@ -126,8 +126,7 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
> > void x86_load_linux(X86MachineState *x86ms,
> > FWCfgState *fw_cfg,
> > int acpi_data_size,
> > - bool pvh_enabled,
> > - bool legacy_no_rng_seed);
> > + bool pvh_enabled);
> >
> > bool x86_machine_is_smm_enabled(const X86MachineState *x86ms);
> > bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms);
> > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> > index 170a331e3f..b231ceda9a 100644
> > --- a/hw/i386/microvm.c
> > +++ b/hw/i386/microvm.c
> > @@ -330,7 +330,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
> > rom_set_fw(fw_cfg);
> >
> > if (machine->kernel_filename != NULL) {
> > - x86_load_linux(x86ms, fw_cfg, 0, true, false);
> > + x86_load_linux(x86ms, fw_cfg, 0, true);
> > }
> >
> > if (mms->option_roms) {
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 6e592bd969..2c5f675ba4 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -804,7 +804,7 @@ void xen_load_linux(PCMachineState *pcms)
> > rom_set_fw(fw_cfg);
> >
> > x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> > - pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
> > + pcmc->pvh_enabled);
> > for (i = 0; i < nb_option_roms; i++) {
> > assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
> > !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
> > @@ -1124,7 +1124,7 @@ void pc_memory_init(PCMachineState *pcms,
> >
> > if (linux_boot) {
> > x86_load_linux(x86ms, fw_cfg, pcmc->acpi_data_size,
> > - pcmc->pvh_enabled, pcmc->legacy_no_rng_seed);
> > + pcmc->pvh_enabled);
> > }
> >
> > for (i = 0; i < nb_option_roms; i++) {
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index df64dd8dcc..839bd65df5 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -476,9 +476,7 @@ DEFINE_I440FX_MACHINE(v7_2, "pc-i440fx-7.2", NULL,
> >
> > static void pc_i440fx_7_1_machine_options(MachineClass *m)
> > {
> > - PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > pc_i440fx_7_2_machine_options(m);
> > - pcmc->legacy_no_rng_seed = true;
> > compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
> > compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
> > }
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 66cd718b70..e6e3966262 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -395,9 +395,7 @@ DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
> >
> > static void pc_q35_7_1_machine_options(MachineClass *m)
> > {
> > - PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > pc_q35_7_2_machine_options(m);
> > - pcmc->legacy_no_rng_seed = true;
> > compat_props_add(m->compat_props, hw_compat_7_1, hw_compat_7_1_len);
> > compat_props_add(m->compat_props, pc_compat_7_1, pc_compat_7_1_len);
> > }
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index 4831193c86..80be3032cc 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -26,7 +26,6 @@
> > #include "qemu/cutils.h"
> > #include "qemu/units.h"
> > #include "qemu/datadir.h"
> > -#include "qemu/guest-random.h"
> > #include "qapi/error.h"
> > #include "qapi/qmp/qerror.h"
> > #include "qapi/qapi-visit-common.h"
> > @@ -771,8 +770,7 @@ static bool load_elfboot(const char *kernel_filename,
> > void x86_load_linux(X86MachineState *x86ms,
> > FWCfgState *fw_cfg,
> > int acpi_data_size,
> > - bool pvh_enabled,
> > - bool legacy_no_rng_seed)
> > + bool pvh_enabled)
> > {
> > bool linuxboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
> > uint16_t protocol;
> > @@ -780,7 +778,7 @@ void x86_load_linux(X86MachineState *x86ms,
> > int dtb_size, setup_data_offset;
> > uint32_t initrd_max;
> > uint8_t header[8192], *setup, *kernel;
> > - hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0, first_setup_data = 0;
> > + hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> > FILE *f;
> > char *vmode;
> > MachineState *machine = MACHINE(x86ms);
> > @@ -790,7 +788,6 @@ void x86_load_linux(X86MachineState *x86ms,
> > const char *dtb_filename = machine->dtb;
> > const char *kernel_cmdline = machine->kernel_cmdline;
> > SevKernelLoaderContext sev_load_ctx = {};
> > - enum { RNG_SEED_LENGTH = 32 };
> >
> > /* Align to 16 bytes as a paranoia measure */
> > cmdline_size = (strlen(kernel_cmdline) + 16) & ~15;
> > @@ -1070,31 +1067,16 @@ void x86_load_linux(X86MachineState *x86ms,
> > kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
> > kernel = g_realloc(kernel, kernel_size);
> >
> > + stq_p(header + 0x250, prot_addr + setup_data_offset);
> >
> > setup_data = (struct setup_data *)(kernel + setup_data_offset);
> > - setup_data->next = cpu_to_le64(first_setup_data);
> > - first_setup_data = prot_addr + setup_data_offset;
> > + setup_data->next = 0;
> > 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->next = cpu_to_le64(first_setup_data);
> > - first_setup_data = prot_addr + setup_data_offset;
> > - 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 we're starting an encrypted VM, it will be OVMF based, which uses the
> > * efi stub for booting and doesn't require any values to be placed in the
> > --
> > MST
> >
>
> 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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (6 preceding siblings ...)
2023-02-08 21:12 ` [PATCH RFC 7/7] Revert "hw/i386: pass RNG seed via setup_data entry" Michael S. Tsirkin
@ 2023-02-09 6:03 ` Dov Murik
2023-02-09 6:34 ` Dov Murik
2023-02-09 15:46 ` Nathan Chancellor
2023-02-10 11:32 ` Daniel P. Berrangé
9 siblings, 1 reply; 25+ messages in thread
From: Dov Murik @ 2023-02-09 6:03 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Tom Lendacky, Gerd Hoffmann,
Daniel P. Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Dov Murik
Hi Michael,
On 08/02/2023 23:12, Michael S. Tsirkin wrote:
> All attempts to fix up passing RNG seed via setup_data entry failed.
> Let's just rip out all of it. We'll start over.
>
>
> Warning: all I did was git revert the relevant patches and resolve the
> (trivial) conflicts. Not even compiled - it's almost midnight here.
>
> Jason this is the kind of approach I'd like to see, not yet another
> pointer math rich patch I need to spend time reviewing. Just get us back
> to where we started. We can redo "x86: use typedef for SetupData struct"
> later if we want, it's benign.
>
> Could you do something like this pls?
> Or test and ack if this patchset happens to work by luck.
>
> Michael S. Tsirkin (7):
> Revert "x86: don't let decompressed kernel image clobber setup_data"
> Revert "x86: do not re-randomize RNG seed on snapshot load"
> Revert "x86: re-initialize RNG seed when selecting kernel"
> Revert "x86: reinitialize RNG seed on system reboot"
> Revert "x86: use typedef for SetupData struct"
> Revert "x86: return modified setup_data only if read as memory, not as
> file"
> Revert "hw/i386: pass RNG seed via setup_data entry"
>
> include/hw/i386/microvm.h | 5 +-
> include/hw/i386/pc.h | 3 -
> include/hw/i386/x86.h | 3 +-
> include/hw/nvram/fw_cfg.h | 31 ----------
> hw/i386/microvm.c | 17 ++----
> hw/i386/pc.c | 4 +-
> hw/i386/pc_piix.c | 2 -
> hw/i386/pc_q35.c | 2 -
> hw/i386/x86.c | 122 ++++++++++----------------------------
> hw/nvram/fw_cfg.c | 21 ++-----
> 10 files changed, 49 insertions(+), 161 deletions(-)
>
I tested this series with SEV measured boot using AmdSev OVMF. The boot
succeeds, and the hashes computed for kernel/initrd/cmdline are
identical to the ones computed by qemu 7.1.0, which are the hashes that
the guest owner would expect.
As for non-EFI (non-SEV) guests, I did a very simple test of starting a
non-EFI guest and it looks OK, but more testing is needed.
So:
Tested-by: Dov Murik <dovmurik@linux.ibm.com>
Thank you!
-Dov
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-09 6:03 ` [PATCH RFC 0/7] revert RNG seed mess Dov Murik
@ 2023-02-09 6:34 ` Dov Murik
0 siblings, 0 replies; 25+ messages in thread
From: Dov Murik @ 2023-02-09 6:34 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel, Jason A. Donenfeld
Cc: x86, linux-kernel, Tom Lendacky, Gerd Hoffmann,
Daniel P. Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers, Dov Murik
On 09/02/2023 8:03, Dov Murik wrote:
> Hi Michael,
>
> On 08/02/2023 23:12, Michael S. Tsirkin wrote:
>> All attempts to fix up passing RNG seed via setup_data entry failed.
>> Let's just rip out all of it. We'll start over.
>>
>>
>> Warning: all I did was git revert the relevant patches and resolve the
>> (trivial) conflicts. Not even compiled - it's almost midnight here.
>>
>> Jason this is the kind of approach I'd like to see, not yet another
>> pointer math rich patch I need to spend time reviewing. Just get us back
>> to where we started. We can redo "x86: use typedef for SetupData struct"
>> later if we want, it's benign.
>>
>> Could you do something like this pls?
>> Or test and ack if this patchset happens to work by luck.
>>
>> Michael S. Tsirkin (7):
>> Revert "x86: don't let decompressed kernel image clobber setup_data"
>> Revert "x86: do not re-randomize RNG seed on snapshot load"
>> Revert "x86: re-initialize RNG seed when selecting kernel"
>> Revert "x86: reinitialize RNG seed on system reboot"
>> Revert "x86: use typedef for SetupData struct"
>> Revert "x86: return modified setup_data only if read as memory, not as
>> file"
>> Revert "hw/i386: pass RNG seed via setup_data entry"
>>
>> include/hw/i386/microvm.h | 5 +-
>> include/hw/i386/pc.h | 3 -
>> include/hw/i386/x86.h | 3 +-
>> include/hw/nvram/fw_cfg.h | 31 ----------
>> hw/i386/microvm.c | 17 ++----
>> hw/i386/pc.c | 4 +-
>> hw/i386/pc_piix.c | 2 -
>> hw/i386/pc_q35.c | 2 -
>> hw/i386/x86.c | 122 ++++++++++----------------------------
>> hw/nvram/fw_cfg.c | 21 ++-----
>> 10 files changed, 49 insertions(+), 161 deletions(-)
>>
>
>
> I tested this series with SEV measured boot using AmdSev OVMF. The boot
> succeeds, and the hashes computed for kernel/initrd/cmdline are
> identical to the ones computed by qemu 7.1.0, which are the hashes that
> the guest owner would expect.
>
I also tested that backporting this series to 7.2.0 (skipping
PATCH 1/7 because it was added after the 7.2.0 release) works OK for
booting SEV guest with AmdSev measured boot (and retains the same
kernel/initrd/cmdline hashes as qemu 7.1.0).
-Dov
> As for non-EFI (non-SEV) guests, I did a very simple test of starting a
> non-EFI guest and it looks OK, but more testing is needed.
>
> So:
>
> Tested-by: Dov Murik <dovmurik@linux.ibm.com>
>
>
> Thank you!
>
> -Dov
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (7 preceding siblings ...)
2023-02-09 6:03 ` [PATCH RFC 0/7] revert RNG seed mess Dov Murik
@ 2023-02-09 15:46 ` Nathan Chancellor
2023-02-10 11:32 ` Daniel P. Berrangé
9 siblings, 0 replies; 25+ messages in thread
From: Nathan Chancellor @ 2023-02-09 15:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Daniel P . Berrangé,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Borislav Petkov, Eric Biggers
On Wed, Feb 08, 2023 at 04:12:23PM -0500, Michael S. Tsirkin wrote:
> All attempts to fix up passing RNG seed via setup_data entry failed.
> Let's just rip out all of it. We'll start over.
>
>
> Warning: all I did was git revert the relevant patches and resolve the
> (trivial) conflicts. Not even compiled - it's almost midnight here.
>
> Jason this is the kind of approach I'd like to see, not yet another
> pointer math rich patch I need to spend time reviewing. Just get us back
> to where we started. We can redo "x86: use typedef for SetupData struct"
> later if we want, it's benign.
>
> Could you do something like this pls?
> Or test and ack if this patchset happens to work by luck.
>
> Michael S. Tsirkin (7):
> Revert "x86: don't let decompressed kernel image clobber setup_data"
> Revert "x86: do not re-randomize RNG seed on snapshot load"
> Revert "x86: re-initialize RNG seed when selecting kernel"
> Revert "x86: reinitialize RNG seed on system reboot"
> Revert "x86: use typedef for SetupData struct"
> Revert "x86: return modified setup_data only if read as memory, not as
> file"
> Revert "hw/i386: pass RNG seed via setup_data entry"
>
> include/hw/i386/microvm.h | 5 +-
> include/hw/i386/pc.h | 3 -
> include/hw/i386/x86.h | 3 +-
> include/hw/nvram/fw_cfg.h | 31 ----------
> hw/i386/microvm.c | 17 ++----
> hw/i386/pc.c | 4 +-
> hw/i386/pc_piix.c | 2 -
> hw/i386/pc_q35.c | 2 -
> hw/i386/x86.c | 122 ++++++++++----------------------------
> hw/nvram/fw_cfg.c | 21 ++-----
> 10 files changed, 49 insertions(+), 161 deletions(-)
>
> --
> MST
>
For the record, all three of the cases that I tested (i386 no EFI,
x86_64 with and without EFI) worked fine with this series. In case it is
useful:
Tested-by: Nathan Chancellor <nathan@kernel.org>
Cheers,
Nathan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-08 21:12 [PATCH RFC 0/7] revert RNG seed mess Michael S. Tsirkin
` (8 preceding siblings ...)
2023-02-09 15:46 ` Nathan Chancellor
@ 2023-02-10 11:32 ` Daniel P. Berrangé
2023-02-20 10:48 ` Daniel P. Berrangé
9 siblings, 1 reply; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-10 11:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers
On Wed, Feb 08, 2023 at 04:12:23PM -0500, Michael S. Tsirkin wrote:
> All attempts to fix up passing RNG seed via setup_data entry failed.
> Let's just rip out all of it. We'll start over.
>
>
> Warning: all I did was git revert the relevant patches and resolve the
> (trivial) conflicts. Not even compiled - it's almost midnight here.
>
> Jason this is the kind of approach I'd like to see, not yet another
> pointer math rich patch I need to spend time reviewing. Just get us back
> to where we started. We can redo "x86: use typedef for SetupData struct"
> later if we want, it's benign.
This approach looks suitable for applying to the 7.2 tree too,
which will be good for fixing the regressions in stable.
>
> Could you do something like this pls?
> Or test and ack if this patchset happens to work by luck.
>
> Michael S. Tsirkin (7):
> Revert "x86: don't let decompressed kernel image clobber setup_data"
> Revert "x86: do not re-randomize RNG seed on snapshot load"
> Revert "x86: re-initialize RNG seed when selecting kernel"
> Revert "x86: reinitialize RNG seed on system reboot"
> Revert "x86: use typedef for SetupData struct"
> Revert "x86: return modified setup_data only if read as memory, not as
> file"
> Revert "hw/i386: pass RNG seed via setup_data entry"
>
> include/hw/i386/microvm.h | 5 +-
> include/hw/i386/pc.h | 3 -
> include/hw/i386/x86.h | 3 +-
> include/hw/nvram/fw_cfg.h | 31 ----------
> hw/i386/microvm.c | 17 ++----
> hw/i386/pc.c | 4 +-
> hw/i386/pc_piix.c | 2 -
> hw/i386/pc_q35.c | 2 -
> hw/i386/x86.c | 122 ++++++++++----------------------------
> hw/nvram/fw_cfg.c | 21 ++-----
> 10 files changed, 49 insertions(+), 161 deletions(-)
>
> --
> MST
>
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-10 11:32 ` Daniel P. Berrangé
@ 2023-02-20 10:48 ` Daniel P. Berrangé
2023-02-20 11:54 ` Michael S. Tsirkin
0 siblings, 1 reply; 25+ messages in thread
From: Daniel P. Berrangé @ 2023-02-20 10:48 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel, Jason A. Donenfeld, x86,
linux-kernel, Dov Murik, Tom Lendacky, Gerd Hoffmann,
Paolo Bonzini, Richard Henderson, H . Peter Anvin,
Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers
On Fri, Feb 10, 2023 at 11:32:58AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 08, 2023 at 04:12:23PM -0500, Michael S. Tsirkin wrote:
> > All attempts to fix up passing RNG seed via setup_data entry failed.
> > Let's just rip out all of it. We'll start over.
> >
> >
> > Warning: all I did was git revert the relevant patches and resolve the
> > (trivial) conflicts. Not even compiled - it's almost midnight here.
> >
> > Jason this is the kind of approach I'd like to see, not yet another
> > pointer math rich patch I need to spend time reviewing. Just get us back
> > to where we started. We can redo "x86: use typedef for SetupData struct"
> > later if we want, it's benign.
>
> This approach looks suitable for applying to the 7.2 tree too,
> which will be good for fixing the regressions in stable.
Since no further alternative has been proposed, can you consider sending
a pull request for this series. This has been broken for too long and
many users & vendors are looking for an official fix to be applied to
master before they backport to 7.2
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 :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/7] revert RNG seed mess
2023-02-20 10:48 ` Daniel P. Berrangé
@ 2023-02-20 11:54 ` Michael S. Tsirkin
0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2023-02-20 11:54 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Jason A. Donenfeld, x86, linux-kernel, Dov Murik,
Tom Lendacky, Gerd Hoffmann, Paolo Bonzini, Richard Henderson,
H . Peter Anvin, Philippe Mathieu-Daudé,
Nathan Chancellor, Borislav Petkov, Eric Biggers
On Mon, Feb 20, 2023 at 10:48:57AM +0000, Daniel P. Berrangé wrote:
> On Fri, Feb 10, 2023 at 11:32:58AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Feb 08, 2023 at 04:12:23PM -0500, Michael S. Tsirkin wrote:
> > > All attempts to fix up passing RNG seed via setup_data entry failed.
> > > Let's just rip out all of it. We'll start over.
> > >
> > >
> > > Warning: all I did was git revert the relevant patches and resolve the
> > > (trivial) conflicts. Not even compiled - it's almost midnight here.
> > >
> > > Jason this is the kind of approach I'd like to see, not yet another
> > > pointer math rich patch I need to spend time reviewing. Just get us back
> > > to where we started. We can redo "x86: use typedef for SetupData struct"
> > > later if we want, it's benign.
> >
> > This approach looks suitable for applying to the 7.2 tree too,
> > which will be good for fixing the regressions in stable.
>
> Since no further alternative has been proposed, can you consider sending
> a pull request for this series. This has been broken for too long and
> many users & vendors are looking for an official fix to be applied to
> master before they backport to 7.2
>
> 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 :|
Will do. Thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread