[Qemu-devel] [PATCH v4 0/3] ramfb: simple boot framebuffer

[Qemu-devel] [PATCH v4 0/3] ramfb: simple boot framebuffer

[Gerd Hoffmann]

  Hi,

So, the first ramfb bits should be ready for merge.  This series
includes the ramfb core support bits, the ramfb standalone device
and vfio-pci-ramfb device for vgpu boot display support.

If you want play with it I recommend getting the bits from

	https://www.kraxel.org/cgit/qemu/log/?h=sirius/ramfb

because they come with an updated seabios and a new vgabios rom and an
experimental OVMF build.  Firmware patches are here:

	https://www.kraxe.org/cgit/seabios/log/?h=ramfb
	https://github.com/kraxel/edk2/commits/ramfb

They should land upstream soon.

cheers,
  Gerd

Gerd Hoffmann (3):
  hw/display: add ramfb, a simple boot framebuffer living in guest ram
  hw/display: add standalone ramfb device
  hw/vfio/display: add ramfb support

 include/hw/display/ramfb.h    | 12 ++++++
 include/hw/vfio/vfio-common.h |  2 +
 hw/arm/sysbus-fdt.c           |  7 ++++
 hw/arm/virt.c                 |  2 +
 hw/display/ramfb-standalone.c | 62 ++++++++++++++++++++++++++++
 hw/display/ramfb.c            | 95 +++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc_piix.c             |  2 +
 hw/i386/pc_q35.c              |  2 +
 hw/vfio/display.c             | 10 +++++
 hw/vfio/pci.c                 | 15 +++++++
 hw/display/Makefile.objs      |  3 ++
 11 files changed, 212 insertions(+)
 create mode 100644 include/hw/display/ramfb.h
 create mode 100644 hw/display/ramfb-standalone.c
 create mode 100644 hw/display/ramfb.c

-- 
2.9.3

[Qemu-devel] [PATCH v4 1/3] hw/display: add ramfb, a simple boot framebuffer living in guest ram

[Gerd Hoffmann]

The boot framebuffer is expected to be configured by the firmware, so it
uses fw_cfg as interface.  Initialization goes as follows:

  (1) Check whenever etc/ramfb is present.
  (2) Allocate framebuffer from RAM.
  (3) Fill struct RAMFBCfg, write it to etc/ramfb.

Done.  You can write stuff to the framebuffer now, and it should appear
automagically on the screen.

Note that this isn't very efficient because it does a full display
update on each refresh.  No dirty tracking.  Dirty tracking would have
to be active for the whole ram slot, so that wouldn't be very efficient
either.  For a boot display which is active for a short time only this
isn't a big deal.  As permanent guest display something better should be
used (if possible).

This is the ramfb core code.  Some windup is needed for display devices
which want have a ramfb boot display.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/display/ramfb.h |  9 +++++
 hw/display/ramfb.c         | 95 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/display/Makefile.objs   |  2 +
 3 files changed, 106 insertions(+)
 create mode 100644 include/hw/display/ramfb.h
 create mode 100644 hw/display/ramfb.c

