Hi
On Tue, Oct 3, 2023 at 2:17 PM Cédric Le Goater <c...@redhat.com> wrote:
>
> On 10/3/23 10:56, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Add a "VFIODisplay" subsection whenever "x-ramfb-migrate" is turned on.
> >
> > Turn it off by default on machines <= 8.1 for compatibility reasons.
>
>
> This change breaks linking on various platforms with :
>
> /usr/bin/ld:
> libqemu-xtensa-softmmu.fa.p/hw_vfio_display.c.o:(.data.rel+0x50): undefined
> reference to `ramfb_vmstate'
>
> Some stubs updates are missing it seems..
>
diff --git a/stubs/ramfb.c b/stubs/ramfb.c
index 48143f3354..cf64733b10 100644
--- a/stubs/ramfb.c
+++ b/stubs/ramfb.c
@@ -2,6 +2,8 @@
#include "qapi/error.h"
#include "hw/display/ramfb.h"
+const VMStateDescription ramfb_vmstate = {};
+
And I think we should also change the "needed" condition to:
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4689f2e5c1..b327844764 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2613,7 +2613,7 @@ static bool vfio_display_needed(void *opaque)
VFIOPCIDevice *vdev = opaque;
/* the only thing that justifies the VFIODisplay sub-section atm */
- return vdev->ramfb_migrate != ON_OFF_AUTO_OFF;
+ return vdev->enable_ramfb && vdev->ramfb_migrate != ON_OFF_AUTO_OFF;
}
> Thanks,
>
> C.
>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > hw/vfio/pci.h | 3 +++
> > hw/core/machine.c | 1 +
> > hw/vfio/display.c | 23 +++++++++++++++++++++++
> > hw/vfio/pci.c | 32 ++++++++++++++++++++++++++++++++
> > 4 files changed, 59 insertions(+)
> >
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> > index 2d836093a8..fd06695542 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -173,6 +173,7 @@ struct VFIOPCIDevice {
> > bool no_kvm_ioeventfd;
> > bool no_vfio_ioeventfd;
> > bool enable_ramfb;
> > + OnOffAuto ramfb_migrate;
> > bool defer_kvm_irq_routing;
> > bool clear_parent_atomics_on_exit;
> > VFIODisplay *dpy;
> > @@ -226,4 +227,6 @@ void vfio_display_reset(VFIOPCIDevice *vdev);
> > int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
> > void vfio_display_finalize(VFIOPCIDevice *vdev);
> >
> > +extern const VMStateDescription vfio_display_vmstate;
> > +
> > #endif /* HW_VFIO_VFIO_PCI_H */
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 47a07d1d9b..f2f8940a85 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -32,6 +32,7 @@
> >
> > GlobalProperty hw_compat_8_1[] = {
> > { "ramfb", "x-migrate", "off" },
> > + { "vfio-pci-nohotplug", "x-ramfb-migrate", "off" }
> > };
> > const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1);
> >
> > diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> > index bec864f482..de5bf71dd1 100644
> > --- a/hw/vfio/display.c
> > +++ b/hw/vfio/display.c
> > @@ -542,3 +542,26 @@ void vfio_display_finalize(VFIOPCIDevice *vdev)
> > vfio_display_edid_exit(vdev->dpy);
> > g_free(vdev->dpy);
> > }
> > +
> > +static bool migrate_needed(void *opaque)
> > +{
> > + /*
> > + * If we are here, it's because vfio_display_needed(), which is only
> > true
> > + * when dpy->ramfb_migrate atm.
> > + *
> > + * If the migration condition is changed, we should check here if
> > + * ramfb_migrate is true. (this will need a way to lookup the
> > associated
> > + * VFIOPCIDevice somehow, or fields to be moved, ..)
> > + */
> > + return true;
> > +}
> > +
> > +const VMStateDescription vfio_display_vmstate = {
> > + .name = "VFIODisplay",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .needed = migrate_needed,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_STRUCT_POINTER(ramfb, VFIODisplay, ramfb_vmstate,
> > RAMFBState),
> > + }
> > +};
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 3b2ca3c24c..4689f2e5c1 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -2608,6 +2608,25 @@ static bool vfio_msix_present(void *opaque, int
> > version_id)
> > return msix_present(pdev);
> > }
> >
> > +static bool vfio_display_needed(void *opaque)
> > +{
> > + VFIOPCIDevice *vdev = opaque;
> > +
> > + /* the only thing that justifies the VFIODisplay sub-section atm */
> > + return vdev->ramfb_migrate != ON_OFF_AUTO_OFF;
> > +}
> > +
> > +const VMStateDescription vmstate_vfio_display = {
> > + .name = "VFIOPCIDevice/VFIODisplay",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .needed = vfio_display_needed,
> > + .fields = (VMStateField[]){
> > + VMSTATE_STRUCT_POINTER(dpy, VFIOPCIDevice, vfio_display_vmstate,
> > VFIODisplay),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > const VMStateDescription vmstate_vfio_pci_config = {
> > .name = "VFIOPCIDevice",
> > .version_id = 1,
> > @@ -2616,6 +2635,10 @@ const VMStateDescription vmstate_vfio_pci_config = {
> > VMSTATE_PCI_DEVICE(pdev, VFIOPCIDevice),
> > VMSTATE_MSIX_TEST(pdev, VFIOPCIDevice, vfio_msix_present),
> > VMSTATE_END_OF_LIST()
> > + },
> > + .subsections = (const VMStateDescription*[]) {
> > + &vmstate_vfio_display,
> > + NULL
> > }
> > };
> >
> > @@ -3275,6 +3298,14 @@ static void vfio_realize(PCIDevice *pdev, Error
> > **errp)
> > if (!vfio_migration_realize(vbasedev, errp)) {
> > goto out_deregister;
> > }
> > + if (vbasedev->enable_migration == ON_OFF_AUTO_OFF) {
> > + if (vdev->ramfb_migrate == ON_OFF_AUTO_AUTO) {
> > + vdev->ramfb_migrate = ON_OFF_AUTO_OFF;
> > + } else if (vdev->ramfb_migrate == ON_OFF_AUTO_ON) {
> > + error_setg(errp, "x-ramfb-migrate requires migration");
> > + goto out_deregister;
> > + }
> > + }
> > }
> >
> > vfio_register_err_notifier(vdev);
> > @@ -3484,6 +3515,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_ON_OFF_AUTO("x-ramfb-migrate", VFIOPCIDevice,
> > ramfb_migrate, ON_OFF_AUTO_AUTO),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
>
>
--
Marc-André Lureau
