On 10/2/23 13:11, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > RAMFB migration was unsupported until now, let's make it conditional. > The following patch will prevent machines <= 8.1 to migrate it. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > hw/vfio/pci.h | 1 + > include/hw/display/ramfb.h | 2 +- > hw/display/ramfb-standalone.c | 8 +++++++- > hw/display/ramfb.c | 6 ++++-- > hw/vfio/display.c | 4 ++-- > hw/vfio/pci.c | 1 + > stubs/ramfb.c | 2 +- > 7 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > index 2d836093a8..671cc78912 100644 > --- a/hw/vfio/pci.h > +++ b/hw/vfio/pci.h > @@ -156,6 +156,7 @@ struct VFIOPCIDevice { > OnOffAuto display; > uint32_t display_xres; > uint32_t display_yres; > + bool ramfb_migrate; > int32_t bootindex; > uint32_t igd_gms; > OffAutoPCIBAR msix_relo; > diff --git a/include/hw/display/ramfb.h b/include/hw/display/ramfb.h > index b33a2c467b..40063b62bd 100644 > --- a/include/hw/display/ramfb.h > +++ b/include/hw/display/ramfb.h > @@ -4,7 +4,7 @@ > /* ramfb.c */ > typedef struct RAMFBState RAMFBState; > void ramfb_display_update(QemuConsole *con, RAMFBState *s); > -RAMFBState *ramfb_setup(Error **errp); > +RAMFBState *ramfb_setup(bool migrate, Error **errp); > > /* ramfb-standalone.c */ > #define TYPE_RAMFB_DEVICE "ramfb" > diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c > index 8c0094397f..6bbd69ccdf 100644 > --- a/hw/display/ramfb-standalone.c > +++ b/hw/display/ramfb-standalone.c > @@ -15,6 +15,7 @@ struct RAMFBStandaloneState { > SysBusDevice parent_obj; > QemuConsole *con; > RAMFBState *state; > + bool migrate; > }; > > static void display_update_wrapper(void *dev) > @@ -37,9 +38,13 @@ static void ramfb_realizefn(DeviceState *dev, Error **errp) > RAMFBStandaloneState *ramfb = RAMFB(dev); > > ramfb->con = graphic_console_init(dev, 0, &wrapper_ops, dev); > - ramfb->state = ramfb_setup(errp); > + ramfb->state = ramfb_setup(ramfb->migrate, errp); > } > > +static Property ramfb_properties[] = { > + DEFINE_PROP_BOOL("migrate", RAMFBStandaloneState, migrate, true), > + DEFINE_PROP_END_OF_LIST(), > +}; > static void ramfb_class_initfn(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -48,6 +53,7 @@ static void ramfb_class_initfn(ObjectClass *klass, void > *data) > dc->realize = ramfb_realizefn; > dc->desc = "ram framebuffer standalone device"; > dc->user_creatable = true; > + device_class_set_props(dc, ramfb_properties); > } > > static const TypeInfo ramfb_info = { > diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c > index 4aaaa7d653..73e08d605f 100644 > --- a/hw/display/ramfb.c > +++ b/hw/display/ramfb.c > @@ -135,7 +135,7 @@ static const VMStateDescription vmstate_ramfb = { > } > }; > > -RAMFBState *ramfb_setup(Error **errp) > +RAMFBState *ramfb_setup(bool migrate, Error **errp) > { > FWCfgState *fw_cfg = fw_cfg_find(); > RAMFBState *s; > @@ -147,7 +147,9 @@ RAMFBState *ramfb_setup(Error **errp) > > s = g_new0(RAMFBState, 1); > > - vmstate_register(NULL, 0, &vmstate_ramfb, s); > + if (migrate) { > + vmstate_register(NULL, 0, &vmstate_ramfb, s); > + } > rom_add_vga("vgabios-ramfb.bin"); > fw_cfg_add_file_callback(fw_cfg, "etc/ramfb", > NULL, ramfb_fw_cfg_write, s, > diff --git a/hw/vfio/display.c b/hw/vfio/display.c > index bec864f482..3f6b251ccd 100644 > --- a/hw/vfio/display.c > +++ b/hw/vfio/display.c > @@ -356,7 +356,7 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, > Error **errp) > &vfio_display_dmabuf_ops, > vdev); > if (vdev->enable_ramfb) { > - vdev->dpy->ramfb = ramfb_setup(errp); > + vdev->dpy->ramfb = ramfb_setup(vdev->ramfb_migrate, errp); > } > vfio_display_edid_init(vdev); > return 0; > @@ -483,7 +483,7 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev, > Error **errp) > &vfio_display_region_ops, > vdev); > if (vdev->enable_ramfb) { > - vdev->dpy->ramfb = ramfb_setup(errp); > + vdev->dpy->ramfb = ramfb_setup(vdev->ramfb_migrate, errp); > } > return 0; > } > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 3b2ca3c24c..6575b8f32d 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3484,6 +3484,7 @@ static const TypeInfo vfio_pci_dev_info = { > > static Property vfio_pci_dev_nohotplug_properties[] = { > DEFINE_PROP_BOOL("ramfb", VFIOPCIDevice, enable_ramfb, false), > + DEFINE_PROP_BOOL("ramfb-migrate", VFIOPCIDevice, ramfb_migrate, true), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/stubs/ramfb.c b/stubs/ramfb.c > index 48143f3354..8869a5db09 100644 > --- a/stubs/ramfb.c > +++ b/stubs/ramfb.c > @@ -6,7 +6,7 @@ void ramfb_display_update(QemuConsole *con, RAMFBState *s) > { > } > > -RAMFBState *ramfb_setup(Error **errp) > +RAMFBState *ramfb_setup(bool migrate, Error **errp) > { > error_setg(errp, "ramfb support not available"); > return NULL;
I'm not a seasoned migration reviewer, so -- apologies for the churn -- I'd prefer if this patch were split in three: - First patch: ramfb_setup() update. All callers pass in constant "false". Stub updated. - Second patch: new property for vfio-pci-nohotplug, hooked up to ramfb_setup() for real. - Third patch: new property for ramfb-standalone, hooked up to ramfb_setup() for real. Without this separation, the hunks are just heaped together in the patch, and I'm having a hard time understanding the data flow. Again, apologies that I'm requesting this for a very small patch. Thanks, Laszlo