diff --git a/include/hw/display/ramfb.h b/include/hw/display/ramfb.h
new file mode 100644
index 0000000000..a3d4c79942
--- /dev/null
+++ b/include/hw/display/ramfb.h
@@ -0,0 +1,9 @@
+#ifndef RAMFB_H
+#define RAMFB_H
+
+/* ramfb.c */
+typedef struct RAMFBState RAMFBState;
+void ramfb_display_update(QemuConsole *con, RAMFBState *s);
+RAMFBState *ramfb_setup(Error **errp);
+
+#endif /* RAMFB_H */
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
new file mode 100644
index 0000000000..6867bce8ae
--- /dev/null
+++ b/hw/display/ramfb.c
@@ -0,0 +1,95 @@
+/*
+ * early boot framebuffer in guest ram
+ * configured using fw_cfg
+ *
+ * Copyright Red Hat, Inc. 2017
+ *
+ * Author:
+ *     Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/loader.h"
+#include "hw/display/ramfb.h"
+#include "ui/console.h"
+#include "sysemu/sysemu.h"
+
+struct QEMU_PACKED RAMFBCfg {
+    uint64_t addr;
+    uint32_t fourcc;
+    uint32_t flags;
+    uint32_t width;
+    uint32_t height;
+    uint32_t stride;
+};
+
+struct RAMFBState {
+    DisplaySurface *ds;
+    uint32_t width, height;
+    struct RAMFBCfg cfg;
+};
+
+static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
+{
+    RAMFBState *s = dev;
+    void *framebuffer;
+    uint32_t stride, fourcc, format;
+    hwaddr addr, length;
+
+    s->width  = be32_to_cpu(s->cfg.width);
+    s->height = be32_to_cpu(s->cfg.height);
+    stride    = be32_to_cpu(s->cfg.stride);
+    fourcc    = be32_to_cpu(s->cfg.fourcc);
+    addr      = be64_to_cpu(s->cfg.addr);
+    length    = stride * s->height;
+    format    = qemu_drm_format_to_pixman(fourcc);
+
+    fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
+            s->width, s->height, addr);
+    framebuffer = address_space_map(&address_space_memory,
+                                    addr, &length, false,
+                                    MEMTXATTRS_UNSPECIFIED);
+    if (!framebuffer || length < stride * s->height) {
+        s->width = 0;
+        s->height = 0;
+        return;
+    }
+    s->ds = qemu_create_displaysurface_from(s->width, s->height,
+                                            format, stride, framebuffer);
+}
+
+void ramfb_display_update(QemuConsole *con, RAMFBState *s)
+{
+    if (!s->width || !s->height) {
+        return;
+    }
+
+    if (s->ds) {
+        dpy_gfx_replace_surface(con, s->ds);
+        s->ds = NULL;
+    }
+
+    /* simple full screen update */
+    dpy_gfx_update_full(con);
+}
+
+RAMFBState *ramfb_setup(Error **errp)
+{
+    FWCfgState *fw_cfg = fw_cfg_find();
+    RAMFBState *s;
+
+    if (!fw_cfg || !fw_cfg->dma_enabled) {
+        error_setg(errp, "ramfb device requires fw_cfg with DMA");
+        return NULL;
+    }
+
+    s = g_new0(RAMFBState, 1);
+
+    fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
+                             NULL, ramfb_fw_cfg_write, s,
+                             &s->cfg, sizeof(s->cfg), false);
+    return s;
+}
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index b5d97ab26d..0af04985d2 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -1,3 +1,5 @@
+common-obj-y += ramfb.o
+
 common-obj-$(CONFIG_ADS7846) += ads7846.o
 common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
 common-obj-$(CONFIG_G364FB) += g364fb.o
-- 
2.9.3

[Qemu-devel] [PATCH v4 2/3] hw/display: add standalone ramfb device

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/display/ramfb.h    |  3 +++
 hw/arm/sysbus-fdt.c           |  7 +++++
 hw/arm/virt.c                 |  2 ++
 hw/display/ramfb-standalone.c | 62 +++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc_piix.c             |  2 ++
 hw/i386/pc_q35.c              |  2 ++
 hw/display/Makefile.objs      |  1 +
 7 files changed, 79 insertions(+)
 create mode 100644 hw/display/ramfb-standalone.c

diff --git a/include/hw/display/ramfb.h b/include/hw/display/ramfb.h
index a3d4c79942..b33a2c467b 100644
--- a/include/hw/display/ramfb.h
+++ b/include/hw/display/ramfb.h
@@ -6,4 +6,7 @@ typedef struct RAMFBState RAMFBState;
 void ramfb_display_update(QemuConsole *con, RAMFBState *s);
 RAMFBState *ramfb_setup(Error **errp);
 
+/* ramfb-standalone.c */
+#define TYPE_RAMFB_DEVICE "ramfb"
+
 #endif /* RAMFB_H */
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index e4c492ea44..277ed872e7 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -36,6 +36,7 @@
 #include "hw/vfio/vfio-platform.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
 #include "hw/vfio/vfio-amd-xgbe.h"
+#include "hw/display/ramfb.h"
 #include "hw/arm/fdt.h"
 
 /*
@@ -406,12 +407,18 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque)
 
 #endif /* CONFIG_LINUX */
 
+static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+    return 0;
+}
+
 /* list of supported dynamic sysbus devices */
 static const NodeCreationPair add_fdt_node_functions[] = {
 #ifdef CONFIG_LINUX
     {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
     {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
 #endif
+    {TYPE_RAMFB_DEVICE, no_fdt_node},
     {"", NULL}, /* last element */
 };
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f0a4fa004c..98b99cf236 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -36,6 +36,7 @@
 #include "hw/arm/virt.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
 #include "hw/vfio/vfio-amd-xgbe.h"
+#include "hw/display/ramfb.h"
 #include "hw/devices.h"
 #include "net/net.h"
 #include "sysemu/device_tree.h"
@@ -1659,6 +1660,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 255;
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
     mc->pci_allow_0_address = true;
diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
new file mode 100644
index 0000000000..c0d241ba01
--- /dev/null
+++ b/hw/display/ramfb-standalone.c
@@ -0,0 +1,62 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/loader.h"
+#include "hw/isa/isa.h"
+#include "hw/display/ramfb.h"
+#include "ui/console.h"
+#include "sysemu/sysemu.h"
+
+#define RAMFB(obj) OBJECT_CHECK(RAMFBStandaloneState, (obj), TYPE_RAMFB_DEVICE)
+
+typedef struct RAMFBStandaloneState {
+    SysBusDevice parent_obj;
+    QemuConsole *con;
+    RAMFBState *state;
+} RAMFBStandaloneState;
+
+static void display_update_wrapper(void *dev)
+{
+    RAMFBStandaloneState *ramfb = RAMFB(dev);
+
+    if (0 /* native driver active */) {
+        /* non-standalone device would run native display update here */;
+    } else {
+        ramfb_display_update(ramfb->con, ramfb->state);
+    }
+}
+
+static const GraphicHwOps wrapper_ops = {
+    .gfx_update = display_update_wrapper,
+};
+
+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);
+}
+
+static void ramfb_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+    dc->realize = ramfb_realizefn;
+    dc->desc = "ram framebuffer standalone device";
+    dc->user_creatable = true;
+}
+
+static const TypeInfo ramfb_info = {
+    .name          = TYPE_RAMFB_DEVICE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(RAMFBStandaloneState),
+    .class_init    = ramfb_class_initfn,
+};
+
+static void ramfb_register_types(void)
+{
+    type_register_static(&ramfb_info);
+}
+
+type_init(ramfb_register_types)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3b87f3cedb..e9b6f064fb 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,6 +28,7 @@
 #include "hw/loader.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
+#include "hw/display/ramfb.h"
 #include "hw/smbios/smbios.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_ids.h"
@@ -423,6 +424,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     m->desc = "Standard PC (i440FX + PIIX, 1996)";
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
+    machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
 }
 
 static void pc_i440fx_3_0_machine_options(MachineClass *m)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 087f2630f9..1a73e1848a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,6 +45,7 @@
 #include "hw/i386/ich9.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/display/ramfb.h"
 #include "hw/smbios/smbios.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
@@ -305,6 +306,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->no_floppy = 1;
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
+    machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
     m->max_cpus = 288;
 }
 
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 0af04985d2..fb8408c6d0 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -1,4 +1,5 @@
 common-obj-y += ramfb.o
+common-obj-y += ramfb-standalone.o
 
 common-obj-$(CONFIG_ADS7846) += ads7846.o
 common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o
-- 
2.9.3

[Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Gerd Hoffmann]

So we have a boot display when using a vgpu as primary display.

Use vfio-pci-ramfb instead of vfio-pci to enable it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/vfio/vfio-common.h |  2 ++
 hw/vfio/display.c             | 10 ++++++++++
 hw/vfio/pci.c                 | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index a9036929b2..a58d7e7e77 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
@@ -143,6 +144,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..409d5a2e3a 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,8 @@ 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 (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
+        vdev->dpy->ramfb = ramfb_setup(errp);
     return 0;
 }
 
@@ -228,6 +233,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 +308,8 @@ 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 (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
+        vdev->dpy->ramfb = ramfb_setup(errp);
     return 0;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 18c493b49e..6a2b42a595 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3234,9 +3234,24 @@ static const TypeInfo vfio_pci_dev_info = {
     },
 };
 
+static void vfio_pci_ramfb_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo vfio_pci_ramfb_dev_info = {
+    .name = "vfio-pci-ramfb",
+    .parent = "vfio-pci",
+    .instance_size = sizeof(VFIOPCIDevice),
+    .class_init = vfio_pci_ramfb_dev_class_init,
+};
+
 static void register_vfio_pci_dev_type(void)
 {
     type_register_static(&vfio_pci_dev_info);
+    type_register_static(&vfio_pci_ramfb_dev_info);
 }
 
 type_init(register_vfio_pci_dev_type)
-- 
2.9.3

Re: [Qemu-devel] [PATCH v4 0/3] ramfb: simple boot framebuffer

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180613084149.14523-1-kraxel@redhat.com
Subject: [Qemu-devel] [PATCH v4 0/3] ramfb: simple boot framebuffer

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1528879723-24675-1-git-send-email-eric.auger@redhat.com -> patchew/1528879723-24675-1-git-send-email-eric.auger@redhat.com
 t [tag update]            patchew/20180612221923.24469-1-mdroth@linux.vnet.ibm.com -> patchew/20180612221923.24469-1-mdroth@linux.vnet.ibm.com
 t [tag update]            patchew/20180613065707.30766-1-david@gibson.dropbear.id.au -> patchew/20180613065707.30766-1-david@gibson.dropbear.id.au
 * [new tag]               patchew/20180613084149.14523-1-kraxel@redhat.com -> patchew/20180613084149.14523-1-kraxel@redhat.com
Switched to a new branch 'test'
a3f6289703 hw/vfio/display: add ramfb support
e928364fde hw/display: add standalone ramfb device
271494a174 hw/display: add ramfb, a simple boot framebuffer living in guest ram

=== OUTPUT BEGIN ===
Checking PATCH 1/3: hw/display: add ramfb, a simple boot framebuffer living in guest ram...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

total: 0 errors, 1 warnings, 109 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: hw/display: add standalone ramfb device...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#71: 
new file mode 100644

total: 0 errors, 1 warnings, 141 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/3: hw/vfio/display: add ramfb support...
ERROR: braces {} are necessary for all arms of this statement
#31: FILE: hw/vfio/display.c:187:
+    if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
[...]

ERROR: braces {} are necessary for all arms of this statement
#50: FILE: hw/vfio/display.c:311:
+    if (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
[...]

total: 2 errors, 0 warnings, 72 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Alex Williamson]

On Wed, 13 Jun 2018 10:41:49 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> So we have a boot display when using a vgpu as primary display.
> 
> Use vfio-pci-ramfb instead of vfio-pci to enable it.

Using a different device here seems like it almost guarantees a very
complicated path to support under libvirt.  What necessitates this
versus a simple ramfb=on option to vfio-pci?

I'm also not sure I understand the usage model, SeaBIOS and OVMF know
how to write to this display, but it seems that the guest does not.  I
suppose in the UEFI case runtime services can be used to continue
writing this display, but BIOS doesn't have such an option, unless
we're somehow emulating VGA here.  So for UEFI, I can imagine this
covers us from power on through firmware boot and up to guest drivers
initializing the GPU (assuming the vGPU supports a kernel mode driver,
does NVIDIA?), but for BIOS it seems we likely still have a break from
the bootloader to GPU driver initialization. For instance, what driver
is used to draw the boot animation (or blue screen) on SeaBIOS Windows
VM?  I'm assuming that this display and the vGPU display are one in the
same, so there's some cut from one to the other.  Thanks,

Alex

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/vfio/vfio-common.h |  2 ++
>  hw/vfio/display.c             | 10 ++++++++++
>  hw/vfio/pci.c                 | 15 +++++++++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index a9036929b2..a58d7e7e77 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
> @@ -143,6 +144,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..409d5a2e3a 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,8 @@ 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 (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
> +        vdev->dpy->ramfb = ramfb_setup(errp);
>      return 0;
>  }
>  
> @@ -228,6 +233,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 +308,8 @@ 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 (strcmp(object_get_typename(OBJECT(vdev)), "vfio-pci-ramfb") == 0)
> +        vdev->dpy->ramfb = ramfb_setup(errp);
>      return 0;
>  }
>  
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 18c493b49e..6a2b42a595 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3234,9 +3234,24 @@ static const TypeInfo vfio_pci_dev_info = {
>      },
>  };
>  
> +static void vfio_pci_ramfb_dev_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo vfio_pci_ramfb_dev_info = {
> +    .name = "vfio-pci-ramfb",
> +    .parent = "vfio-pci",
> +    .instance_size = sizeof(VFIOPCIDevice),
> +    .class_init = vfio_pci_ramfb_dev_class_init,
> +};
> +
>  static void register_vfio_pci_dev_type(void)
>  {
>      type_register_static(&vfio_pci_dev_info);
> +    type_register_static(&vfio_pci_ramfb_dev_info);
>  }
>  
>  type_init(register_vfio_pci_dev_type)

Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Gerd Hoffmann]

