OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [PATCH] virtio-ccw: split descriptor/available/used rings (alternate)


On Thu, 10 Oct 2013 08:43:44 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Oct 09, 2013 at 05:59:36PM +0200, Cornelia Huck wrote:
> > Extend vq_info_block so that the addresses for descriptor table,
> > available ring and used ring may be transmitted independently.
> > 
> > Depending upon the selected revision, post a command reject instead
> > of a channel program check if the driver uses the legacy format
> > and length checks are suppressed.
> > 
> > VIRTIO-23
> > 
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > 
> > ---
> > 
> > This is an alternate approach, extending the exiting structure instead
> > of creating a different layout. I'm not 100% sure whether doing a
> > command reject instead of a channel program check in case of a short
> > buffer is the right approach, though. Doing a channel program check
> > would probably cover that error just as well, and we could resolve
> > VIRTIO-23 independently of VIRTIO-42.
> > ---
> >  virtio-v1.0-wd01-part1-specification.txt |   32 +++++++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
> > index ae646db..baff12f 100644
> > --- a/virtio-v1.0-wd01-part1-specification.txt
> > +++ b/virtio-v1.0-wd01-part1-specification.txt
> > @@ -1642,15 +1642,41 @@ host about the location used for its queue. The transmitted
> >  structure is
> >  
> >  struct vq_info_block {
> > +	__u64 desc;
> > +	__u32 res0;
> > +	__u16 index;
> > +	__u16 num;
> > +	__u64 avail;
> > +	__u64 used;
> > +} __attribute__ ((packed));
> > +
> > +desc, avail and used contain the guest addresses for the descriptor table,
> > +available ring and used ring for queue index, respectively. The actual
> > +virtqueue size (number of allocated buffers) is transmitted in num.
> > +res0 is reserved and must contain 0; otherwise, the device MUST post a
> > +unit check with command reject.
> > +
> > +If the revision selected by the driver is at least 1, the device MUST
> > +post a unit check with command reject if the transmitted data is between
> > +16 and 31 bytes if the driver suppressed incorrect length indication
> > +for the channel command. Otherwise, the normal conditions for handling
> > +incorrect data lenghts apply.
> 
> Also I don't understand the following: is there any
> flexibility for drivers wrt the transmitted data length?
> Above structure is 32 bytes in size.
> So any other length is a driver bug.

Not really. The driver may transmit a larger buffer then is needed, and
suppress length checking via a ccw flag. The device can then process
the data it needs, and disregard the rest. This is used sometimes for
variable-length responses where a driver can just supply the largest
possible buffer and check afterwards how much data it got. Depending on
the command, this may work with short buffers as well.

(In the virtio-ccw code so far, I required a minimum length and allowed
a larger length when length checks have been turned off.)

> 
> I'm guessing there's any
> number of other possibly invalid values that drivers can supply
> in some fields.
> E.g. stick a wrong PA outside RAM in one of the fields - seems
> more likely to happen actually.
> Why worry what happens then?

There are really two different cases there:

- The driver puts in values that are obviously incorrect, like a reserved
  field that is != 0 - this should be answered with a check, most likely
  a channel program check. (We may want to do this as well if an address
  can be immediately verified to be incorrect; many commands for other
  devices do.)
- The driver puts in junk that looks valid. The command will succeed,
  problems will happen later.

> 
> 
> > +2.3.3.2.2.1. Legacy Interface: A Note on Configuring a Virtqueue
> > +----------------------------------------------------------------
> > +
> > +For a legacy driver or for a driver that selected revision 0,
> > +CCW_CMD_SET_VQ uses the following communication block:
> > +
> > +struct vq_info_block_legacy {
> >  	__u64 queue;
> >  	__u32 align;
> >  	__u16 index;
> >  	__u16 num;
> >  } __attribute__ ((packed));
> >  
> > -queue contains the guest address for queue index. The actual
> > -number of allocated buffers is transmitted in num and their
> > -alignment in align.
> > +queue contains the guest address for queue index, num the number of buffers
> > +and align the alignment.
> >  
> >  100.3.3.2.2.  Virtqueue Layout
> >  ------------------------------
> > -- 
> > 1.7.9.5
> 



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]