1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/display/vga-isa: Fix migration of the isa-vga device

Thomas Huth posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260326113457.159065-1-thuth@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/display/vga-isa.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
[PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Thomas Huth 1 week ago 
From: Thomas Huth <thuth@redhat.com>

QEMU currently crashes when migrating a guest that uses the
isa-vga device as display. This happens because vga_isa_class_initfn()
registers a vmsd for vmstate_vga_common that operates on VGACommonState.
But the isa-vga device is derived from ISADevice, not from VGACommonState,
so the migration code tries to fill in the data for VGACommonState to
the memory that is a ISADevice instead, which is of cause causing trouble.

We need an indirection here as it's also e.g. done in vga-pci.c, so
that the migration data gets filled into the right location.

While we're at it, also drop the "global_vmstate = true" here. Since
migration was broken for this device during the last 15 years (!) anyway,
we don't have to worry about maintaining backward compatibility with this
switch for older versions of QEMU anymore.

Fixes: 7435b791ca9 ("vga-isa: convert to qdev")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/display/vga-isa.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 95d85ff69a5..5f55c884a1b 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -32,6 +32,7 @@
 #include "qemu/timer.h"
 #include "hw/core/loader.h"
 #include "hw/core/qdev-properties.h"
+#include "migration/vmstate.h"
 #include "ui/console.h"
 #include "qom/object.h"
 
@@ -62,7 +63,6 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
 
-    s->global_vmstate = true;
     if (!vga_common_init(s, OBJECT(dev), errp)) {
         return;
     }
@@ -88,6 +88,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     rom_add_vga(VGABIOS_FILENAME);
 }
 
