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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



  Hi,

> > @@ -0,0 +1,158 @@
> > +#ifndef VIRTGPU_HW_H
> > +#define VIRTGPU_HW_H
> 
> Non-trivial file, deserves a copyright and license notice.

Added.

> > +
> > +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.

I don't think this will ever be shipped as library header for external
users ...

> > +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?

Yes.

> Is it worth adding a compile-time assertion about the
> size of the struct to ensure the compiler doesn't add any additional
> padding?

Makes sense.  What is the usual trick to do that?

thanks,
  Gerd


_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux