Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jan 20, 2014 at 02:28:00PM +1030, Rusty Russell wrote:
> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > Move the ccw payload description to tables, instead of using
> > C structures with gcc extensions or textual descriptions.
> 
> I think it's horrible.
> 
> Almost everyone reading this will understand C.  And they can cut and
> paste the code from the spec into their implementation, which helps
> everyone.
> 
> We'll just have to live with the lack of padding, and let's try not to
> add any more packed-requiring structures.  If we deprecate
> virtio-balloon, the rest will be contained in CCW.
> 
> Cheers,
> Rusty.

For CCW, how about my idea of adding optional padding explicitly?
In the compatibility section, we can explicitly specify the
the length.
Drivers can use offsetof(padding) and avoid packed attribute.

For example:

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>


diff --git a/content.tex b/content.tex
index 803615d..f912090 100644
--- a/content.tex
+++ b/content.tex
@@ -2077,7 +2077,8 @@ communication block:
 	struct virtio_feature_desc {
 		le32 features;
 		u8 index;
-	} __attribute__ ((packed));
+		u8 padding[3]; /* Optional padding */
+	};
 \end{lstlisting}
 
 features are the 32 bits of features currently accessed, while
@@ -2092,6 +2093,18 @@ For communicating its supported features to the device, the driver
 uses the CCW_CMD_WRITE_FEAT command, denoting a features/index
 combination.
 
+padding field is optional. If present, it is ignored by the
+device.
+
+\paragraph{Legacy Interface: A Note on Handling Device Features}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Handling Device Features / Legacy Interface: A Note on Handling Device Features}
+
+Legacy devices MAY not support the optional padding field;
+they MAY reject CCW_CMD_READ_FEAT and CCW_CMD_WRITE_FEAT
+commands unless the communication block byte count is exactly 5.
+
+Transitional drivers MUST make the communication block byte count
+be exactly 5, up to and excluding the optional padding field.
+
 \subsubsection{Device Configuration}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Device Configuration}
 
 The device's configuration space is located in host memory. It is

-- 
MST