From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Implement RAMFB migration, and add properties to enable it only on >= 8.2
machines, + a few related cleanups.

Cedric, did you get the chance to test the VFIO display/ramfb code?

thanks

v4: (Laszlo review and suggestions)
- change migrate_needed() to assert(ramfb_exists)
- rename vfio_display_needed() to vfio_display_migration_needed(),
  update the condition and associated comment
- move the ramfb-migrate option check and add a check for ramfb=on
- add a stub to fix compilation on some architectures

v3:
- add a "x-" prefix to properties, as they are not meant for users.
- RAMFB now exports a ramfb_vmstate for actual devices to include
- VFIOPCIDevice now has a VFIODisplay optional subsection whenever ramfb
  migration is required (untested)

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1859424

Marc-André Lureau (3):
  ramfb: add migration support
  ramfb-standalone: add migration support
  hw/vfio: add ramfb migration support

 hw/vfio/pci.h                 |  3 +++
 include/hw/display/ramfb.h    |  4 ++++
 hw/core/machine.c             |  5 +++-
 hw/display/ramfb-standalone.c | 27 +++++++++++++++++++++
 hw/display/ramfb.c            | 19 +++++++++++++++
 hw/vfio/display.c             | 20 ++++++++++++++++
 hw/vfio/pci.c                 | 44 +++++++++++++++++++++++++++++++++++
 stubs/ramfb.c                 |  2 ++
 8 files changed, 123 insertions(+), 1 deletion(-)

-- 
2.41.0

On 10/5/23 13:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> Implement RAMFB migration, and add properties to enable it only on >= 8.2
> machines, + a few related cleanups.
> 
> Cedric, did you get the chance to test the VFIO display/ramfb code?

Done with a :

GPU 00000000:00:03.0
     Product Name                          : GRID RTX6000-1Q
     Product Brand                         : NVIDIA RTX Virtual Workstation


rhle9+gdm was running and it looks pretty good after migration.

I would like to go through the models now (I really don't like the
VFIODisplay attributes being in VFIOPCIDevice, it's ugly)

Thanks,

C.


> 
> thanks
> 
> v4: (Laszlo review and suggestions)
> - change migrate_needed() to assert(ramfb_exists)
> - rename vfio_display_needed() to vfio_display_migration_needed(),
>    update the condition and associated comment
> - move the ramfb-migrate option check and add a check for ramfb=on
> - add a stub to fix compilation on some architectures
> 
> v3:
> - add a "x-" prefix to properties, as they are not meant for users.
> - RAMFB now exports a ramfb_vmstate for actual devices to include
> - VFIOPCIDevice now has a VFIODisplay optional subsection whenever ramfb
>    migration is required (untested)
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1859424
> 
> Marc-André Lureau (3):
>    ramfb: add migration support
>    ramfb-standalone: add migration support
>    hw/vfio: add ramfb migration support
> 
>   hw/vfio/pci.h                 |  3 +++
>   include/hw/display/ramfb.h    |  4 ++++
>   hw/core/machine.c             |  5 +++-
>   hw/display/ramfb-standalone.c | 27 +++++++++++++++++++++
>   hw/display/ramfb.c            | 19 +++++++++++++++
>   hw/vfio/display.c             | 20 ++++++++++++++++
>   hw/vfio/pci.c                 | 44 +++++++++++++++++++++++++++++++++++
>   stubs/ramfb.c                 |  2 ++
>   8 files changed, 123 insertions(+), 1 deletion(-)
> 

On 10/5/23 13:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> Implement RAMFB migration, and add properties to enable it only on >= 8.2
> machines, + a few related cleanups.
> 
> Cedric, did you get the chance to test the VFIO display/ramfb code?

Nope. I was busy with VFIO stuff. I haven't even read Laszlo's
email yet. I will try this or next week.

That said, could we avoid adding another migration property in
VFIOPCIDevice and use the available "enable-migration" ?

C.


> thanks
> 
> v4: (Laszlo review and suggestions)
> - change migrate_needed() to assert(ramfb_exists)
> - rename vfio_display_needed() to vfio_display_migration_needed(),
>    update the condition and associated comment
> - move the ramfb-migrate option check and add a check for ramfb=on
> - add a stub to fix compilation on some architectures
> 
> v3:
> - add a "x-" prefix to properties, as they are not meant for users.
> - RAMFB now exports a ramfb_vmstate for actual devices to include
> - VFIOPCIDevice now has a VFIODisplay optional subsection whenever ramfb
>    migration is required (untested)
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1859424
> 
> Marc-André Lureau (3):
>    ramfb: add migration support
>    ramfb-standalone: add migration support
>    hw/vfio: add ramfb migration support
> 
>   hw/vfio/pci.h                 |  3 +++
>   include/hw/display/ramfb.h    |  4 ++++
>   hw/core/machine.c             |  5 +++-
>   hw/display/ramfb-standalone.c | 27 +++++++++++++++++++++
>   hw/display/ramfb.c            | 19 +++++++++++++++
>   hw/vfio/display.c             | 20 ++++++++++++++++
>   hw/vfio/pci.c                 | 44 +++++++++++++++++++++++++++++++++++
>   stubs/ramfb.c                 |  2 ++
>   8 files changed, 123 insertions(+), 1 deletion(-)
> 

On 10/5/23 14:01, Cédric Le Goater wrote:
> On 10/5/23 13:30, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> Implement RAMFB migration, and add properties to enable it only on >= 8.2
>> machines, + a few related cleanups.
>>
>> Cedric, did you get the chance to test the VFIO display/ramfb code?
> 
> Nope. I was busy with VFIO stuff. I haven't even read Laszlo's
> email yet. I will try this or next week.
> 
> That said, could we avoid adding another migration property in
> VFIOPCIDevice and use the available "enable-migration" ?

I'm not entirely sure, but I suspect we can't / shouldn't do that.
"x-ramfb-migrate" is effectively a machine type compat prop, so if it
doesn't *precisely* line up with enable-migration (i.e., if they aren't
equivalent), then we shouldn't merge them. AFAICT, a 8.1 machine type
may have "enable-migration" set, but it should still have
"x-ramfb-migrate" clear.

Laszlo

On 10/5/23 16:16, Laszlo Ersek wrote:
> On 10/5/23 14:01, Cédric Le Goater wrote:
>> On 10/5/23 13:30, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> Hi,
>>>
>>> Implement RAMFB migration, and add properties to enable it only on >= 8.2
>>> machines, + a few related cleanups.
>>>
>>> Cedric, did you get the chance to test the VFIO display/ramfb code?
>>
>> Nope. I was busy with VFIO stuff. I haven't even read Laszlo's
>> email yet. I will try this or next week.
>>
>> That said, could we avoid adding another migration property in
>> VFIOPCIDevice and use the available "enable-migration" ?
> 
> I'm not entirely sure, but I suspect we can't / shouldn't do that.
> "x-ramfb-migrate" is effectively a machine type compat prop, so if it
> doesn't *precisely* line up with enable-migration (i.e., if they aren't
> equivalent), then we shouldn't merge them. AFAICT, a 8.1 machine type
> may have "enable-migration" set, but it should still have
> "x-ramfb-migrate" clear.

or more precisely, not clear, but "auto".

Laszlo