[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio] [PATCH] ccw: split descriptor/available/used rings
On Tue, Oct 08, 2013 at 11:17:45AM +0200, Cornelia Huck wrote: > On Mon, 7 Oct 2013 17:57:25 +0300 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Mon, Oct 07, 2013 at 04:32:17PM +0200, Cornelia Huck wrote: > > > 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) > > > > This would burden 1.0 spec with legacy considerations. > > IMO that's ugly. > > > > Rusty sent a proposal where guest sets features > > before configuring VQs. > > > > If we vote this in, host can detect whether guest acked the VIRTIO_1 > > feature bit and handle the command appropriately. > > Yes, it can. I'd still prefer to make this as failsafe as possible. > Simply extending the structure would satisfy this (and there is > precedent for this - it has been done e.g. for SenseID). This reminds me - our use of the SenseID format for device identification is not in the spec, is it? We should probably add it - want to do this?
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]