Re: [Xen-devel] [PATCH V5 2/5] Add XEN pvSCSI protocol description

[Juergen Gross <jgross@xxxxxxxx>]

On 08/21/2014 09:26 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Aug 20, 2014 at 04:01:57PM +0200, Juergen Gross wrote:
> > On 08/20/2014 03:25 PM, Konrad Rzeszutek Wilk wrote:
> > > On Mon, Aug 18, 2014 at 11:31:47AM +0200, jgross@xxxxxxxx wrote:
...
> > > > +struct vscsiif_request {
> > > > +       uint16_t rqid;          /* private guest value, echoed in resp  */
> > > > +       uint8_t act;            /* command between backend and frontend */
> > > > +       uint8_t cmd_len;        /* valid CDB bytes */
> > > > +
> > > > +       uint8_t cmnd[VSCSIIF_MAX_COMMAND_SIZE]; /* the CDB */
> > > > +       uint16_t timeout_per_command;
> > > > +       uint16_t channel, id, lun;      /* (virtual) device specification */
> > > > +       uint16_t ref_rqid;              /* command abort reference */
> > > > +       uint8_t sc_data_direction;      /* for DMA_TO_DEVICE(1)
> > > > +                                          DMA_FROM_DEVICE(2)
> > > > +                                          DMA_NONE(3) requests */
> > > > +       uint8_t nr_segments;            /* Number of pieces of scatter-gather */
> > > > +#define VSCSIIF_SG_GRANT       0x80    /* flag: SG elements via grant page */
> > > > +                                       /* nr_segments counts grant pages with
> > > > +                                          SG elements
> > > Stop missing. However I am a bit lost. It says that the 'nr_segments' will have 
> > > the
> > > count of grant pages with SG elements. Does that mean the req.seg[0].gref points
> > > to an page which will have an array of grant references? And each grant 
> > > reference
> > > will point to a data page? What about the 'offset' and 'length' of them?
> > >
> > > Or does it mean that the 'reg.seg[0].gref' points an page that is filled with
> > > 'struct scsiif_request_segment' ? If so, where would the could of those
> > > segments be in? In 'nr_segments'? If that is so where does the VSCSIIF_SG_GRANT
> > > go? Won't we collide?
> > nr_segments (without the VSCSIIF_SG_GRANT bit) always counts the number
> > of populated seg[] entries. If VSCSIIF_SG_GRANT is set, each seg[] entry
> > references another array of struct scsiif_request_segment using a grant
> > ref, offset into the granted page and a length of the array in bytes.
> Shouldn't there be a comment about that? The blkif.h has a pretty lengthy
> one when it comes to indirect descriptors that could be copied as it
> sounds exactly like the same thing.
I already added the following:

/*
 * Request a SCSI operation specified via a CDB in vscsiif_request.cmnd.
 * The target is specified via channel, id and lun.
 * The operation to be performed is specified via a CDB in cmnd[], the 
length
 * of the CDB is in cmd_len. sc_data_direction specifies the direction 
of data
 * (to the device, from the device, or none at all).
 * If data is to be transferred to or from the device the buffer(s) in the
 * guest memory is/are specified via one or multiple scsiif_request_segment
 * descriptors each specifying a memory page via a grant_ref_t, a 
offset into
 * the page and the length of the area in that page. All 
scsiif_request_segment
 * areas concatenated form the resulting data buffer used by the operation.
 * If the number of scsiif_request_segment areas is not too large (less 
than
 * or equal VSCSIIF_SG_TABLESIZE) the areas can be specified directly 
in the
 * seg[] array and the number of valid scsiif_request_segment elements 
is to be
 * set in nr_segments.
 * If "feature-sg-grant" in the Xenstore is set it is possible to 
specify more
 * than VSCSIIF_SG_TABLESIZE scsiif_request_segment elements via 
indirection.
 * The maximum number of allowed scsiif_request_segment elements is the 
value
 * of the "feature-sg-grant" entry from Xenstore. When using 
indirection the
 * seg[] array doesn't contain specifications of the data buffers, but
 * references to scsiif_request_segment arrays, which in turn reference the
 * data buffers. While nr_segments holds the number of populated seg[] 
entries
 * (plus the set VSCSIIF_SG_GRANT bit), the number of 
scsiif_request_segment
 * elements referencing the target data buffers is calculated from the 
lengths
 * of the seg[] elements (the sum of all valid seg[].length divided by the
 * size of one scsiif_request_segment structure).
 */
#define VSCSIIF_ACT_SCSI_CDB           1

> > The resulting number of struct scsiif_request_segment is the sum of
> > seg[0..nr_segments-1].length / sizeof(struct scsiif_request_segment).
> Where the nr_segments can only go Up to VSCSIIF_SG_TABLESIZE, so the max
> total SG entries you can is 13312 ( 4096 / 8 = 512 max per page, times
> 26).
In theory, yes. SG_ALL (being 128 today) is limiting this value.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel