On Mon, 13 Jan 2014 12:53:14 +0100
Thomas Huth <> wrote:
> On Sun, 12 Jan 2014 13:24:09 +0200
> "Michael S. Tsirkin" <> wrote:
>
> > On Fri, Jan 10, 2014 at 01:49:49PM +0100, Thomas Huth wrote:
> > >
> > > Here's my feedback for Virtio draft 01, chapter 4:
> [...]
> > > Chapter 4.3.2.*:
> > >
> > > - In this chapter, the C-structs are marked with "__attribute__ ((packed));"
> > > which is just a GNU-C extension, as far as I know. In the other chapters,
> > > the structs are not marked with this string. So for consistency, I'd remove
> > > them here, too (and maybe state somewhere at the beginning of the spec
> > > that structs are considered to be without compiler padding inbetween)
> >
> > I agree since C spec seems to allow compilers to add
> > arbitrary padding anywhere in theory, we should mention all structures
> > assume there's no padding.
> >
> > And in most places, we don't really need the packing as
> > fields are aligned properly so any sane compiler will just do the right
> > thing.
> >
> > Unfortunately CCW has several instances where any reasonable
> > compiler would instroduce padding, I think just a note won't be enough
> > since compiler-introduced issues are so hard to debug.
> > So I think that instead of asking people to use compiler extensions to
> > suppress padding, it's best just to add padding explicitly.
> >
> > Cornelia, can we do this?
> > For example
> >
> > struct virtio_feature_desc {
> > be32 features;
> > u8 index;
> > u8 padding[3]; /* Optional padding */
> > };
>
> Another possibility would be to abandon the C structs in the spec
> completely and describe the memory layout of the buffers with a table,
> something like this:
>
> +---------+------+----------+
> | Offset | Size | Name |
> +---------+------+----------+
> | 0 | 4 | features |
> +---------+------+----------+
> | 4 | 1 | index |
> +---------+------+----------+
>
> That would also be easier to read for those who don't know C and would
> like to write a driver or device for virtio in another programming
> language.
I like that idea. It is independent of programming language, and we
could add further information (e.g. endianness) in another column.
And it is compatible with how things work now :)
There are a number of other C struct definitions in other sections of
the document (generic, transport and device parts). If we convert ccw,
I suggest that we convert those as well. We could maybe keep the C
structures as example implementations in an appendix, but I'm not sure
how much value this adds.