On Wed, Jun 13, 2018 at 01:50:47PM -0600, Alex Williamson wrote:
> On Wed, 13 Jun 2018 10:41:49 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > So we have a boot display when using a vgpu as primary display.
> > 
> > Use vfio-pci-ramfb instead of vfio-pci to enable it.
> 
> Using a different device here seems like it almost guarantees a very
> complicated path to support under libvirt.  What necessitates this
> versus a simple ramfb=on option to vfio-pci?

Well, it's simliar to qxl vs. qxl-vga.  It's not qxl,vga={on,off} and
libvirt has no problems to deal with that ...

Another more technical reason is (again) hotplug.  ramfb needs an fw_cfg
entry for configuration, and fw_cfg entries can't be hotplugged.  So
hotplugging vfio-pci with ramfb=on isn't going to fly.  So we need a
separate device with hotplug turned off.

> I'm also not sure I understand the usage model, SeaBIOS and OVMF know
> how to write to this display, but it seems that the guest does not.

Yes.

> I suppose in the UEFI case runtime services can be used to continue
> writing this display,

Yes.

> but BIOS doesn't have such an option, unless we're somehow emulating
> VGA here.

vgabios support is in the pipeline, including text mode emulation (at
vgabios level, direct access to vga window @ 0xa0000 doesn't work).

> So for UEFI, I can imagine this
> covers us from power on through firmware boot and up to guest drivers
> initializing the GPU (assuming the vGPU supports a kernel mode driver,
> does NVIDIA?),

Yes.  Shouldn't matter whenever the driver is kernel or userspace.

> but for BIOS it seems we likely still have a break from
> the bootloader to GPU driver initialization.

Depends.  vgacon (text mode console) doesn't work.  fbcon @ vesafb works.

> For instance, what driver
> is used to draw the boot animation (or blue screen) on SeaBIOS Windows
> VM?

Windows depends on vgabios for that and it works fine.

> I'm assuming that this display and the vGPU display are one in the
> same, so there's some cut from one to the other.

Yes.  If the vfio query plane ioctl reports a valid guest video mode
configuration the vgpu display will be used, ramfb otherwise.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Laszlo Ersek]

On 06/14/18 00:36, Gerd Hoffmann wrote:
> On Wed, Jun 13, 2018 at 01:50:47PM -0600, Alex Williamson wrote:

>> I suppose in the UEFI case runtime services can be used to continue
>> writing this display,
> 
> Yes.

Small clarification for the wording -- "UEFI runtime services" do not
include anything display- or graphics-related. Instead, the OS kernel
may inherit the framebuffer properties (base address, size, pixel
format), and continue accessing the framebuffer directly.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Erik Skultety]

On Thu, Jun 14, 2018 at 12:36:25AM +0200, Gerd Hoffmann wrote:
> On Wed, Jun 13, 2018 at 01:50:47PM -0600, Alex Williamson wrote:
> > On Wed, 13 Jun 2018 10:41:49 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > So we have a boot display when using a vgpu as primary display.
> > >
> > > Use vfio-pci-ramfb instead of vfio-pci to enable it.
> >
> > Using a different device here seems like it almost guarantees a very
> > complicated path to support under libvirt.  What necessitates this
> > versus a simple ramfb=on option to vfio-pci?
>
> Well, it's simliar to qxl vs. qxl-vga.  It's not qxl,vga={on,off} and
> libvirt has no problems to deal with that ...
>
> Another more technical reason is (again) hotplug.  ramfb needs an fw_cfg
> entry for configuration, and fw_cfg entries can't be hotplugged.  So
> hotplugging vfio-pci with ramfb=on isn't going to fly.  So we need a
> separate device with hotplug turned off.

Well if that's not supposed to work ever, libvirt's hotplug code could format
the following FWIW:
"-device vfio-pci [opts],ramfb=off"

As such, new device wouldn't be that of an issue for libvirt if vfio-pci and
vfio-pci-ramfb are back to back compatible with all the device options that are
available for vfio-pci (I mean in terms of using an mdev). Because in that
case, what libvirt could do is to look whether we're supposed to turn on the
display, if so, then we need support for this in capabilities to query and then
we could prefer this new device over the "legacy" vfio-pci one. However, if we
expect a case where QEMU would succeed to start with an mdev mapped to this
new ramfb device but not with vfio-pci, then that's an issue. Otherwise I don't
necessarily see a problem, if QEMU supports this new device and we need
display, let's use that, otherwise let's use the old vfio-pci device. But I'm
still curious about the ramfb=off possibility I asked above for hotplug
nonetheless.

Thanks,
Erik

>
> > I'm also not sure I understand the usage model, SeaBIOS and OVMF know
> > how to write to this display, but it seems that the guest does not.
>
> Yes.
>
> > I suppose in the UEFI case runtime services can be used to continue
> > writing this display,
>
> Yes.
>
> > but BIOS doesn't have such an option, unless we're somehow emulating
> > VGA here.
>
> vgabios support is in the pipeline, including text mode emulation (at
> vgabios level, direct access to vga window @ 0xa0000 doesn't work).
>
> > So for UEFI, I can imagine this
> > covers us from power on through firmware boot and up to guest drivers
> > initializing the GPU (assuming the vGPU supports a kernel mode driver,
> > does NVIDIA?),
>
> Yes.  Shouldn't matter whenever the driver is kernel or userspace.
>
> > but for BIOS it seems we likely still have a break from
> > the bootloader to GPU driver initialization.
>
> Depends.  vgacon (text mode console) doesn't work.  fbcon @ vesafb works.
>
> > For instance, what driver
> > is used to draw the boot animation (or blue screen) on SeaBIOS Windows
> > VM?
>
> Windows depends on vgabios for that and it works fine.
>
> > I'm assuming that this display and the vGPU display are one in the
> > same, so there's some cut from one to the other.
>
> Yes.  If the vfio query plane ioctl reports a valid guest video mode
> configuration the vgpu display will be used, ramfb otherwise.
>
> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH v4 3/3] hw/vfio/display: add ramfb support

[Gerd Hoffmann]

> > Well, it's simliar to qxl vs. qxl-vga.  It's not qxl,vga={on,off} and
> > libvirt has no problems to deal with that ...
> >
> > Another more technical reason is (again) hotplug.  ramfb needs an fw_cfg
> > entry for configuration, and fw_cfg entries can't be hotplugged.  So
> > hotplugging vfio-pci with ramfb=on isn't going to fly.  So we need a
> > separate device with hotplug turned off.
> 
> Well if that's not supposed to work ever, libvirt's hotplug code could format
> the following FWIW:
> "-device vfio-pci [opts],ramfb=off"
> 
> As such, new device wouldn't be that of an issue for libvirt if vfio-pci and
> vfio-pci-ramfb are back to back compatible with all the device options that are
> available for vfio-pci (I mean in terms of using an mdev). Because in that
> case, what libvirt could do is to look whether we're supposed to turn on the
> display, if so, then we need support for this in capabilities to query and then
> we could prefer this new device over the "legacy" vfio-pci one. However, if we
> expect a case where QEMU would succeed to start with an mdev mapped to this
> new ramfb device but not with vfio-pci, then that's an issue.

vfio-pci and vfio-pci-ramfb are identical, except for the later having a
boot display (with display=on), and vfio-pci-ramfb not being
hotplugable.  So, yes, all pcu/mdev devices should work with both
vfio-pci variants.

> But I'm still curious about the ramfb=off possibility I asked above
> for hotplug nonetheless.

Well, the problem is introspection.  qemu can report via qmp whenever a
specific device supports hotplug.  A device which can both be
hot-pluggable or not hot-pluggable depending on some condition is a
problem here ...

cheers,
  Gerd