From patchwork Wed Feb 4 10:55:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 5776011 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A04089F302 for ; Wed, 4 Feb 2015 10:55:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BA2A8201FA for ; Wed, 4 Feb 2015 10:55:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 84FA3201E4 for ; Wed, 4 Feb 2015 10:55:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965127AbbBDKzR (ORCPT ); Wed, 4 Feb 2015 05:55:17 -0500 Received: from mail.linux-iscsi.org ([67.23.28.174]:50444 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964864AbbBDKzO (ORCPT ); Wed, 4 Feb 2015 05:55:14 -0500 Received: from [192.168.1.65] (75-37-194-224.lightspeed.lsatca.sbcglobal.net [75.37.194.224]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 2613A22D9A6; Wed, 4 Feb 2015 10:22:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1423045338; bh=3xpx298THlkaQzw2/CqkX15adoJgBX9 zzvY/3ybdroo=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=M0NTtBZsIm+GIl/0RtiwURLhPvsaFnB0Is5baVvcGyuXHUb0ojr+o7ADsRjmqcZe3 y+O7DoT+eolg8eBWoTJ8y5f+05Z9jGY2kVKgT67aD7Uqn4zK6e+Zt99B3qMPL5Qy1sN G9XvQgXxHbXY+EmxUH1IW4X8O+/aWkcvvkDw0+c= Message-ID: <1423047312.28700.51.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH-v3 5/9] vhost/scsi: Add ANY_LAYOUT vhost_virtqueue callback From: "Nicholas A. Bellinger" To: "Michael S. Tsirkin" Cc: "Nicholas A. Bellinger" , target-devel , linux-scsi , kvm-devel , Paolo Bonzini , Al Viro , Christoph Hellwig Date: Wed, 04 Feb 2015 02:55:12 -0800 In-Reply-To: <1423046467.28700.48.camel@haakon3.risingtidesystems.com> References: <1422945003-24538-1-git-send-email-nab@daterainc.com> <1422945003-24538-6-git-send-email-nab@daterainc.com> <20150203101436.GO2830@redhat.com> <1423042825.28700.26.camel@haakon3.risingtidesystems.com> <20150204094252.GI9920@redhat.com> <1423046467.28700.48.camel@haakon3.risingtidesystems.com> X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2015-02-04 at 02:41 -0800, Nicholas A. Bellinger wrote: > On Wed, 2015-02-04 at 10:42 +0100, Michael S. Tsirkin wrote: > > On Wed, Feb 04, 2015 at 01:40:25AM -0800, Nicholas A. Bellinger wrote: > > > > > + /* > > > > > + * Any associated T10_PI bytes for the outgoing / incoming > > > > > + * payloads are included in calculation of exp_data_len here. > > > > > + */ > > > > > + if (out_size > req_size) { > > > > > + data_direction = DMA_TO_DEVICE; > > > > > + exp_data_len = out_size - req_size; > > > > > + } else if (in_size > rsp_size) { > > > > > + data_direction = DMA_FROM_DEVICE; > > > > > + exp_data_len = in_size - rsp_size; > > > > > + } else { > > > > > + data_direction = DMA_NONE; > > > > > + exp_data_len = 0; > > > > > + } > > > > > > > > We must validate this doesn't cause exp_data_len to be negative. > > > > > > > > > > AFAICT, exp_data_len is always >= 0 here. > > > > What guarantees out_size > req_size and in_size > rsp_size, > > respectively? > > > > Mmm, point taken. > > So moving this part after copy_from_iter() ensures that at least > req_size bytes exists of out_size. Making this change now. > > For in_size > rsp_size there is no guarantee, and falls back to > data_direction = DMA_NONE + exp_data_len = 0; > > Is this what you had in mind..? > Something akin to: --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index f72fc56..daf10b7 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -1047,30 +1047,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq) target = &v_req.lun[1]; } /* - * Determine data_direction by calculating the total outgoing - * iovec sizes / incoming iovec sizes vs. virtio-scsi request / - * response headers respectively. - * * FIXME: Not correct for BIDI operation */ out_size = iov_length(vq->iov, out); in_size = iov_length(&vq->iov[out], in); /* - * Any associated T10_PI bytes for the outgoing / incoming - * payloads are included in calculation of exp_data_len here. - */ - if (out_size > req_size) { - data_direction = DMA_TO_DEVICE; - exp_data_len = out_size - req_size; - } else if (in_size > rsp_size) { - data_direction = DMA_FROM_DEVICE; - exp_data_len = in_size - rsp_size; - } else { - data_direction = DMA_NONE; - exp_data_len = 0; - } - /* * Copy over the virtio-scsi request header, which for a * ANY_LAYOUT enabled guest may span multiple iovecs, or a * single iovec may contain both the header + outgoing @@ -1102,8 +1084,9 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq) continue; } /* - * Determine start of T10_PI or data payload iovec based upon - * data_direction. + * Determine data_direction by calculating the total outgoing + * iovec sizes + incoming iovec sizes vs. virtio-scsi request + + * response headers respectively. * * For DMA_TO_DEVICE this is out_iter, which is already pointing * to the right place. @@ -1111,16 +1094,27 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq) * For DMA_FROM_DEVICE, the iovec will be just past the end * of the virtio-scsi response header in either the same * or immediately following iovec. + * + * Any associated T10_PI bytes for the outgoing / incoming + * payloads are included in calculation of exp_data_len here. */ prot_bytes = 0; - if (data_direction == DMA_TO_DEVICE) { + if (out_size > req_size) { + data_direction = DMA_TO_DEVICE; + exp_data_len = out_size - req_size; data_iter = out_iter; - } else if (data_direction == DMA_FROM_DEVICE) { + } else if (in_size > rsp_size) { + data_direction = DMA_FROM_DEVICE; + exp_data_len = in_size - rsp_size; + iov_iter_init(&in_iter, READ, &vq->iov[out], in, rsp_size + exp_data_len); iov_iter_advance(&in_iter, rsp_size); data_iter = in_iter; + } else { + data_direction = DMA_NONE; + exp_data_len = 0; } /* * If T10_PI header + payload is present, setup prot_iter values