Re: vhost-scsi support for ANY_LAYOUT

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

On Tue, Jan 27, 2015 at 09:42:22AM +0100, Paolo Bonzini wrote:
> 
> 
> On 27/01/2015 07:35, Nicholas A. Bellinger wrote:
> > Hi MST & Paolo,
> > 
> > So I'm currently working on vhost-scsi support for ANY_LAYOUT, and
> > wanted to verify some assumptions based upon your earlier emails..
> > 
> > *) When ANY_LAYOUT is negotiated by vhost-scsi, it's expected that
> > virtio-scsi request + response headers will (always..?) be within a
> > single iovec.
> > 
> > *) When ANY_LAYOUT is negotiated by vhost-scsi, it's expected that
> > virtio-scsi request + response headers may be (but not always..?)
> > combined with data-out + data-in payloads into a single iovec.
> > 
> > *) When ANY_LAYOUT + T10_PI is negotiated by vhost-scsi, it's expected
> > that PI and data payloads for data-out + data-in may be (but not
> > always..?) within the same iovec.  Consequently, both headers + PI +
> > data-payloads may also be within a single iovec.
> 
> s/expected/possible/g
> 
> Any split between header and payload is possible.  It's also possible to
> split the header in multiple parts, e.g. put the CDB or sense buffer in
> a separate iovec.  Even a single field could be split across two iovecs.
> 
> > *) Is it still safe to use 'out' + 'in' values from vhost_get_vq_desc()
> > in order to determine the data_direction...?  If not, what's the
> > preferred way of determining this information for get_user_pages_fast()
> > permission bits and target_submit_cmd_map_sgls()..?
> 
> No, it's not safe.  What QEMU does is to check if the output buffers are
> collectively larger than the request header, and if the input buffers
> are collectively larger than the response header.  This lets you cpmpute
> the data direction.

Generally true, but I'd like to clarify.

I think what we can say is that if there's in value,
host is expected to write data, and if there's out value,
to read data.

But this includes the headers so generally you need to get total length
for in and/or out (separately) and subtract header length.
But if you know there's e.g. no in header, then you can use in value
to figure out gup flags.
Or if you see there's no in data, you know you can use gup with read
only flags.


> > Also, what is required on the QEMU side in order to start generating
> > ANY_LAYOUT style iovecs to verify the WIP changes..? 
> 
> Nothing on the QEMU side.  You could test any-layout payloads by
> changing the kernel side.  Splitting the sense buffer into its own iovec
> for example is a trivial patch to drivers/scsi/virtio_scsi.c.
> 
> It also helps to adhere to coding conventions.  For example in QEMU I'm
> never using iov[N], and I'm restricting usage of iovcnt to (iov, iovcnt)
> parameter pairs.  This was suggested by Michael and found a couple bugs
> indeed.
> 
> Paolo