Re: [Xen-devel] [PATCH] blkif: add indirect descriptors interface to public headers

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxx] On Behalf Of Roger Pau Monne
> Sent: 12 November 2013 10:37
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Keir (Xen.org); Jan Beulich; Roger Pau Monne
> Subject: [Xen-devel] [PATCH] blkif: add indirect descriptors interface to
> public headers
> 
> Indirect descriptors introduce a new block operation
> (BLKIF_OP_INDIRECT) that passes grant references instead of segments
> in the request. This grant references are filled with arrays of
> blkif_request_segment_aligned, this way we can send more segments in a
> request.
> 
> This interface is already implemented in Linux >= 3.11.
> 
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> ---
>  xen/include/public/io/blkif.h |   51
> +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index b9b9d98..84eb7fd 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -468,6 +468,30 @@
>  #define BLKIF_OP_DISCARD           5
> 
>  /*
> + * Recognized if "feature-max-indirect-segments" in present in the backend
> + * xenbus info. The "feature-max-indirect-segments" node contains the
> maximum
> + * number of segments allowed by the backend per request. If the node is
> + * present, the frontend might use blkif_request_indirect structs in order to
> + * issue requests with more than BLKIF_MAX_SEGMENTS_PER_REQUEST
> (11). The
> + * maximum number of indirect segments is fixed by the backend, but the
> + * frontend can issue requests with any number of indirect segments as
> long as
> + * it's less than the number provided by the backend. The indirect_grefs
> field
> + * in blkif_request_indirect should be filled by the frontend with the
> + * grant references of the pages that are holding the indirect segments.
> + * This pages are filled with an array of blkif_request_segment_aligned
> + * that hold the information about the segments. The number of indirect
> + * pages to use is determined by the maximum number of segments
> + * an indirect request contains. Every indirect page can contain a maximum
> + * of 512 segments (PAGE_SIZE/sizeof(blkif_request_segment_aligned)),
> + * so to calculate the number of indirect pages to use we have to do
> + * ceil(indirect_segments/512).
> + *
> + * If a backend does not recognize BLKIF_OP_INDIRECT, it should *not*
> + * create the "feature-max-indirect-segments" node!
> + */
> +#define BLKIF_OP_INDIRECT          6
> +
> +/*
>   * Maximum scatter/gather segments per request.
>   * This is carefully chosen so that sizeof(blkif_ring_t) <= PAGE_SIZE.
>   * NB. This could be 12 if the ring indexes weren't stored in the same page.
> @@ -475,6 +499,11 @@
>  #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11
> 
>  /*
> + * Maximum number of indirect pages to use per request.
> + */
> +#define BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST 8
> +
> +/*
>   * NB. first_sect and last_sect in blkif_request_segment, as well as
>   * sector_number in blkif_request, are always expressed in 512-byte units.
>   * However they must be properly aligned to the real sector size of the
> @@ -517,6 +546,28 @@ struct blkif_request_discard {
>  };
>  typedef struct blkif_request_discard blkif_request_discard_t;
> 
> +struct blkif_request_indirect {
> +    uint8_t        operation;    /* BLKIF_OP_INDIRECT                    */
> +    uint8_t        indirect_op;  /* BLKIF_OP_{READ/WRITE}                */
> +    uint16_t       nr_segments;  /* number of segments                   */

This is going to be a problem. What alignment boundary are you expecting the 
next field to start on? AFAIK 32-bit gcc will 4-byte align it, 32-bit MSVC will 
8-byte align it.

  Paul

> +    uint64_t       id;           /* private guest value, echoed in resp  */
> +    blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
> +    blkif_vdev_t   handle;       /* same as for read/write requests      */
> +    grant_ref_t
> indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST];
> +#ifdef __i386__
> +    uint64_t       pad;          /* Make it 64 byte aligned on i386      */
> +#endif
> +};
> +typedef struct blkif_request_indirect blkif_request_indirect_t;
> +
> +struct blkif_request_segment_aligned {
> +    grant_ref_t gref;            /* reference to I/O buffer frame        */
> +    /* @first_sect: first sector in frame to transfer (inclusive).   */
> +    /* @last_sect: last sector in frame to transfer (inclusive).     */
> +    uint8_t     first_sect, last_sect;
> +    uint16_t    _pad; /* padding to make it 8 bytes, so it's cache-aligned */
> +};
> +
>  struct blkif_response {
>      uint64_t        id;              /* copied from request */
>      uint8_t         operation;       /* copied from request */
> --
> 1.7.7.5 (Apple Git-26)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel