Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

[Eric Blake]

On 09/11/2014 09:09 AM, Gerd Hoffmann wrote:
> This patch adds the header file with structs and defines for
> the virtio based gpu device.  Covers 2d operations only.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  include/hw/virtio/virtgpu_hw.h | 158 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 158 insertions(+)
>  create mode 100644 include/hw/virtio/virtgpu_hw.h
> 
> diff --git a/include/hw/virtio/virtgpu_hw.h b/include/hw/virtio/virtgpu_hw.h
> new file mode 100644
> index 0000000..461f452
> --- /dev/null
> +++ b/include/hw/virtio/virtgpu_hw.h
> @@ -0,0 +1,158 @@
> +#ifndef VIRTGPU_HW_H
> +#define VIRTGPU_HW_H

Non-trivial file, deserves a copyright and license notice.

> +
> +enum virtgpu_ctrl_type {
> +        VIRTGPU_UNDEFINED = 0,
> +
> +        /* 2d commands */
> +        VIRTGPU_CMD_GET_DISPLAY_INFO = 0x0100,

Please consider also adding:

#define VIRTGPU_CMD_GET_DISPLAY_INFO VIRTGPU_CMD_GET_DISPLAY_INFO

and friends.  It makes it MUCH nicer for application software to probe
for later extensions if every member of the enum is also associated with
a preprocessor macro.


> +
> +struct virtgpu_ctrl_hdr {
> +        uint32_t type;
> +        uint32_t flags;
> +        uint64_t fence_id;
> +        uint32_t ctx_id;
> +        uint32_t padding;
> +};
> +

Is the padding to ensure that this is aligned regardless of 32-bit or
64-bit hosts?  Is it worth adding a compile-time assertion about the
size of the struct to ensure the compiler doesn't add any additional
padding?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature