[PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Thomas Huth]

For downstream versions of QEMU, we'd like to be able to compile QEMU
without the FDC code included (since it's not required for modern VMs
anymore and the FDC code has rather a bad reputation, see the VENOM CVE).

The q35 machine can already be instantiated without FDC, but for being
able to link a binary without the FDC code, the Kconfig file needs some
tweaks and there are two spots in the pc code that directly call functions
from the FDC code - those need to be disabled via #ifdefs.

The third patch changes the i440fx and isapc machine types so that
they can work without the FDC device, too, in case it has not been
compiled into the binary. It's marked as RFC since I assume that the
FDC was originally a fix compononent of these motherboards, so I'm
unsure whether we should allow the disablement there. OTOH, it seems
to work fine, and the FDC is only disabled when it is not available
in the binary, so I hope this patch is fine, too.

Thomas Huth (3):
  hw/i386/pc: Allow to compile without CONFIG_FDC_ISA
  hw/i386/Kconfig: Allow to compile Q35 without FDC_ISA
  hw/i386: Add the possibility to use i440fx and isapc without FDC

 hw/i386/pc.c      | 13 +++++++++----
 hw/i386/pc_piix.c |  6 ++++--
 hw/i386/Kconfig   |  2 +-
 3 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.44.0

[PATCH 1/3] hw/i386/pc: Allow to compile without CONFIG_FDC_ISA

[Thomas Huth]

The q35 machine can work without FDC. But to be able to also link
a QEMU binary that does not include the FDC code, we have to make
it possible to disable the spots that call into the FDC code.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/i386/pc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 08c7de416f..93c48f6747 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -439,16 +439,19 @@ static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
 
 static void pc_cmos_init_floppy(MC146818RtcState *rtc_state, ISADevice *floppy)
 {
-    int val, nb, i;
+    int val, nb;
     FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
                                    FLOPPY_DRIVE_TYPE_NONE };
 
+#ifdef CONFIG_FDC_ISA
     /* floppy type */
     if (floppy) {
-        for (i = 0; i < 2; i++) {
+        for (int i = 0; i < 2; i++) {
             fd_type[i] = isa_fdc_get_drive_type(floppy, i);
         }
     }
+#endif
+
     val = (cmos_get_fd_drive_type(fd_type[0]) << 4) |
         cmos_get_fd_drive_type(fd_type[1]);
     mc146818rtc_set_cmos_data(rtc_state, 0x10, val);
@@ -1132,7 +1135,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
     int i;
     DriveInfo *fd[MAX_FD];
     qemu_irq *a20_line;
-    ISADevice *fdc, *i8042, *port92, *vmmouse;
+    ISADevice *i8042, *port92, *vmmouse;
 
     serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS);
     parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
@@ -1142,11 +1145,13 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
         create_fdctrl |= !!fd[i];
     }
     if (create_fdctrl) {
-        fdc = isa_new(TYPE_ISA_FDC);
+#ifdef CONFIG_FDC_ISA
+        ISADevice *fdc = isa_new(TYPE_ISA_FDC);
         if (fdc) {
             isa_realize_and_unref(fdc, isa_bus, &error_fatal);
             isa_fdc_init_drives(fdc, fd);
         }
+#endif
     }
 
     if (!create_i8042) {
-- 
2.44.0

[PATCH 2/3] hw/i386/Kconfig: Allow to compile Q35 without FDC_ISA

[Thomas Huth]

The q35 machine can be used without floppy disk controller (FDC),
but due to our current Kconfig setup, the FDC code is still always
included in the binary. To fix this, the "PC" config option should
only imply the "FDC_ISA" instead of always selecting it.

The i440fx and the isa-pc machine currently always instantiate
the FDC, so we have to add the select statements now there instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/i386/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index a6ee052f9a..0b08580862 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -32,7 +32,7 @@ config PC
     imply VGA_PCI
     imply VIRTIO_VGA
     imply NVDIMM
-    select FDC_ISA
+    imply FDC_ISA
     select I8259
     select I8254
     select PCKBD
@@ -70,6 +70,7 @@ config I440FX
     imply VMPORT
     imply VMMOUSE
     select ACPI_PIIX4
+    select FDC_ISA
     select PC_PCI
     select PC_ACPI
     select PCI_I440FX
@@ -83,6 +84,7 @@ config ISAPC
     bool
     imply VGA_ISA
     select ISA_BUS
+    select FDC_ISA
     select PC
     select IDE_ISA
     # FIXME: it is in the same file as i440fx, and does not compile
-- 
2.44.0

[RFC PATCH 3/3] hw/i386: Add the possibility to use i440fx and isapc without FDC

[Thomas Huth]

The i440fx and the isapc machines can be used in binaries without
FDC, too. We just have to make sure that they don't try to instantiate
the FDC when it is not available.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/i386/pc_piix.c | 6 ++++--
 hw/i386/Kconfig   | 2 --
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8850c49c66..99efb3c45c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -317,8 +317,8 @@ static void pc_init1(MachineState *machine, const char *pci_type)
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, true,
-                         0x4);
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
+                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
 
     pc_nic_init(pcmc, isa_bus, pcms->pcibus);
 
@@ -501,6 +501,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
     m->default_nic = "e1000";
+    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
@@ -931,6 +932,7 @@ static void isapc_machine_options(MachineClass *m)
     pcmc->has_reserved_memory = false;
     m->default_nic = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
 }
 
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 0b08580862..f2ef6d1ef2 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -70,7 +70,6 @@ config I440FX
     imply VMPORT
     imply VMMOUSE
     select ACPI_PIIX4
-    select FDC_ISA
     select PC_PCI
     select PC_ACPI
     select PCI_I440FX
@@ -84,7 +83,6 @@ config ISAPC
     bool
     imply VGA_ISA
     select ISA_BUS
-    select FDC_ISA
     select PC
     select IDE_ISA
     # FIXME: it is in the same file as i440fx, and does not compile
-- 
2.44.0

Re: [PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Philippe Mathieu-Daudé]

Hi Thomas,

On 25/4/24 20:43, Thomas Huth wrote:
> For downstream versions of QEMU, we'd like to be able to compile QEMU
> without the FDC code included (since it's not required for modern VMs
> anymore and the FDC code has rather a bad reputation, see the VENOM CVE).

IIRC you still need to keep the i440fx+piix machine DS due to
migration compatibility, right?

Are you able to migrate a VM booted with FDC to a non-FDC one?

> The q35 machine can already be instantiated without FDC, but for being
> able to link a binary without the FDC code, the Kconfig file needs some
> tweaks and there are two spots in the pc code that directly call functions
> from the FDC code - those need to be disabled via #ifdefs.

Is it useful to you to have q35 without FDC but i440fx+piix with?
Or are you removing it from i440fx+piix due to shared code with q35?

> The third patch changes the i440fx and isapc machine types so that
> they can work without the FDC device, too, in case it has not been
> compiled into the binary. It's marked as RFC since I assume that the
> FDC was originally a fix compononent of these motherboards, so I'm
> unsure whether we should allow the disablement there. OTOH, it seems
> to work fine, and the FDC is only disabled when it is not available
> in the binary, so I hope this patch is fine, too.
> 
> Thomas Huth (3):
>    hw/i386/pc: Allow to compile without CONFIG_FDC_ISA
>    hw/i386/Kconfig: Allow to compile Q35 without FDC_ISA
>    hw/i386: Add the possibility to use i440fx and isapc without FDC
> 
>   hw/i386/pc.c      | 13 +++++++++----
>   hw/i386/pc_piix.c |  6 ++++--
>   hw/i386/Kconfig   |  2 +-
>   3 files changed, 14 insertions(+), 7 deletions(-)
> 

Re: [PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Thomas Huth]

On 25/04/2024 22.56, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 25/4/24 20:43, Thomas Huth wrote:
>> For downstream versions of QEMU, we'd like to be able to compile QEMU
>> without the FDC code included (since it's not required for modern VMs
>> anymore and the FDC code has rather a bad reputation, see the VENOM CVE).
> 
> IIRC you still need to keep the i440fx+piix machine DS due to
> migration compatibility, right?
> 
> Are you able to migrate a VM booted with FDC to a non-FDC one?

Migration from a machine with FDC to one without is likely not possible. But 
that's also not the purpose of this series. It's about having the 
possibility to build a QEMU binary without FDC at all in case you don't have 
to support old machine types with FDC anymore.

>> The q35 machine can already be instantiated without FDC, but for being
>> able to link a binary without the FDC code, the Kconfig file needs some
>> tweaks and there are two spots in the pc code that directly call functions
>> from the FDC code - those need to be disabled via #ifdefs.
> 
> Is it useful to you to have q35 without FDC but i440fx+piix with?
> Or are you removing it from i440fx+piix due to shared code with q35?

I think it might get useful.

  Thomas

Re: [PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Kevin Wolf]

[ Cc: qemu-block ]

Am 25.04.2024 um 20:43 hat Thomas Huth geschrieben:
> For downstream versions of QEMU, we'd like to be able to compile QEMU
> without the FDC code included (since it's not required for modern VMs
> anymore and the FDC code has rather a bad reputation, see the VENOM CVE).
> 
> The q35 machine can already be instantiated without FDC, but for being
> able to link a binary without the FDC code, the Kconfig file needs some
> tweaks and there are two spots in the pc code that directly call functions
> from the FDC code - those need to be disabled via #ifdefs.
> 
> The third patch changes the i440fx and isapc machine types so that
> they can work without the FDC device, too, in case it has not been
> compiled into the binary. It's marked as RFC since I assume that the
> FDC was originally a fix compononent of these motherboards, so I'm
> unsure whether we should allow the disablement there. OTOH, it seems
> to work fine, and the FDC is only disabled when it is not available
> in the binary, so I hope this patch is fine, too.
> 
> Thomas Huth (3):
>   hw/i386/pc: Allow to compile without CONFIG_FDC_ISA
>   hw/i386/Kconfig: Allow to compile Q35 without FDC_ISA
>   hw/i386: Add the possibility to use i440fx and isapc without FDC
> 
>  hw/i386/pc.c      | 13 +++++++++----
>  hw/i386/pc_piix.c |  6 ++++--
>  hw/i386/Kconfig   |  2 +-
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> -- 
> 2.44.0
> 
> 