+static const VMStateDescription vmstate_vga_isa = {
+    .name = "vga-isa",
+    .version_id = 1,
+    .fields = (const VMStateField[]) {
+        VMSTATE_STRUCT(state, ISAVGAState, 0, vmstate_vga_common, VGACommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const Property vga_isa_properties[] = {
     DEFINE_PROP_UINT32("vgamem_mb", ISAVGAState, state.vram_size_mb, 8),
 };
@@ -98,7 +107,7 @@ static void vga_isa_class_initfn(ObjectClass *klass, const void *data)
 
     dc->realize = vga_isa_realizefn;
     device_class_set_legacy_reset(dc, vga_isa_reset);
-    dc->vmsd = &vmstate_vga_common;
+    dc->vmsd = &vmstate_vga_isa;
     device_class_set_props(dc, vga_isa_properties);
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
 }
-- 
2.53.0
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Fabiano Rosas 1 week ago 
Thomas Huth <thuth@redhat.com> writes:

> From: Thomas Huth <thuth@redhat.com>
>
> QEMU currently crashes when migrating a guest that uses the
> isa-vga device as display. This happens because vga_isa_class_initfn()
> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
> But the isa-vga device is derived from ISADevice, not from VGACommonState,
> so the migration code tries to fill in the data for VGACommonState to
> the memory that is a ISADevice instead, which is of cause causing trouble.
>
> We need an indirection here as it's also e.g. done in vga-pci.c, so
> that the migration data gets filled into the right location.
>
> While we're at it, also drop the "global_vmstate = true" here. Since
> migration was broken for this device during the last 15 years (!) anyway,
> we don't have to worry about maintaining backward compatibility with this
> switch for older versions of QEMU anymore.
>
> Fixes: 7435b791ca9 ("vga-isa: convert to qdev")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/display/vga-isa.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
> index 95d85ff69a5..5f55c884a1b 100644
> --- a/hw/display/vga-isa.c
> +++ b/hw/display/vga-isa.c
> @@ -32,6 +32,7 @@
>  #include "qemu/timer.h"
>  #include "hw/core/loader.h"
>  #include "hw/core/qdev-properties.h"
> +#include "migration/vmstate.h"
>  #include "ui/console.h"
>  #include "qom/object.h"
>  
> @@ -62,7 +63,6 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>      MemoryRegion *vga_io_memory;
>      const MemoryRegionPortio *vga_ports, *vbe_ports;
>  
> -    s->global_vmstate = true;
>      if (!vga_common_init(s, OBJECT(dev), errp)) {
>          return;
>      }
> @@ -88,6 +88,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>      rom_add_vga(VGABIOS_FILENAME);
>  }
>  
> +static const VMStateDescription vmstate_vga_isa = {
> +    .name = "vga-isa",
> +    .version_id = 1,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_STRUCT(state, ISAVGAState, 0, vmstate_vga_common, VGACommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const Property vga_isa_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", ISAVGAState, state.vram_size_mb, 8),
>  };
> @@ -98,7 +107,7 @@ static void vga_isa_class_initfn(ObjectClass *klass, const void *data)
>  
>      dc->realize = vga_isa_realizefn;
>      device_class_set_legacy_reset(dc, vga_isa_reset);
> -    dc->vmsd = &vmstate_vga_common;
> +    dc->vmsd = &vmstate_vga_isa;
>      device_class_set_props(dc, vga_isa_properties);
>      set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
>  }

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Peter Maydell 1 week ago 
On Thu, 26 Mar 2026 at 11:35, Thomas Huth <thuth@redhat.com> wrote:
>
> From: Thomas Huth <thuth@redhat.com>
>
> QEMU currently crashes when migrating a guest that uses the
> isa-vga device as display. This happens because vga_isa_class_initfn()
> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
> But the isa-vga device is derived from ISADevice, not from VGACommonState,
> so the migration code tries to fill in the data for VGACommonState to
> the memory that is a ISADevice instead, which is of cause causing trouble.
>
> We need an indirection here as it's also e.g. done in vga-pci.c, so
> that the migration data gets filled into the right location.
>
> While we're at it, also drop the "global_vmstate = true" here. Since
> migration was broken for this device during the last 15 years (!) anyway,
> we don't have to worry about maintaining backward compatibility with this
> switch for older versions of QEMU anymore.

That means the only remaining users of global_vmstate = true are:
 * TYPE_VGA_MMIO : used only by the MIPS Jazz board, which is not
   versioned, so we are OK with migration compat breaks
 * TYPE_ISA_CIRRUS_VGA

I don't suppose that migration is also currently broken for Cirrus ? :-)

-- PMM
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Thomas Huth 1 week ago 
On 26/03/2026 12.59, Peter Maydell wrote:
> On Thu, 26 Mar 2026 at 11:35, Thomas Huth <thuth@redhat.com> wrote:
>>
>> From: Thomas Huth <thuth@redhat.com>
>>
>> QEMU currently crashes when migrating a guest that uses the
>> isa-vga device as display. This happens because vga_isa_class_initfn()
>> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
>> But the isa-vga device is derived from ISADevice, not from VGACommonState,
>> so the migration code tries to fill in the data for VGACommonState to
>> the memory that is a ISADevice instead, which is of cause causing trouble.
>>
>> We need an indirection here as it's also e.g. done in vga-pci.c, so
>> that the migration data gets filled into the right location.
>>
>> While we're at it, also drop the "global_vmstate = true" here. Since
>> migration was broken for this device during the last 15 years (!) anyway,
>> we don't have to worry about maintaining backward compatibility with this
>> switch for older versions of QEMU anymore.
> 
> That means the only remaining users of global_vmstate = true are:
>   * TYPE_VGA_MMIO : used only by the MIPS Jazz board, which is not
>     versioned, so we are OK with migration compat breaks
>   * TYPE_ISA_CIRRUS_VGA
> 
> I don't suppose that migration is also currently broken for Cirrus ? :-)

No, Cirrus seems to be fine. But I think we should likely do something like 
commit 1fcfdc435a3e25ab9037f6f7b8ab for the isa-cirrus-vga device, too, so 
we can at least get rid of this in a couple of years...

  Thomas
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Peter Maydell 1 week ago 
On Thu, 26 Mar 2026 at 12:04, Thomas Huth <thuth@redhat.com> wrote:
>
> On 26/03/2026 12.59, Peter Maydell wrote:
> > On Thu, 26 Mar 2026 at 11:35, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> From: Thomas Huth <thuth@redhat.com>
> >>
> >> QEMU currently crashes when migrating a guest that uses the
> >> isa-vga device as display. This happens because vga_isa_class_initfn()
> >> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
> >> But the isa-vga device is derived from ISADevice, not from VGACommonState,
> >> so the migration code tries to fill in the data for VGACommonState to
> >> the memory that is a ISADevice instead, which is of cause causing trouble.
> >>
> >> We need an indirection here as it's also e.g. done in vga-pci.c, so
> >> that the migration data gets filled into the right location.
> >>
> >> While we're at it, also drop the "global_vmstate = true" here. Since
> >> migration was broken for this device during the last 15 years (!) anyway,
> >> we don't have to worry about maintaining backward compatibility with this
> >> switch for older versions of QEMU anymore.
> >
> > That means the only remaining users of global_vmstate = true are:
> >   * TYPE_VGA_MMIO : used only by the MIPS Jazz board, which is not
> >     versioned, so we are OK with migration compat breaks
> >   * TYPE_ISA_CIRRUS_VGA
> >
> > I don't suppose that migration is also currently broken for Cirrus ? :-)
>
> No, Cirrus seems to be fine. But I think we should likely do something like
> commit 1fcfdc435a3e25ab9037f6f7b8ab for the isa-cirrus-vga device, too, so
> we can at least get rid of this in a couple of years...

If we're going to do that it would be nice to get that in for 11.0.

I do notice that the Cirrus devices have their own vmstate handle
saving and restoring all the common VGA state, instead of using
vmstate_vga_common, and they miss out all the vbe entries at
the bottom...

-- PMM
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Thomas Huth 1 week ago 
On 26/03/2026 15.57, Peter Maydell wrote:
> On Thu, 26 Mar 2026 at 12:04, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 26/03/2026 12.59, Peter Maydell wrote:
>>> On Thu, 26 Mar 2026 at 11:35, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> From: Thomas Huth <thuth@redhat.com>
>>>>
>>>> QEMU currently crashes when migrating a guest that uses the
>>>> isa-vga device as display. This happens because vga_isa_class_initfn()
>>>> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
>>>> But the isa-vga device is derived from ISADevice, not from VGACommonState,
>>>> so the migration code tries to fill in the data for VGACommonState to
>>>> the memory that is a ISADevice instead, which is of cause causing trouble.
>>>>
>>>> We need an indirection here as it's also e.g. done in vga-pci.c, so
>>>> that the migration data gets filled into the right location.
>>>>
>>>> While we're at it, also drop the "global_vmstate = true" here. Since
>>>> migration was broken for this device during the last 15 years (!) anyway,
>>>> we don't have to worry about maintaining backward compatibility with this
>>>> switch for older versions of QEMU anymore.
>>>
>>> That means the only remaining users of global_vmstate = true are:
>>>    * TYPE_VGA_MMIO : used only by the MIPS Jazz board, which is not
>>>      versioned, so we are OK with migration compat breaks
>>>    * TYPE_ISA_CIRRUS_VGA
>>>
>>> I don't suppose that migration is also currently broken for Cirrus ? :-)
>>
>> No, Cirrus seems to be fine. But I think we should likely do something like
>> commit 1fcfdc435a3e25ab9037f6f7b8ab for the isa-cirrus-vga device, too, so
>> we can at least get rid of this in a couple of years...
> 
> If we're going to do that it would be nice to get that in for 11.0.

I agree. Patch suggested here:

https://lore.kernel.org/qemu-devel/20260326154850.301609-1-thuth@redhat.com/

  Thomas
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by BALATON Zoltan 1 week ago 
On Thu, 26 Mar 2026, Peter Maydell wrote:
> On Thu, 26 Mar 2026 at 12:04, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 26/03/2026 12.59, Peter Maydell wrote:
>>> On Thu, 26 Mar 2026 at 11:35, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> From: Thomas Huth <thuth@redhat.com>
>>>>
>>>> QEMU currently crashes when migrating a guest that uses the
>>>> isa-vga device as display. This happens because vga_isa_class_initfn()
>>>> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
>>>> But the isa-vga device is derived from ISADevice, not from VGACommonState,
>>>> so the migration code tries to fill in the data for VGACommonState to
>>>> the memory that is a ISADevice instead, which is of cause causing trouble.
>>>>
>>>> We need an indirection here as it's also e.g. done in vga-pci.c, so
>>>> that the migration data gets filled into the right location.
>>>>
>>>> While we're at it, also drop the "global_vmstate = true" here. Since
>>>> migration was broken for this device during the last 15 years (!) anyway,
>>>> we don't have to worry about maintaining backward compatibility with this
>>>> switch for older versions of QEMU anymore.
>>>
>>> That means the only remaining users of global_vmstate = true are:
>>>   * TYPE_VGA_MMIO : used only by the MIPS Jazz board, which is not
>>>     versioned, so we are OK with migration compat breaks
>>>   * TYPE_ISA_CIRRUS_VGA
>>>
>>> I don't suppose that migration is also currently broken for Cirrus ? :-)
>>
>> No, Cirrus seems to be fine. But I think we should likely do something like
>> commit 1fcfdc435a3e25ab9037f6f7b8ab for the isa-cirrus-vga device, too, so
>> we can at least get rid of this in a couple of years...
>
> If we're going to do that it would be nice to get that in for 11.0.
>
> I do notice that the Cirrus devices have their own vmstate handle
> saving and restoring all the common VGA state, instead of using
> vmstate_vga_common, and they miss out all the vbe entries at
> the bottom...

That might be OK as cirrus-vga does not seem to support Bochs DISPI VBE 
ports so there should be nothing there to save/restore.

Regards,
BALATON Zoltan
Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device
Posted by Peter Maydell 1 week ago 
On Thu, 26 Mar 2026 at 15:17, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Thu, 26 Mar 2026, Peter Maydell wrote:
> > I do notice that the Cirrus devices have their own vmstate handle
> > saving and restoring all the common VGA state, instead of using
> > vmstate_vga_common, and they miss out all the vbe entries at
> > the bottom...
>
> That might be OK as cirrus-vga does not seem to support Bochs DISPI VBE
> ports so there should be nothing there to save/restore.

Mmm, I did wonder about that, but I couldn't see anything where
we disabled exposing those ports to the guest. But looking closer
the cirrus code never calls vga_init(), so it does indeed not
expose any of the VBE state to the guest and doesn't need to
migrate it.

-- PMM

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.