From: Max Reitz <mreitz@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 5/9] virtio-gpu: add 3d mode and virgl rendering support.
Date: Mon, 14 Sep 2015 20:14:54 +0200 [thread overview]
Message-ID: <55F70E9E.3020603@redhat.com> (raw)
In-Reply-To: <1441797654-15350-6-git-send-email-kraxel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4381 bytes --]
On 09.09.2015 13:20, Gerd Hoffmann wrote:
> Add virglrenderer library detection. Add 3d mode to virtio-gpu,
> wire up virglrenderer library. When in 3d mode render using the
> new context management and texture scanout callbacks.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> configure | 32 +++
> hw/display/Makefile.objs | 6 +-
> hw/display/virtio-gpu-3d.c | 598 +++++++++++++++++++++++++++++++++++++++++
> hw/display/virtio-gpu.c | 130 ++++++++-
> include/hw/virtio/virtio-gpu.h | 22 +-
> trace-events | 8 +
> 6 files changed, 785 insertions(+), 11 deletions(-)
> create mode 100644 hw/display/virtio-gpu-3d.c
As always when it comes to virtio-gpu, I'm just a very, very naive
reviewer, thus I only have "local" things to complain about.
[snip]
> diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
> new file mode 100644
> index 0000000..ac62fb4
> --- /dev/null
> +++ b/hw/display/virtio-gpu-3d.c
> @@ -0,0 +1,598 @@
[snip]
> +static void virgl_resource_detach_backing(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_resource_detach_backing detach_rb;
> + struct iovec *res_iovs = NULL;
> + int num_iovs = 0;
> +
> + VIRTIO_GPU_FILL_CMD(detach_rb);
> + trace_virtio_gpu_cmd_res_back_detach(detach_rb.resource_id);
> +
> + virgl_renderer_resource_detach_iov(detach_rb.resource_id,
> + &res_iovs,
> + &num_iovs);
> + if (res_iovs == NULL || num_iovs == 0) {
> + return;
> + }
> + virtio_gpu_cleanup_mapping_iov(res_iovs, num_iovs);
Is res_iovs leaked here?
> +}
[snip]
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index a67d927..4dbdd12 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
[snip]
> @@ -45,6 +62,27 @@ static void update_cursor_data_simple(VirtIOGPU *g,
> pixels * sizeof(uint32_t));
> }
>
> +#ifdef CONFIG_VIRGL
> +
> +static void update_cursor_data_virgl(VirtIOGPU *g,
> + struct virtio_gpu_scanout *s,
> + uint32_t resource_id)
> +{
> + uint32_t width, height;
> + uint32_t pixels, *data;
> +
> + data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
> + if (!data) {
> + return;
> + }
> +
> + pixels = s->current_cursor->width * s->current_cursor->height;
> + memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
> + free(data);
width and height are unused; should they be compared against
s->current_cursor->{width,height} to spot discrepancies?
> +}
> +
> +#endif
> +
> static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor)
> {
> struct virtio_gpu_scanout *s;
[snip]
> @@ -92,9 +131,23 @@ static void virtio_gpu_set_config(VirtIODevice *vdev, const uint8_t *config)
> static uint64_t virtio_gpu_get_features(VirtIODevice *vdev, uint64_t features,
> Error **errp)
> {
> + VirtIOGPU *g = VIRTIO_GPU(vdev);
> +
> + if (virtio_gpu_virgl_enabled(g->conf)) {
> + features |= (1 << VIRTIO_GPU_FEATURE_VIRGL);
> + }
> return features;
> }
>
> +static void virtio_gpu_set_features(VirtIODevice *vdev, uint64_t features)
> +{
> + static const uint32_t virgl = (1 << VIRTIO_GPU_FEATURE_VIRGL);
> + VirtIOGPU *g = VIRTIO_GPU(vdev);
> +
> + g->use_virgl_renderer = ((features & virgl) == virgl);
Could a non-well-behaving guest just set this feature bit even if it was
not reported by virtio_gpu_get_features() because it has been disabled?
This would then result in g->use_virgl_renderer being set to true, which
would have two implications:
(1) In case CONFIG_VIRGL is not defined, no implication right now. But
maybe in the future there may be cases where the flag is queried
even with !defined(CONFIG_VIRGL).
(2) virgl may be disabled even with CONFIG_VIRGL being defined; namely
with HOST_WORDS_BIGENDIAN. That would probably be a problem, because
we'd then call the virgl functions even though we meant not to.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
next prev parent reply other threads:[~2015-09-14 18:15 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-09 11:20 [Qemu-devel] [PATCH 0/9] add virgl rendering support Gerd Hoffmann
2015-09-09 11:20 ` [Qemu-devel] [PATCH 1/9] shaders: initialize vertexes once Gerd Hoffmann
2015-09-10 12:40 ` Marc-André Lureau
2015-09-10 13:22 ` Gerd Hoffmann
2015-09-10 13:42 ` Marc-André Lureau
2015-09-10 14:59 ` Gerd Hoffmann
2015-09-14 16:00 ` Max Reitz
2015-09-14 16:19 ` Max Reitz
2015-09-15 7:10 ` Gerd Hoffmann
2015-09-09 11:20 ` [Qemu-devel] [PATCH 2/9] sdl2: quick & dirty flicker workaround Gerd Hoffmann
2015-09-10 12:40 ` Marc-André Lureau
2015-09-14 16:16 ` Max Reitz
2015-09-09 11:20 ` [Qemu-devel] [PATCH 3/9] ui/console: add opengl context and scanout support interfaces Gerd Hoffmann
2015-09-10 12:40 ` Marc-André Lureau
2015-09-15 8:30 ` Paolo Bonzini
2015-09-09 11:20 ` [Qemu-devel] [PATCH 4/9] virtio-gpu: update headers for virgl/3d Gerd Hoffmann
2015-09-10 12:41 ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 5/9] virtio-gpu: add 3d mode and virgl rendering support Gerd Hoffmann
2015-09-14 18:14 ` Max Reitz [this message]
2015-09-15 7:33 ` Gerd Hoffmann
2015-09-15 8:33 ` Paolo Bonzini
2015-09-09 11:20 ` [Qemu-devel] [PATCH 6/9] sdl2/opengl: add opengl context and scanout support Gerd Hoffmann
2015-09-14 18:49 ` Max Reitz
2015-09-15 7:54 ` Gerd Hoffmann
2015-09-16 13:44 ` Max Reitz
2015-09-09 11:20 ` [Qemu-devel] [PATCH 7/9] opengl: add egl-context.[ch] helpers Gerd Hoffmann
2015-09-11 14:13 ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 8/9] gtk/opengl: add opengl context and scanout support (egl) Gerd Hoffmann
2015-09-11 14:36 ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 9/9] gtk/opengl: add opengl context and scanout support (GtkGLArea) Gerd Hoffmann
2015-09-11 14:44 ` Marc-André Lureau
2015-09-14 13:50 ` Gerd Hoffmann
2015-09-11 16:10 ` [Qemu-devel] [PATCH 0/9] add virgl rendering support Marc-André Lureau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F70E9E.3020603@redhat.com \
--to=mreitz@redhat.com \
--cc=kraxel@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.