* [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
@ 2024-02-20 15:53 Paolo Bonzini
2024-02-20 18:58 ` Philippe Mathieu-Daudé
2024-02-20 22:43 ` Bernhard Beschow
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2024-02-20 15:53 UTC (permalink / raw)
To: qemu-devel; +Cc: philmd
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);
+
object_property_add_link(OBJECT(pcms), "rtc_state",
TYPE_ISA_DEVICE,
(Object **)&x86ms->rtc,
@@ -625,7 +630,7 @@ void pc_cmos_init(PCMachineState *pcms,
object_property_set_link(OBJECT(pcms), "rtc_state", OBJECT(s),
&error_abort);
- set_boot_dev(s, MACHINE(pcms)->boot_config.order, &error_fatal);
+ set_boot_dev(pcms, s, MACHINE(pcms)->boot_config.order, &error_fatal);
val = 0;
val |= 0x02; /* FPU is there */
@@ -1559,6 +1564,20 @@ static void pc_machine_set_vmport(Object *obj, Visitor *v, const char *name,
visit_type_OnOffAuto(v, name, &pcms->vmport, errp);
}
+static bool pc_machine_get_fd_bootchk(Object *obj, Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ return pcms->fd_bootchk;
+}
+
+static void pc_machine_set_fd_bootchk(Object *obj, bool value, Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ pcms->fd_bootchk = value;
+}
+
static bool pc_machine_get_smbus(Object *obj, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
@@ -1747,6 +1766,7 @@ static void pc_machine_initfn(Object *obj)
#ifdef CONFIG_HPET
pcms->hpet_enabled = true;
#endif
+ pcms->fd_bootchk = true;
pcms->default_bus_bypass_iommu = false;
pc_system_flash_create(pcms);
diff --git a/system/globals.c b/system/globals.c
index b6d4e72530e..5d0046ba105 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -41,7 +41,6 @@ int vga_interface_type = VGA_NONE;
bool vga_interface_created;
Chardev *parallel_hds[MAX_PARALLEL_PORTS];
int win2k_install_hack;
-int fd_bootchk = 1;
int graphic_rotate;
QEMUOptionRom option_rom[MAX_OPTION_ROMS];
int nb_option_roms;
diff --git a/system/vl.c b/system/vl.c
index a82555ae155..4627004304b 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2927,7 +2927,7 @@ void qemu_init(int argc, char **argv)
optarg, FD_OPTS);
break;
case QEMU_OPTION_no_fd_bootchk:
- fd_bootchk = 0;
+ qdict_put_str(machine_opts_dict, "fd-bootchk", "off");
break;
case QEMU_OPTION_netdev:
default_net = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 8547254dbf9..a9e0107b4f0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2650,7 +2650,7 @@ DEF("no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk,
SRST
``-no-fd-bootchk``
Disable boot signature checking for floppy disks in BIOS. May be
- needed to boot from old floppy disks.
+ needed to boot from old floppy disks. Synonym of ``-m fd-bootchk=off``.
ERST
DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
2024-02-20 15:53 [PATCH] vl, pc: turn -no-fd-bootchk into a machine property Paolo Bonzini
@ 2024-02-20 18:58 ` Philippe Mathieu-Daudé
2024-02-20 22:43 ` Bernhard Beschow
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-20 18:58 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Bernhard Beschow
On 20/2/24 16:53, Paolo Bonzini wrote:
> 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);
> +
> object_property_add_link(OBJECT(pcms), "rtc_state",
> TYPE_ISA_DEVICE,
> (Object **)&x86ms->rtc,
> @@ -625,7 +630,7 @@ void pc_cmos_init(PCMachineState *pcms,
> object_property_set_link(OBJECT(pcms), "rtc_state", OBJECT(s),
> &error_abort);
>
> - set_boot_dev(s, MACHINE(pcms)->boot_config.order, &error_fatal);
> + set_boot_dev(pcms, s, MACHINE(pcms)->boot_config.order, &error_fatal);
>
> val = 0;
> val |= 0x02; /* FPU is there */
> @@ -1559,6 +1564,20 @@ static void pc_machine_set_vmport(Object *obj, Visitor *v, const char *name,
> visit_type_OnOffAuto(v, name, &pcms->vmport, errp);
> }
>
> +static bool pc_machine_get_fd_bootchk(Object *obj, Error **errp)
> +{
> + PCMachineState *pcms = PC_MACHINE(obj);
> +
> + return pcms->fd_bootchk;
> +}
> +
> +static void pc_machine_set_fd_bootchk(Object *obj, bool value, Error **errp)
> +{
> + PCMachineState *pcms = PC_MACHINE(obj);
> +
> + pcms->fd_bootchk = value;
> +}
> +
> static bool pc_machine_get_smbus(Object *obj, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1747,6 +1766,7 @@ static void pc_machine_initfn(Object *obj)
> #ifdef CONFIG_HPET
> pcms->hpet_enabled = true;
> #endif
> + pcms->fd_bootchk = true;
> pcms->default_bus_bypass_iommu = false;
>
> pc_system_flash_create(pcms);
> diff --git a/system/globals.c b/system/globals.c
> index b6d4e72530e..5d0046ba105 100644
> --- a/system/globals.c
> +++ b/system/globals.c
> @@ -41,7 +41,6 @@ int vga_interface_type = VGA_NONE;
> bool vga_interface_created;
> Chardev *parallel_hds[MAX_PARALLEL_PORTS];
> int win2k_install_hack;
> -int fd_bootchk = 1;
> int graphic_rotate;
> QEMUOptionRom option_rom[MAX_OPTION_ROMS];
> int nb_option_roms;
> diff --git a/system/vl.c b/system/vl.c
> index a82555ae155..4627004304b 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2927,7 +2927,7 @@ void qemu_init(int argc, char **argv)
> optarg, FD_OPTS);
> break;
> case QEMU_OPTION_no_fd_bootchk:
> - fd_bootchk = 0;
> + qdict_put_str(machine_opts_dict, "fd-bootchk", "off");
> break;
> case QEMU_OPTION_netdev:
> default_net = 0;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8547254dbf9..a9e0107b4f0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2650,7 +2650,7 @@ DEF("no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk,
> SRST
> ``-no-fd-bootchk``
> Disable boot signature checking for floppy disks in BIOS. May be
> - needed to boot from old floppy disks.
> + needed to boot from old floppy disks. Synonym of ``-m fd-bootchk=off``.
Thank you!
Shouldn't we deprecate -no-fd-bootchk in favor of -m fd-bootchk=off?
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ERST
>
> DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
2024-02-20 15:53 [PATCH] vl, pc: turn -no-fd-bootchk into a machine property Paolo Bonzini
2024-02-20 18:58 ` Philippe Mathieu-Daudé
@ 2024-02-20 22:43 ` Bernhard Beschow
2024-02-21 9:04 ` Paolo Bonzini
1 sibling, 1 reply; 6+ messages in thread
From: Bernhard Beschow @ 2024-02-20 22:43 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: philmd
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.
Note that I've submitted a series attempting to simplify initialization of the PC machines which still waits for reviews: https://patchew.org/QEMU/20240208220349.4948-1-shentey@gmail.com/
Thanks,
Bernhard
>+
> object_property_add_link(OBJECT(pcms), "rtc_state",
> TYPE_ISA_DEVICE,
> (Object **)&x86ms->rtc,
>@@ -625,7 +630,7 @@ void pc_cmos_init(PCMachineState *pcms,
> object_property_set_link(OBJECT(pcms), "rtc_state", OBJECT(s),
> &error_abort);
>
>- set_boot_dev(s, MACHINE(pcms)->boot_config.order, &error_fatal);
>+ set_boot_dev(pcms, s, MACHINE(pcms)->boot_config.order, &error_fatal);
>
> val = 0;
> val |= 0x02; /* FPU is there */
>@@ -1559,6 +1564,20 @@ static void pc_machine_set_vmport(Object *obj, Visitor *v, const char *name,
> visit_type_OnOffAuto(v, name, &pcms->vmport, errp);
> }
>
>+static bool pc_machine_get_fd_bootchk(Object *obj, Error **errp)
>+{
>+ PCMachineState *pcms = PC_MACHINE(obj);
>+
>+ return pcms->fd_bootchk;
>+}
>+
>+static void pc_machine_set_fd_bootchk(Object *obj, bool value, Error **errp)
>+{
>+ PCMachineState *pcms = PC_MACHINE(obj);
>+
>+ pcms->fd_bootchk = value;
>+}
>+
> static bool pc_machine_get_smbus(Object *obj, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>@@ -1747,6 +1766,7 @@ static void pc_machine_initfn(Object *obj)
> #ifdef CONFIG_HPET
> pcms->hpet_enabled = true;
> #endif
>+ pcms->fd_bootchk = true;
> pcms->default_bus_bypass_iommu = false;
>
> pc_system_flash_create(pcms);
>diff --git a/system/globals.c b/system/globals.c
>index b6d4e72530e..5d0046ba105 100644
>--- a/system/globals.c
>+++ b/system/globals.c
>@@ -41,7 +41,6 @@ int vga_interface_type = VGA_NONE;
> bool vga_interface_created;
> Chardev *parallel_hds[MAX_PARALLEL_PORTS];
> int win2k_install_hack;
>-int fd_bootchk = 1;
> int graphic_rotate;
> QEMUOptionRom option_rom[MAX_OPTION_ROMS];
> int nb_option_roms;
>diff --git a/system/vl.c b/system/vl.c
>index a82555ae155..4627004304b 100644
>--- a/system/vl.c
>+++ b/system/vl.c
>@@ -2927,7 +2927,7 @@ void qemu_init(int argc, char **argv)
> optarg, FD_OPTS);
> break;
> case QEMU_OPTION_no_fd_bootchk:
>- fd_bootchk = 0;
>+ qdict_put_str(machine_opts_dict, "fd-bootchk", "off");
> break;
> case QEMU_OPTION_netdev:
> default_net = 0;
>diff --git a/qemu-options.hx b/qemu-options.hx
>index 8547254dbf9..a9e0107b4f0 100644
>--- a/qemu-options.hx
>+++ b/qemu-options.hx
>@@ -2650,7 +2650,7 @@ DEF("no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk,
> SRST
> ``-no-fd-bootchk``
> Disable boot signature checking for floppy disks in BIOS. May be
>- needed to boot from old floppy disks.
>+ needed to boot from old floppy disks. Synonym of ``-m fd-bootchk=off``.
> ERST
>
> DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
2024-02-20 22:43 ` Bernhard Beschow
@ 2024-02-21 9:04 ` Paolo Bonzini
2024-02-21 9:47 ` Bernhard Beschow
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2024-02-21 9:04 UTC (permalink / raw)
To: Bernhard Beschow; +Cc: qemu-devel, philmd
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
2024-02-21 9:04 ` Paolo Bonzini
@ 2024-02-21 9:47 ` Bernhard Beschow
2024-02-21 17:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Bernhard Beschow @ 2024-02-21 9:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, philmd, Peter Maydell, Michael S. Tsirkin
Am 21. Februar 2024 09:04:21 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>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.
Great! I'll rebase my PC initialization series on top of Peter's reset cleanup series which probably results in folding cmos initialization into pc.c.
Best regards,
Bernhard
>
>Paolo
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-21 17:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-20 15:53 [PATCH] vl, pc: turn -no-fd-bootchk into a machine property Paolo Bonzini
2024-02-20 18:58 ` Philippe Mathieu-Daudé
2024-02-20 22:43 ` Bernhard Beschow
2024-02-21 9:04 ` Paolo Bonzini
2024-02-21 9:47 ` Bernhard Beschow
2024-02-21 17:34 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).