[Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support

Gerd Hoffmann posted 2 patches 7 years, 1 month ago
[Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support
Posted by Gerd Hoffmann 7 years, 1 month ago
So we have a boot display when using a vgpu as primary display.

ramfb depends on a fw_cfg file.  fw_cfg files can not be added and
removed at runtime, therefore a ramfb-enabled vfio device can't be
hotplugged.

Add a nohotplug variant of the vfio-pci device (as child class).  Add
the ramfb property to the nohotplug variant only.  So to enable the vgpu
display with boot support use this:

  -device vfio-pci-nohotplug,display=on,ramfb=on,sysfsdev=...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/vfio/pci.h                 |  1 +
 include/hw/vfio/vfio-common.h |  2 ++
 hw/vfio/display.c             | 12 ++++++++++++
 hw/vfio/pci.c                 | 25 +++++++++++++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 52b065421a..904e286586 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -149,6 +149,7 @@ typedef struct VFIOPCIDevice {
 #define VFIO_FEATURE_ENABLE_IGD_OPREGION \
                                 (1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
     OnOffAuto display;
+    bool enable_ramfb;
     int32_t bootindex;
     uint32_t igd_gms;
     OffAutoPCIBAR msix_relo;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 821def0565..0d85a0a6f8 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -26,6 +26,7 @@
 #include "qemu/queue.h"
 #include "qemu/notify.h"
 #include "ui/console.h"
+#include "hw/display/ramfb.h"
 #ifdef CONFIG_LINUX
 #include <linux/vfio.h>
 #endif
@@ -146,6 +147,7 @@ typedef struct VFIODMABuf {
 
 typedef struct VFIODisplay {
     QemuConsole *con;
+    RAMFBState *ramfb;
     struct {
         VFIORegion buffer;
         DisplaySurface *surface;
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 59c0e5d1d7..dead30e626 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -124,6 +124,9 @@ static void vfio_display_dmabuf_update(void *opaque)
 
     primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
     if (primary == NULL) {
+        if (dpy->ramfb) {
+            ramfb_display_update(dpy->con, dpy->ramfb);
+        }
         return;
     }
 
@@ -181,6 +184,9 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
     vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
                                           &vfio_display_dmabuf_ops,
                                           vdev);
+    if (vdev->enable_ramfb) {
+        vdev->dpy->ramfb = ramfb_setup(errp);
+    }
     return 0;
 }
 
@@ -228,6 +234,9 @@ static void vfio_display_region_update(void *opaque)
         return;
     }
     if (!plane.drm_format || !plane.size) {
+        if (dpy->ramfb) {
+            ramfb_display_update(dpy->con, dpy->ramfb);
+        }
         return;
     }
     format = qemu_drm_format_to_pixman(plane.drm_format);
@@ -300,6 +309,9 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
     vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
                                           &vfio_display_region_ops,
                                           vdev);
+    if (vdev->enable_ramfb) {
+        vdev->dpy->ramfb = ramfb_setup(errp);
+    }
     return 0;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 866f0deeb7..a0047f4942 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3067,6 +3067,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
             goto out_teardown;
         }
     }
+    if (vdev->enable_ramfb && vdev->dpy == NULL) {
+        error_setg(errp, "ramfb=on requires display=on");
+        goto out_teardown;
+    }
 
     vfio_register_err_notifier(vdev);
     vfio_register_req_notifier(vdev);
@@ -3258,9 +3262,30 @@ static const TypeInfo vfio_pci_dev_info = {
     },
 };
 
