From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] blkif: add indirect descriptors interface to public headers Date: Tue, 12 Nov 2013 09:22:54 -0500 Message-ID: <20131112142254.GC9306@phenom.dumpdata.com> References: <1384252612-22573-1-git-send-email-roger.pau@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD017021D@AMSPEX01CL01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VgErt-0007CG-Vp for xen-devel@lists.xenproject.org; Tue, 12 Nov 2013 14:23:02 +0000 Content-Disposition: inline In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD017021D@AMSPEX01CL01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant Cc: "xen-devel@lists.xenproject.org" , "Keir (Xen.org)" , Jan Beulich , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On Tue, Nov 12, 2013 at 01:46:19PM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > > bounces@lists.xen.org] On Behalf Of Roger Pau Monne > > Sent: 12 November 2013 10:37 > > To: xen-devel@lists.xenproject.org > > 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 >=3D 3.11. > > = > > Signed-off-by: Roger Pau Monn=E9 > > Cc: Keir Fraser > > Cc: Jan Beulich > > --- > > 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/blki= f.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 bac= kend > > + * 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 o= rder 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_gre= fs > > field > > + * in blkif_request_indirect should be filled by the frontend with the > > + * grant references of the pages that are holding the indirect segment= s. > > + * 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 max= imum > > + * 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) <=3D PAGE_SIZ= E. > > * 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 un= its. > > * 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 M= SVC will 8-byte align it. > = Oh no. I thought that the Linux one had this set correctly, ah it did: = = struct blkif_request_indirect { = = uint8_t indirect_op; = = uint16_t nr_segments; = = #ifdef CONFIG_X86_64 = = uint32_t _pad1; /* offsetof(blkif_...,u.indirect.id) = =3D=3D 8 */ #endif = = uint64_t id; = = blkif_sector_t sector_number; = = blkif_vdev_t handle; = = uint16_t _pad2; = = grant_ref_t indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST]= ; = #ifdef CONFIG_X86_64 = = uint32_t _pad3; /* make it 64 byte aligned */ = = #else = = uint64_t _pad3; /* make it 64 byte aligned */ = = #endif = = } __attribute__((__packed__)); = Wheew. > Paul > = > > + uint64_t id; /* private guest value, echoed in res= p */ > > + 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-ali= gned */ > > +}; > > + > > 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@lists.xen.org > > http://lists.xen.org/xen-devel > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel