[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 3/3] hw/vfio: add ramfb migration support
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH v5 3/3] hw/vfio: add ramfb migration support |
Date: |
Tue, 10 Oct 2023 11:08:34 +0400 |
Hi
On Tue, Oct 10, 2023 at 11:03 AM Cédric Le Goater <clg@redhat.com> wrote:
>
> On 10/9/23 12:19, Laszlo Ersek wrote:
> > On 10/9/23 08:32, marcandre.lureau@redhat.com wrote:
> >> From: Marc-André Lureau <marcandre.lureau@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.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >> hw/vfio/pci.h | 3 +++
> >> hw/core/machine.c | 1 +
> >> hw/vfio/display.c | 21 +++++++++++++++++++++
> >> hw/vfio/pci.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >> stubs/ramfb.c | 2 ++
> >> 5 files changed, 71 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 6305f2d7a4..05aef2cf9f 100644
> >> --- a/hw/core/machine.c
> >> +++ b/hw/core/machine.c
> >> @@ -35,6 +35,7 @@
> >> GlobalProperty hw_compat_8_1[] = {
> >> { TYPE_PCI_BRIDGE, "x-pci-express-writeable-slt-bug", "true" },
> >> { "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..2739ba56ec 100644
> >> --- a/hw/vfio/display.c
> >> +++ b/hw/vfio/display.c
> >> @@ -542,3 +542,24 @@ void vfio_display_finalize(VFIOPCIDevice *vdev)
> >> vfio_display_edid_exit(vdev->dpy);
> >> g_free(vdev->dpy);
> >> }
> >> +
> >> +static bool migrate_needed(void *opaque)
> >> +{
> >> + VFIODisplay *dpy = opaque;
> >> + bool ramfb_exists = dpy->ramfb != NULL;
> >> +
> >> + /* see vfio_display_migration_needed() */
> >> + assert(ramfb_exists);
> >> + return ramfb_exists;
> >> +}
> >> +
> >> +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),
> >> + VMSTATE_END_OF_LIST(),
> >> + }
> >> +};
> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> >> index 3b2ca3c24c..e44ed21180 100644
> >> --- a/hw/vfio/pci.c
> >> +++ b/hw/vfio/pci.c
> >> @@ -2608,6 +2608,32 @@ static bool vfio_msix_present(void *opaque, int
> >> version_id)
> >> return msix_present(pdev);
> >> }
> >>
> >> +static bool vfio_display_migration_needed(void *opaque)
> >> +{
> >> + VFIOPCIDevice *vdev = opaque;
> >> +
> >> + /*
> >> + * We need to migrate the VFIODisplay object if ramfb *migration* was
> >> + * explicitly requested (in which case we enforced both ramfb=on and
> >> + * display=on), or ramfb migration was left at the default "auto"
> >> + * setting, and *ramfb* was explicitly requested (in which case we
> >> + * enforced display=on).
> >> + */
> >> + return vdev->ramfb_migrate == ON_OFF_AUTO_ON ||
> >> + (vdev->ramfb_migrate == ON_OFF_AUTO_AUTO && vdev->enable_ramfb);
> >> +}
> >> +
> >> +const VMStateDescription vmstate_vfio_display = {
> >> + .name = "VFIOPCIDevice/VFIODisplay",
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> + .needed = vfio_display_migration_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 +2642,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
> >> }
> >> };
> >>
> >> @@ -3271,6 +3301,19 @@ static void vfio_realize(PCIDevice *pdev, Error
> >> **errp)
> >> }
> >> }
> >>
> >> + if (vdev->ramfb_migrate == ON_OFF_AUTO_ON && !vdev->enable_ramfb) {
> >> + warn_report("x-ramfb-migrate=on but ramfb=off");
> >> + vdev->ramfb_migrate = ON_OFF_AUTO_OFF;
> >
> > the warning could give a hint about the resultant action taken (i.e.,
> > forcing off x-ramfb-migrate), but don't repost just for that; it's a nit.
>
> yes.
>
> How about :
>
> warn_report("x-ramfb-migrate=on but ramfb=off. Forcing
> x-ramfb-migrate to off.");
Sure, that's better.
>
> I can amend the patch if you agree.
thanks!