Re: [PATCH 1/3] hw/i386/pc: Allow to compile without CONFIG_FDC_ISA

[Philippe Mathieu-Daudé]

Hi Thomas,

On 25/4/24 20:43, Thomas Huth wrote:
> The q35 machine can work without FDC. But to be able to also link
> a QEMU binary that does not include the FDC code, we have to make
> it possible to disable the spots that call into the FDC code.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/i386/pc.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 08c7de416f..93c48f6747 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -439,16 +439,19 @@ static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
>   
>   static void pc_cmos_init_floppy(MC146818RtcState *rtc_state, ISADevice *floppy)
>   {
> -    int val, nb, i;
> +    int val, nb;
>       FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
>                                      FLOPPY_DRIVE_TYPE_NONE };
>   
> +#ifdef CONFIG_FDC_ISA
>       /* floppy type */
>       if (floppy) {
> -        for (i = 0; i < 2; i++) {
> +        for (int i = 0; i < 2; i++) {
>               fd_type[i] = isa_fdc_get_drive_type(floppy, i);
>           }
>       }
> +#endif
> +
>       val = (cmos_get_fd_drive_type(fd_type[0]) << 4) |
>           cmos_get_fd_drive_type(fd_type[1]);
>       mc146818rtc_set_cmos_data(rtc_state, 0x10, val);
> @@ -1132,7 +1135,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
>       int i;
>       DriveInfo *fd[MAX_FD];
>       qemu_irq *a20_line;
> -    ISADevice *fdc, *i8042, *port92, *vmmouse;
> +    ISADevice *i8042, *port92, *vmmouse;
>   
>       serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS);
>       parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
> @@ -1142,11 +1145,13 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
>           create_fdctrl |= !!fd[i];
>       }
>       if (create_fdctrl) {
> -        fdc = isa_new(TYPE_ISA_FDC);
> +#ifdef CONFIG_FDC_ISA
> +        ISADevice *fdc = isa_new(TYPE_ISA_FDC);
>           if (fdc) {
>               isa_realize_and_unref(fdc, isa_bus, &error_fatal);
>               isa_fdc_init_drives(fdc, fd);
>           }
> +#endif
>       }

So the problems are these 2 functions:

FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
{
     FDCtrlISABus *isa = ISA_FDC(fdc);

     return isa->state.drives[i].drive;
}

used by cmos_get_fd_drive_type() and

void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds)
{
     fdctrl_init_drives(&ISA_FDC(fdc)->state.bus, fds);
}

which expands to:

void fdctrl_init_drives(FloppyBus *bus, DriveInfo **fds)
{
     DeviceState *dev;
     int i;

     for (i = 0; i < MAX_FD; i++) {
         if (fds[i]) {
             dev = qdev_new("floppy");
             qdev_prop_set_uint32(dev, "unit", i);
             qdev_prop_set_enum(dev, "drive-type", FLOPPY_DRIVE_TYPE_AUTO);
             qdev_prop_set_drive_err(dev, "drive", 
blk_by_legacy_dinfo(fds[i]),
                                     &error_fatal);
             qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
         }
     }
}

Pre-QOM code I suppose, since it is odd to create the floppy
drives out of the FDC realize() method. Somehow DriveInfo[]
should be passed to the FDC object before it is realized.

For cmos_get_fd_drive_type, we could expose the FloppyDriveType
as a QOM property. Then we only need the QOM API to access the
FDC, and it can be built as a generic modularized QDev object,
usable in a generic qemu-system binary.

This is why I was holding this series review, I needed to give
it some thoughts w.r.t. single binary. I'm gonna ack the series,
postponing the FDC rework.

Regards,

Phil.

Re: [PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Philippe Mathieu-Daudé]

On 25/4/24 20:43, Thomas Huth wrote:

> Thomas Huth (3):
>    hw/i386/pc: Allow to compile without CONFIG_FDC_ISA
>    hw/i386/Kconfig: Allow to compile Q35 without FDC_ISA
>    hw/i386: Add the possibility to use i440fx and isapc without FDC

Series:
Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/3] Make it possible to compile the x86 binaries without FDC

[Paolo Bonzini]

On Thu, Apr 25, 2024 at 8:43 PM Thomas Huth <thuth@redhat.com> wrote:
> OTOH, it seems
> to work fine, and the FDC is only disabled when it is not available
> in the binary, so I hope this patch is fine, too.

We do the same for parallel so i think it should be fine---definitely
for -nodefaults, and I'd say in general too.  The CMOS byte already
has a way to communicate no-floppy (0, see cmos_get_fd_drive_type).

Paolo