Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property

[Paolo Bonzini]

On Tue, Feb 20, 2024 at 11:43 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 20. Februar 2024 15:53:52 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
> >Add a fd-bootchk property to PC machine types, so that -no-fd-bootchk
> >returns an error if the machine does not support booting from floppies
> >and checking for boot signatures therein.
> >
> >Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >---
> > include/hw/i386/pc.h |  2 +-
> > hw/i386/pc.c         | 30 +++++++++++++++++++++++++-----
> > system/globals.c     |  1 -
> > system/vl.c          |  2 +-
> > qemu-options.hx      |  2 +-
> > 5 files changed, 28 insertions(+), 9 deletions(-)
> >
> >diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >index 02a0deedd3c..e5382a02e7a 100644
> >--- a/include/hw/i386/pc.h
> >+++ b/include/hw/i386/pc.h
> >@@ -50,6 +50,7 @@ typedef struct PCMachineState {
> >     bool hpet_enabled;
> >     bool i8042_enabled;
> >     bool default_bus_bypass_iommu;
> >+    bool fd_bootchk;
> >     uint64_t max_fw_size;
> >
> >     /* ACPI Memory hotplug IO base address */
> >@@ -147,7 +148,6 @@ OBJECT_DECLARE_TYPE(PCMachineState, PCMachineClass, 
> >PC_MACHINE)
> > GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
> >
> > /* pc.c */
> >-extern int fd_bootchk;
> >
> > void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
> >
> >diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >index 28194014f82..31f4bb25a3e 100644
> >--- a/hw/i386/pc.c
> >+++ b/hw/i386/pc.c
> >@@ -399,8 +399,8 @@ static int boot_device2nibble(char boot_device)
> >     return 0;
> > }
> >
> >-static void set_boot_dev(MC146818RtcState *s, const char *boot_device,
> >-                         Error **errp)
> >+static void set_boot_dev(PCMachineState *pcms, MC146818RtcState *s,
> >+                         const char *boot_device, Error **errp)
> > {
> > #define PC_MAX_BOOT_DEVICES 3
> >     int nbds, bds[3] = { 0, };
> >@@ -420,12 +420,14 @@ static void set_boot_dev(MC146818RtcState *s, const 
> >char *boot_device,
> >         }
> >     }
> >     mc146818rtc_set_cmos_data(s, 0x3d, (bds[1] << 4) | bds[0]);
> >-    mc146818rtc_set_cmos_data(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 
> >0x1));
> >+    mc146818rtc_set_cmos_data(s, 0x38, (bds[2] << 4) | !pcms->fd_bootchk);
> > }
> >
> > static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
> > {
> >-    set_boot_dev(opaque, boot_device, errp);
> >+    PCMachineState *pcms = PC_MACHINE(current_machine);
> >+
> >+    set_boot_dev(pcms, opaque, boot_device, errp);
> > }
> >
> > static void pc_cmos_init_floppy(MC146818RtcState *rtc_state, ISADevice 
> > *floppy)
> >@@ -617,6 +619,9 @@ void pc_cmos_init(PCMachineState *pcms,
> >     mc146818rtc_set_cmos_data(s, 0x5c, val >> 8);
> >     mc146818rtc_set_cmos_data(s, 0x5d, val >> 16);
> >
> >+    object_property_add_bool(obj, "fd-bootchk", pc_machine_get_fd_bootchk,
> >+                             pc_machine_set_fd_bootchk);
>
> Isn't it possible to turn this into a class property or add the property in 
> pc_machine_initfn()? Aggregating properties in one place seems more 
> comprehensible to me.

Sure, I placed it in pc_cmos_init because rtc_state is already created here.

Paolo