Re: [PATCH v3] pc: hide linuxboot RNG seed behind a machine property

[Michael S. Tsirkin]

On Wed, Aug 10, 2022 at 09:25:05PM +0200, Paolo Bonzini wrote:
> 
> 
> Il mer 10 ago 2022, 19:06 Michael S. Tsirkin <mst@redhat.com> ha scritto:
> 
>     > @@ -1387,6 +1405,7 @@ static void x86_machine_initfn(Object *obj)
>     >      x86ms->acpi = ON_OFF_AUTO_AUTO;
>     >      x86ms->pit = ON_OFF_AUTO_AUTO;
>     >      x86ms->pic = ON_OFF_AUTO_AUTO;
>     > +    x86ms->linuxboot_randomness = ON_OFF_AUTO_OFF;
>     >      x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
>     >      x86ms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>     >      x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
> 
> 
>     This is a weird thing to do in that there's no way to make it auto now.
> 
> 
> Sure, -M x-linuxboot-randomness=auto works. And making it already on/off/auto
> will limit future patches to the injection logic rather than the QOM
> boilerplate.
> 
> Paolo

Hmm. It's unusual that auto is not the same as "no value at all".
But I guess there's no rule saying that can't be the case.
OK then.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>




> 
>     And in that case, let's just make it a simple boolean property for now?
> 
>     > @@ -1426,6 +1445,12 @@ static void x86_machine_class_init(ObjectClass
>     *oc, void *data)
>     >      object_class_property_set_description(oc, X86_MACHINE_PIT,
>     >          "Enable i8254 PIT");
>     > 
>     > +    object_class_property_add(oc, X86_MACHINE_LINUXBOOT_RANDOMNESS,
>     "OnOffAuto",
>     > +        x86_machine_get_linuxboot_randomness,
>     x86_machine_set_linuxboot_randomness,
>     > +        NULL, NULL);
>     > +    object_class_property_set_description(oc,
>     X86_MACHINE_LINUXBOOT_RANDOMNESS,
>     > +        "Pass random number seed to -kernel Linux image");
>     > +
>     >      object_class_property_add(oc, X86_MACHINE_PIC, "OnOffAuto",
>     >                                x86_machine_get_pic,
>     >                                x86_machine_set_pic,
>     > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>     > index 8435733bd6..9cc3f5d338 100644
>     > --- a/include/hw/i386/pc.h
>     > +++ b/include/hw/i386/pc.h
>     > @@ -128,9 +128,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..d7a2eb6f1c 100644
>     > --- a/include/hw/i386/x86.h
>     > +++ b/include/hw/i386/x86.h
>     > @@ -70,6 +70,7 @@ struct X86MachineState {
>     >      OnOffAuto acpi;
>     >      OnOffAuto pit;
>     >      OnOffAuto pic;
>     > +    OnOffAuto linuxboot_randomness;
>     > 
>     >      char *oem_id;
>     >      char *oem_table_id;
>     > @@ -94,6 +95,7 @@ struct X86MachineState {
>     >  #define X86_MACHINE_OEM_ID           "x-oem-id"
>     >  #define X86_MACHINE_OEM_TABLE_ID     "x-oem-table-id"
>     >  #define X86_MACHINE_BUS_LOCK_RATELIMIT  "bus-lock-ratelimit"
>     > +#define X86_MACHINE_LINUXBOOT_RANDOMNESS "x-linuxboot-randomness"
>     > 
>     >  #define TYPE_X86_MACHINE   MACHINE_TYPE_NAME("x86")
>     >  OBJECT_DECLARE_TYPE(X86MachineState, X86MachineClass, X86_MACHINE)
>     > @@ -126,8 +128,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);
>     > --
>     > 2.37.1
> 
>