[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description
On Fri, Jan 31, 2014 at 03:25:31PM +1030, Rusty Russell wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > On Thu, Jan 30, 2014 at 01:10:29PM +0100, Cornelia Huck wrote: > >> I think what makes this complicated is that we specify datastructures > >> that are supposed to work for both legacy and non-legacy, when they > >> are in fact different (at least on be). > > > > So how about something like attribute((virtio_endian)) (a bit too long) > > or ve32 (slightly ugly). > > > >> How about adding "A note on legacy interfaces" for each of these cases, > >> where we specify the data structures with native-endian where > >> appropriate, and keep the explicit le notation for the standard? We'd > >> end up with some duplication, but we'd be very clear on what is > >> expected. > > > > In practice, this will make it harder to write transitional > > drivers. Driver authors will have to reverse-engineer field > > layouts and note that offsets happen to match. > > That's not being nice to readers. > > > > Take a look at virtio drivers that Rusty wrote: > > ring layouts *don't* use le tags because sometimes > > they are used with be. > > Sure, but it will always be hard to write such drivers. There shouldn't > be many people doing that though. And they'll be doing it for backwards > compatibility, where they'll presumably test... > > I'm more concerned about people writing non-legacy drivers and getting > endianness wrong. But if you feel strongly, we could put a comment > above each one? > > Cheers, > Rusty. Sounds good, let's do that. And let's add a comment where we define leXX. > diff --git a/content.tex b/content.tex > index 48698fc..fa33435 100644 > --- a/content.tex > +++ b/content.tex > @@ -349,6 +349,7 @@ Drivers MUST NOT add a descriptor chain over than $2^{32}$ bytes long in total; > this implies that loops in the descriptor chain are forbidden! > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct vring_desc { > /* Address (guest-physical). */ > le64 addr; > @@ -409,6 +410,7 @@ the device MUST ignore the write-only flag (flags\&VRING_DESC_F_WRITE) in the de > \subsection{The Virtqueue Available Ring}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Available Ring} > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct vring_avail { > #define VRING_AVAIL_F_NO_INTERRUPT 1 > le16 flags; > @@ -445,6 +447,7 @@ entirely. > \subsection{The Virtqueue Used Ring}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring} > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct vring_used { > #define VRING_USED_F_NO_NOTIFY 1 > le16 flags; > @@ -453,6 +456,7 @@ entirely. > le16 avail_event; /* Only if VIRTIO_RING_F_EVENT_IDX */ > }; > > + /* Note: LEGACY version was not little endian! */ > /* le32 is used here for ids for padding reasons. */ > struct vring_used_elem { > /* Index of start of used descriptor chain. */ > @@ -2435,6 +2439,7 @@ and transmitq0..transmitqN respectively; > is negotiated. Legal values for this field are 1 to 0x8000. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_net_config { > u8 mac[6]; > le16 status; > @@ -2521,6 +2526,7 @@ placed in the receiveq0..receiveqN. In each case, the packet > itself is preceeded by a header: > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_net_hdr { > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 > u8 flags; > @@ -2848,6 +2854,7 @@ queue incoming packets into one of the multiple receiveq0..receiveqN > depending on the packet flow. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_net_ctrl_mq { > le16 virtqueue_pairs; > }; > @@ -2984,6 +2991,7 @@ present. The availability of the others all depend on various feature > bits as indicated above. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_blk_config { > le64 capacity; > le32 size_max; > @@ -3241,6 +3249,7 @@ data and outgoing characters are placed in the transmit queue. > acknowledging the feature. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_console_config { > le16 cols; > le16 rows; > @@ -3318,6 +3327,7 @@ when a port is closed or hot-unplugged. > buffer and the events associated are: > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_console_control { > le32 id; /* Port number */ > le16 event; /* The kind of control event */ > @@ -3507,6 +3517,7 @@ as follows: > compatibility, unsupported statistics should be omitted. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_balloon_stat { > #define VIRTIO_BALLOON_S_SWAP_IN 0 > #define VIRTIO_BALLOON_S_SWAP_OUT 1 > @@ -3598,6 +3609,7 @@ targets that receive and process the requests. > and cdb_size are writable by the driver. > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_scsi_config { > le32 num_queues; > le32 seg_max; > @@ -3682,6 +3694,7 @@ consumed with no order constraints. > Requests have the following format: > > \begin{lstlisting} > + /* Note: LEGACY version was not little endian! */ > struct virtio_scsi_req_cmd { > // Read-only > u8 lun[8]; > @@ -3842,6 +3855,7 @@ The following commands are defined: > #define VIRTIO_SCSI_T_TMF_QUERY_TASK 6 > #define VIRTIO_SCSI_T_TMF_QUERY_TASK_SET 7 > > + /* Note: LEGACY version was not little endian! */ > struct virtio_scsi_ctrl_tmf > { > // Read-only part > > > >
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]