Re: [virtio] [PATCH] ccw: split descriptor/available/used rings

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Mon, 23 Sep 2013 13:39:30 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Resolves VIRTIO-23.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  virtio-v1.0-wd01-part1-specification.txt | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
> index cafc34f..08cb857 100644
> --- a/virtio-v1.0-wd01-part1-specification.txt
> +++ b/virtio-v1.0-wd01-part1-specification.txt
> @@ -1187,7 +1187,28 @@ Afterwards, CCW_CMD_SET_VQ is issued by the guest to inform the
>  host about the location used for its queue. The transmitted
>  structure is
> 
> -struct vq_info_block {
> +struct vq_info_block_legacy {

That's not legacy, no?

> +	__u64 desc;
> +	__u64 avail;
> +	__u64 used;
> +	__u16 index;
> +	__u16 num;
> +	__u32 padding;
> +} __attribute__ ((packed));

Hm. This means the legacy and non-legacy cases transmit two
incompatible structures with the same channel command. I'm not sure
whether this is a good idea.

The structures are different in length, and the host will fail if the
structure transmitted is too short, but it is possible for the guest to
transmit the longer structure to a legacy host and turn off the length
check. Sure, that should not happen, but I'm not too fond of this
structure.

So we should either:

- extend the old structure in a compatible way
- use a new channel command for this purpose (and prepend the structure
  with a version field in case we want to change it again in the future)

> +
> +queue contains the guest address for Descriptor Table,
> +Available Ring and Used Ring for virtqueue index respectively.
> +The actual virtqueue size (number of allocated buffers) is
> +transmitted in num.
> +padding is a reserved field, and must be set to 0.
> +
> +2.3.3.2.1.1 Legacy Interface: A Note on Configuring a Virtqueue
> +----------------------------------
> +
> +When configured through the legacy interface, transitional
> +devices expect CCW_CMD_SET_VQ to transfer the following structure:
> +
> +struct vq_info_block_legacy {
>  	__u64 queue;
>  	__u32 align;
>  	__u16 index;