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