| Message ID | 20230208211212.41951-1-mst@redhat.com |
|---|---|
| Headers | show |
| Series | revert RNG seed mess | expand |
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
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
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
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
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
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!
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(-)