Re: [virtio-dev] [PATCH 2/2] Add GPU device type.

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> I've just moved the implementation and guest driver here over to not
> using a union anymore,
> http://cgit.freedesktop.org/~airlied/qemu/log/?h=virtio-gpu
> http://cgit.freedesktop.org/~airlied/linux/log/?h=virtio-vga-3d

Finally found the time to continue working on the virtio-gpu spec.

Adapted to the latest changes and to some suggestions in this thread.
More to be done.  Current state is here:

    http://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-gpu

I've got some questions/suggestions while going over this:


First:  How we'll go handle error reporting?  It's not really there at
the moment ...

I think we should drop the VIRTGPU_CMD_HAS_RESP bit and have all
commands return a response.  Usually that would be just
VIRTIO_GPU_RESP_SUCCESS.  VIRTIO_GPU_CMD_GET_DISPLAY_INFO will return
VIRTIO_GPU_RESP_DISPLAY_CHANGE of course.  On error we can return
VIRTIO_GPU_RESP_ERROR with an error information struct, like this:

struct virtio_gpu_display_info {
	struct virtio_gpu_resp_hdr hdr;
	le32 error_code;
	le32 error_id;
};

where error_id may (depending on error code) refer to the object the
error applies to, i.e. for resource related errors that would be the
resource id.

For "can't continue until device reset" class of errors we can add a
VIRTIO_GPU_EVENT_FATAL_ERROR event bit.


> > As I understand it:  You create one (or more) cursor resources using
> > normal command queue commands.  Then you'll set cursor_id to the
> > resource id you want use as cursor, fill the other fields.
> > generation_count being incremented (relative to the previous command)
> > implies the cursor sprite needs to be reloaded.  same generation_count
> > indicates only the position (x & y) has changed.  Correct dave?
> 
> Yes correct,

Second:  Is the generation count used for anything but indicating the
cursor should be reloaded?  I tend to think we should make the command
and cursor queue pretty much identical (i.e. same format for commands
and responses).  Then have two commands for the cursor, one to load it
from a resource and one to simply move it.  command queue accepts all
commands.  cursor queue accepts only cursor commands.

I guess we need a scanout_id field for the cursor updates so you can
place the cursor on any head ... ?

Do we want support for multiple cursors?


Third: ressource management.  Currently the guest can allocate unlimited
ressources on the host using the VIRTIO_GPU_CMD_RESOURCE_CREATE_2D
command.  We certainly want apply a limit here so the guest can't DoS
the host.  And we should communicate that to the guest, either by
providing a command to query it or in config space.

so long,
  Gerd