+static Property vfio_pci_dev_nohotplug_properties[] = {
+    DEFINE_PROP_BOOL("ramfb", VFIOPCIDevice, enable_ramfb, false),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vfio_pci_nohotplug_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = vfio_pci_dev_nohotplug_properties;
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo vfio_pci_nohotplug_dev_info = {
+    .name = "vfio-pci-nohotplug",
+    .parent = "vfio-pci",
+    .instance_size = sizeof(VFIOPCIDevice),
+    .class_init = vfio_pci_nohotplug_dev_class_init,
+};
+
 static void register_vfio_pci_dev_type(void)
 {
     type_register_static(&vfio_pci_dev_info);
+    type_register_static(&vfio_pci_nohotplug_dev_info);
 }
 
 type_init(register_vfio_pci_dev_type)
-- 
2.9.3


Re: [Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support
Posted by Alex Williamson 7 years ago
On Mon, 17 Sep 2018 08:17:29 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> So we have a boot display when using a vgpu as primary display.
> 
> ramfb depends on a fw_cfg file.  fw_cfg files can not be added and
> removed at runtime, therefore a ramfb-enabled vfio device can't be
> hotplugged.
> 
> Add a nohotplug variant of the vfio-pci device (as child class).  Add
> the ramfb property to the nohotplug variant only.  So to enable the vgpu
> display with boot support use this:
> 
>   -device vfio-pci-nohotplug,display=on,ramfb=on,sysfsdev=...
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/vfio/pci.h                 |  1 +
>  include/hw/vfio/vfio-common.h |  2 ++
>  hw/vfio/display.c             | 12 ++++++++++++
>  hw/vfio/pci.c                 | 25 +++++++++++++++++++++++++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 52b065421a..904e286586 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -149,6 +149,7 @@ typedef struct VFIOPCIDevice {
>  #define VFIO_FEATURE_ENABLE_IGD_OPREGION \
>                                  (1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
>      OnOffAuto display;
> +    bool enable_ramfb;
>      int32_t bootindex;
>      uint32_t igd_gms;
>      OffAutoPCIBAR msix_relo;

Hi Gerd,

One tiny nit here, we can move this new bool down in the struct with
the rest of the bools for better alignment.  I can change that on
commit.  However, I'm not having luck getting ramfb to work; the
display is only getting initialized once the guest driver loads.  This
is a 440FX/SeaBIOS VM, it looks like you've already updated bios.bin in
qemu.git with ramfb support, but I also see the same results with
bios.bin from your seabios.git package.  I'm using libvirt to launch
the guest with a wrapper around qemu-system-x86_64 to replace vfio-pci
with vfio-pci-nohotplug, resulting in a command line including:

-bios /usr/share/seabios.git/bios.bin \
-spice port=0,disable-ticketing,gl=on,rendernode=/dev/dri/by-path/pci-0000:00:02.0-render,seamless-migration=on \
-device vfio-pci-nohotplug,id=hostdev0,sysfsdev=...,display=on,bus=pci.0,addr=0x9 \
-set device.hostdev0.x-igd-opregion=on \
-set device.hostdev0.ramfb=on \

Relevant XML blobs:

    <graphics type='spice'>
      <listen type='none'/>
      <gl enable='yes' rendernode='/dev/dri/by-path/pci-0000:00:02.0-render'/>
    </graphics>
    <video>
      <model type='none'/>
    </video>
    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci' display='on'>
      <source>
        <address uuid='cd4fa69f-c24c-476f-a61d-abca705e2a13'/>
      </source>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </hostdev>
...
  <qemu:commandline>
    <qemu:arg value='-set'/>
    <qemu:arg value='device.hostdev0.x-igd-opregion=on'/>
    <qemu:arg value='-set'/>
    <qemu:arg value='device.hostdev0.ramfb=on'/>
  </qemu:commandline>

This is a Windows 10 VM, but as I understand this ramfb support, I
think I'm still supposed to see SeaBIOS boot messages and perhaps even
the Windows boot animation before the guest driver takes over, is that
correct?  What am I missing?  Thanks,

Alex

Re: [Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support
Posted by Gerd Hoffmann 7 years ago
  Hi,

> >      OnOffAuto display;
> > +    bool enable_ramfb;
> >      int32_t bootindex;
> >      uint32_t igd_gms;
> >      OffAutoPCIBAR msix_relo;
> 
> Hi Gerd,
> 
> One tiny nit here, we can move this new bool down in the struct with
> the rest of the bools for better alignment.  I can change that on
> commit.

I've grouped it with the display option because it is display related
too, but if you prefer to group the bools instead this is fine with me.

> However, I'm not having luck getting ramfb to work; the
> display is only getting initialized once the guest driver loads.  This
> is a 440FX/SeaBIOS VM, it looks like you've already updated bios.bin in
> qemu.git with ramfb support,

Yes, seabios (and vgabios) bundled with 3.0 (and master branch) should work fine.

There is a standalone device you can use for testing (-vga none -device
ramfb), to see whenever the firmware side of things works correctly.

OVMF has ramfb support too btw (merged a few months back).

> -device vfio-pci-nohotplug,id=hostdev0,sysfsdev=...,display=on,bus=pci.0,addr=0x9 \

>     <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci' display='on'>
>       <source>
>         <address uuid='cd4fa69f-c24c-476f-a61d-abca705e2a13'/>
>       </source>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
>     </hostdev>

Hmm, that actually uses vfio-pci-nohotplug?
But when ramfb=on doesn't throw an error, then yes, appearently.

>   <qemu:commandline>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.x-igd-opregion=on'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.ramfb=on'/>
>   </qemu:commandline>

I have this (additionally):

    <qemu:arg value='-set'/>
    <qemu:arg value='device.hostdev0.driver=vfio-pci-nohotplug'/>

> This is a Windows 10 VM, but as I understand this ramfb support, I
> think I'm still supposed to see SeaBIOS boot messages and perhaps even
> the Windows boot animation before the guest driver takes over, is that
> correct?

Yes, you should see both.

cheers,
  Gerd


Re: [Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support
Posted by Alex Williamson 7 years ago
On Fri, 12 Oct 2018 10:43:02 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > >      OnOffAuto display;
> > > +    bool enable_ramfb;
> > >      int32_t bootindex;
> > >      uint32_t igd_gms;
> > >      OffAutoPCIBAR msix_relo;  
> > 
> > Hi Gerd,
> > 
> > One tiny nit here, we can move this new bool down in the struct with
> > the rest of the bools for better alignment.  I can change that on
> > commit.  
> 
> I've grouped it with the display option because it is display related
> too, but if you prefer to group the bools instead this is fine with me.

Not so much grouping the bools as simply making the struct a bit more
space efficient by not adding obvious alignment holes.

> > However, I'm not having luck getting ramfb to work; the
> > display is only getting initialized once the guest driver loads.  This
> > is a 440FX/SeaBIOS VM, it looks like you've already updated bios.bin in
> > qemu.git with ramfb support,  
> 
> Yes, seabios (and vgabios) bundled with 3.0 (and master branch) should work fine.
> 
> There is a standalone device you can use for testing (-vga none -device
> ramfb), to see whenever the firmware side of things works correctly.
> 
> OVMF has ramfb support too btw (merged a few months back).

Yes, I'm also trying with an OVMF build from your firmware repo.

> > -device vfio-pci-nohotplug,id=hostdev0,sysfsdev=...,display=on,bus=pci.0,addr=0x9 \  
> 
> >     <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci' display='on'>
> >       <source>
> >         <address uuid='cd4fa69f-c24c-476f-a61d-abca705e2a13'/>
> >       </source>
> >       <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
> >     </hostdev>  
> 
> Hmm, that actually uses vfio-pci-nohotplug?
> But when ramfb=on doesn't throw an error, then yes, appearently.

That's where my wrapper script was replacing
s/vfio-pci/vfio-pci-nohotplug/, I didn't know about the driver override
you list below, that's much cleaner.
 
> >   <qemu:commandline>
> >     <qemu:arg value='-set'/>
> >     <qemu:arg value='device.hostdev0.x-igd-opregion=on'/>
> >     <qemu:arg value='-set'/>
> >     <qemu:arg value='device.hostdev0.ramfb=on'/>
> >   </qemu:commandline>  
> 
> I have this (additionally):
> 
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.driver=vfio-pci-nohotplug'/>
> 
> > This is a Windows 10 VM, but as I understand this ramfb support, I
> > think I'm still supposed to see SeaBIOS boot messages and perhaps even
> > the Windows boot animation before the guest driver takes over, is that
> > correct?  
> 
> Yes, you should see both.

Ok, I'll update my xml and also try the raw ramfb device and see if I
can make it work.  Thanks,

Alex