[PULL 05/10] x86: disable rng seeding via setup_data

Michael S. Tsirkin posted 10 patches 3 years, 5 months ago
There is a newer version of this series
[PULL 05/10] x86: disable rng seeding via setup_data
Posted by Michael S. Tsirkin 3 years, 5 months ago
From: Gerd Hoffmann <kraxel@redhat.com>

Causes regressions when doing direct kernel boots with OVMF.

At this point in the release cycle the only sensible action
is to just disable this for 7.1 and sort it properly in the
7.2 devel cycle.

Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-Id: <20220817083940.3174933-1-kraxel@redhat.com>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/microvm.c | 2 +-
 hw/i386/pc_piix.c | 2 +-
 hw/i386/pc_q35.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 7fe8cce03e..52cafa003d 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -332,7 +332,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, true);
     }
 
     if (mms->option_roms) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a5c65c1c35..20962c34e7 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
     m->alias = "pc";
     m->is_default = true;
     pcmc->default_cpu_version = 1;
+    pcmc->legacy_no_rng_seed = true;
 }
 
 DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
@@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
     pc_i440fx_7_1_machine_options(m);
     m->alias = NULL;
     m->is_default = false;
-    pcmc->legacy_no_rng_seed = true;
     pcmc->enforce_amd_1tb_hole = false;
     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 3a35193ff7..2e5dae9a89 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
     pc_q35_machine_options(m);
     m->alias = "q35";
     pcmc->default_cpu_version = 1;
+    pcmc->legacy_no_rng_seed = true;
 }
 
 DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
@@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_7_1_machine_options(m);
     m->alias = NULL;
-    pcmc->legacy_no_rng_seed = true;
     pcmc->enforce_amd_1tb_hole = false;
     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
-- 
MST


Re: [PULL 05/10] x86: disable rng seeding via setup_data
Posted by Paolo Bonzini 3 years, 5 months ago
On 8/17/22 18:14, Michael S. Tsirkin wrote:
> @@ -332,7 +332,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, true);
>       }
>   
>       if (mms->option_roms) {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a5c65c1c35..20962c34e7 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
>       m->alias = "pc";
>       m->is_default = true;
>       pcmc->default_cpu_version = 1;
> +    pcmc->legacy_no_rng_seed = true;
>   }
>   
>   DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
> @@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
>       pc_i440fx_7_1_machine_options(m);
>       m->alias = NULL;
>       m->is_default = false;
> -    pcmc->legacy_no_rng_seed = true;
>       pcmc->enforce_amd_1tb_hole = false;
>       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 3a35193ff7..2e5dae9a89 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
>       pc_q35_machine_options(m);
>       m->alias = "q35";
>       pcmc->default_cpu_version = 1;
> +    pcmc->legacy_no_rng_seed = true;
>   }
>   
>   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
>       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_7_1_machine_options(m);
>       m->alias = NULL;
> -    pcmc->legacy_no_rng_seed = true;
>       pcmc->enforce_amd_1tb_hole = false;
>       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);

Why not just revert the whole patch?

Paolo
Re: [PULL 05/10] x86: disable rng seeding via setup_data
Posted by Gerd Hoffmann 3 years, 5 months ago
  Hi,

> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 3a35193ff7..2e5dae9a89 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> >       pc_q35_machine_options(m);
> >       m->alias = "q35";
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >       pc_q35_7_1_machine_options(m);
> >       m->alias = NULL;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> 
> Why not just revert the whole patch?

Tried that first.  Plain revert not working, there are conflicts.
So just disabling the code looked simpler and safer to me.

take care,
  Gerd
Re: [PULL 05/10] x86: disable rng seeding via setup_data
Posted by Jason A. Donenfeld 3 years, 5 months ago
Hi Gerd, Michael, Paolo,

On Thu, Aug 18, 2022 at 01:56:14PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > index 3a35193ff7..2e5dae9a89 100644
> > > --- a/hw/i386/pc_q35.c
> > > +++ b/hw/i386/pc_q35.c
> > > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> > >       pc_q35_machine_options(m);
> > >       m->alias = "q35";
> > >       pcmc->default_cpu_version = 1;
> > > +    pcmc->legacy_no_rng_seed = true;
> > >   }
> > >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> > >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > >       pc_q35_7_1_machine_options(m);
> > >       m->alias = NULL;
> > > -    pcmc->legacy_no_rng_seed = true;
> > >       pcmc->enforce_amd_1tb_hole = false;
> > >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> > >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> > 
> > Why not just revert the whole patch?
> 
> Tried that first.  Plain revert not working, there are conflicts.
> So just disabling the code looked simpler and safer to me.

Yea, this is fine with me. This commit will be easy enough to revert in
7.2 when things are hopefully working properly in all circumstances.

Jason
Re: [PULL 05/10] x86: disable rng seeding via setup_data
Posted by Michael S. Tsirkin 3 years, 5 months ago
On Thu, Aug 18, 2022 at 11:27:30AM +0200, Paolo Bonzini wrote:
> On 8/17/22 18:14, Michael S. Tsirkin wrote:
> > @@ -332,7 +332,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, true);
> >       }
> >       if (mms->option_roms) {
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index a5c65c1c35..20962c34e7 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
> >       m->alias = "pc";
> >       m->is_default = true;
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
> > @@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
> >       pc_i440fx_7_1_machine_options(m);
> >       m->alias = NULL;
> >       m->is_default = false;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 3a35193ff7..2e5dae9a89 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> >       pc_q35_machine_options(m);
> >       m->alias = "q35";
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >       pc_q35_7_1_machine_options(m);
> >       m->alias = NULL;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> 
> Why not just revert the whole patch?
> 
> Paolo

At this point I was looking for a minimally intrusive change.

-- 
MST