Re: [Qemu-devel] [PATCH 2/2] virtio-gpu/2d: add virtio gpu core code

[Max Reitz <mreitz@redhat.com>]

On 29.05.2015 13:45, Gerd Hoffmann wrote:
> This patch adds the core code for virtio gpu emulation,
> covering 2d support.
>
> Written by Dave Airlie and Gerd Hoffmann.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/display/Makefile.objs       |   2 +
>   hw/display/virtio-gpu.c        | 918 +++++++++++++++++++++++++++++++++++++++++
>   include/hw/virtio/virtio-gpu.h | 147 +++++++
>   trace-events                   |  14 +
>   4 files changed, 1081 insertions(+)
>   create mode 100644 hw/display/virtio-gpu.c
>   create mode 100644 include/hw/virtio/virtio-gpu.h

[snip]

> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> new file mode 100644
> index 0000000..f4c00a8
> --- /dev/null
> +++ b/hw/display/virtio-gpu.c
[snip]

> +static uint32_t virtio_gpu_get_features(VirtIODevice *vdev, uint32_t features)
> +{
> +    return features;
> +}

Does this series rely on some other patches? Because in master, 
VirtioDeviceClass::get_features() is still uint64_t (*)(VirtioDevice *, 
uint64_t) (which results in "hw/display/virtio-gpu.c:886:23: error: 
assignment from incompatible pointer type" for me).

(in an earlier series, this function was uint64_t ()(VirtIODevice *, 
uint64_t))

[snip]

> +int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab,
> +                                  struct virtio_gpu_ctrl_command *cmd,
> +                                  struct iovec **iov)
> +{
> +    struct virtio_gpu_mem_entry *ents;
> +    size_t esize, s;
> +    int i;
> +
> +    if (ab->nr_entries > 1024) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: nr_entries is too big (%d > 1024)\n",
> +                      __func__, ab->nr_entries);
> +        return -1;
> +    }

Same question I had in said earlier series: Do you want to change this 
to 16384, because 1024 may be too small?

[snip]

> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> new file mode 100644
> index 0000000..8015beb
> --- /dev/null
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -0,0 +1,147 @@
> +/*
> + * Virtio GPU Device
> + *
> + * Copyright Red Hat, Inc. 2013-2014
> + *
> + * Authors:
> + *     Dave Airlie <airlied@redhat.com>
> + *     Gerd Hoffmann <kraxel@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef _QEMU_VIRTIO_VGA_H
> +#define _QEMU_VIRTIO_VGA_H
> +
> +#include "qemu/queue.h"
> +#include "ui/qemu-pixman.h"
> +#include "ui/console.h"
> +#include "hw/virtio/virtio.h"
> +#include "hw/pci/pci.h"
> +
> +#include "standard-headers/linux/virtio_gpu.h"
> +#define TYPE_VIRTIO_GPU "virtio-gpu-device"
> +#define VIRTIO_GPU(obj)                                        \
> +        OBJECT_CHECK(VirtIOGPU, (obj), TYPE_VIRTIO_GPU)
> +
> +#define VIRTIO_ID_GPU 16
> +
> +#define VIRTIO_GPU_MAX_RES 16

Still unused. Still intentional? :-)

Max