On Tue, 13 Feb 2018 17:18:46 +0100
Gerd Hoffmann <kra...@redhat.com> wrote:
> Wire up region-based display.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> hw/vfio/pci.h | 1 +
> include/hw/vfio/vfio-common.h | 8 ++++
> hw/vfio/display.c | 102
> +++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 109 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 1d005d922d..9fe0f3f198 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -148,6 +148,7 @@ typedef struct VFIOPCIDevice {
> bool no_kvm_msi;
> bool no_kvm_msix;
> bool no_geforce_quirks;
> + VFIODisplay *dpy;
> } VFIOPCIDevice;
>
> uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index f3a2ac9fee..fc8ae14fb7 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -142,6 +142,14 @@ typedef struct VFIOGroup {
> QLIST_ENTRY(VFIOGroup) container_next;
> } VFIOGroup;
>
> +typedef struct VFIODisplay {
> + QemuConsole *con;
> + struct {
> + VFIORegion buffer;
> + DisplaySurface *surface;
> + } region;
> +} VFIODisplay;
> +
> void vfio_put_base_device(VFIODevice *vbasedev);
> void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
> void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 4249be398d..819a0cb08c 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -19,6 +19,105 @@
> #include "qapi/error.h"
> #include "pci.h"
>
> +/* ---------------------------------------------------------------------- */
> +
> +static void vfio_display_region_update(void *opaque)
> +{
> + VFIOPCIDevice *vdev = opaque;
> + VFIODisplay *dpy = vdev->dpy;
> + struct vfio_device_gfx_plane_info plane = {
> + .argsz = sizeof(plane),
> + .flags = VFIO_GFX_PLANE_TYPE_REGION
> + };
> + pixman_format_code_t format = PIXMAN_x8r8g8b8;
nit, unused initialization
> + int ret;
> +
> + ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_QUERY_GFX_PLANE, &plane);
> + if (ret < 0) {
> + fprintf(stderr, "ioctl VFIO_DEVICE_QUERY_GFX_PLANE: %s\n",
> + strerror(errno));
nit, should this be an error_report()?
> + return;
> + }
> + if (!plane.drm_format || !plane.size) {
> + return;
> + }
> + format = qemu_drm_format_to_pixman(plane.drm_format);
> + if (!format) {
> + return;
> + }
> +
> + if (dpy->region.buffer.size &&
> + dpy->region.buffer.nr != plane.region_index) {
> + /* region changed */
> + vfio_region_exit(&dpy->region.buffer);
Don't we need to also call vfio_region_finalize()? _exit only deletes
mmap MemoryRegions from the base MemoryRegion, _finalize does the
actual munmap, unparent calls, and frees dynamic memory. As is, this
leaks memory afaict.
> + memset(&dpy->region.buffer, 0, sizeof(dpy->region.buffer));
_finalize could cleanup the VFIORegion rather than requiring this
memset, we simply haven't needed it yet since regions are generally
tied to the life cycle of the device.
> + dpy->region.surface = NULL;
> + }
> +
> + if (dpy->region.surface &&
> + (surface_width(dpy->region.surface) != plane.width ||
> + surface_height(dpy->region.surface) != plane.height ||
> + surface_format(dpy->region.surface) != format)) {
> + /* size changed */
> + dpy->region.surface = NULL;
> + }
> +
> + if (!dpy->region.buffer.size) {
> + /* mmap region */
> + ret = vfio_region_setup(OBJECT(vdev), &vdev->vbasedev,
> + &dpy->region.buffer,
> + plane.region_index,
> + "display");
> + if (ret != 0) {
> + fprintf(stderr, "%s: vfio_region_setup(%d): %s\n",
> + __func__, plane.region_index, strerror(-ret));
> + goto err1;
> + }
> + ret = vfio_region_mmap(&dpy->region.buffer);
> + if (ret != 0) {
> + fprintf(stderr, "%s: vfio_region_mmap(%d): %s\n", __func__,
> + plane.region_index, strerror(-ret));
> + goto err2;
> + }
> + assert(dpy->region.buffer.mmaps[0].mmap != NULL);
> + }
> +
> + if (dpy->region.surface == NULL) {
> + /* create surface */
> + dpy->region.surface = qemu_create_displaysurface_from
> + (plane.width, plane.height, format,
> + plane.stride, dpy->region.buffer.mmaps[0].mmap);
> + dpy_gfx_replace_surface(dpy->con, dpy->region.surface);
> + }
> +
> + /* full screen update */
> + dpy_gfx_update(dpy->con, 0, 0,
> + surface_width(dpy->region.surface),
> + surface_height(dpy->region.surface));
> + return;
> +
> +err2:
> + vfio_region_exit(&dpy->region.buffer);
This one probably needs to be replaced with _finalize, if _mmap fails,
_exit doesn't do anything useful.
> +err1:
> + memset(&dpy->region.buffer, 0, sizeof(dpy->region.buffer));
vfio_region_setup has no failure path that modifies the provided
VFIORegion, so this is unnecessary if _finalize is called and cleans up.
> +}
> +
> +static const GraphicHwOps vfio_display_region_ops = {
> + .gfx_update = vfio_display_region_update,
> +};
> +
> +static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
> +{
> + vdev->dpy = g_new0(VFIODisplay, 1);
> + vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
> + &vfio_display_region_ops,
> + vdev);
> + /* TODO: disable hotplug (there is no graphic_console_close) */
via DeviceClass.hotpluggable? I know that QE will immediately test
this, so it needs to be sorted out sooner than later.
> + return 0;
> +}
> +
> +/* ---------------------------------------------------------------------- */
> +
> int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
> {
> struct vfio_device_gfx_plane_info probe;
> @@ -38,8 +137,7 @@ int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
> probe.flags = VFIO_GFX_PLANE_TYPE_PROBE | VFIO_GFX_PLANE_TYPE_REGION;
> ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_QUERY_GFX_PLANE, &probe);
> if (ret == 0) {
> - error_setg(errp, "vfio-display: region support not implemented yet");
> - return -1;
> + return vfio_display_region_init(vdev, errp);
> }
>
> if (vdev->display == ON_OFF_AUTO_AUTO) {
