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

[Gerd Hoffmann <kraxel@redhat.com>]

On Mi, 2015-02-25 at 15:04 -0500, Max Reitz wrote:
> On 2015-02-23 at 05:23, Gerd Hoffmann wrote:
> > This patch adds the header file with structs and defines for
> > the virtio based gpu device.  Covers 2d operations only.
> >
> > Written by Dave Airlie and Gerd Hoffmann.
> >
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >   include/hw/virtio/virtgpu_hw.h | 203 +++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 203 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..d6641e8
> > --- /dev/null
> > +++ b/include/hw/virtio/virtgpu_hw.h
> > @@ -0,0 +1,203 @@
> > +/*
> > + * Virtio GPU Device
> > + *
> > + * Copyright Red Hat, Inc. 2013-2014
> 
> Hm... Well, if there haven't been any changes since 2014...

The header files hasn't seen any 2015 changes indeed ;)

> > + * 3. Neither the name of IBM nor the names of its contributors
> 
> Is the "IBM" intended?

Looks like cut+paste from virtio_ring.h ...

'IBM' probably comes from rusty being employed by IBM.

> > +struct virtio_gpu_ctrl_hdr {
> > +    uint32_t type;
> > +    uint32_t flags;
> > +    uint64_t fence_id;
> > +    uint32_t ctx_id;
> > +    uint32_t padding;
> > +};
> 
> Shouldn't all these structures be QEMU_PACKED? In practice, it won't 
> matter (I hope!), but if you're relying on it...

It should not indeed, I carefully checked we don't have any funky stuff
in there, like unaligned 64bit fields which have different packing in
i686 and x86_64.

Also the header files is shared between linux and qemu, so adding
QEMU_PACKED there is a bit difficult.

cheers,
  Gerd