Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752617AbaBYGCH (ORCPT ); Tue, 25 Feb 2014 01:02:07 -0500 Received: from mail.linux-iscsi.org ([67.23.28.174]:41049 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751439AbaBYGCE (ORCPT ); Tue, 25 Feb 2014 01:02:04 -0500 Message-ID: <1393308311.12334.14.camel@haakon3.risingtidesystems.com> Subject: Re: [RFC 0/6] vhost/scsi: Add T10 PI SGL passthrough support From: "Nicholas A. Bellinger" To: Paolo Bonzini Cc: "Nicholas A. Bellinger" , target-devel , linux-scsi , linux-kernel , kvm-devel , "Michael S. Tsirkin" , "Martin K. Petersen" , Christoph Hellwig , Hannes Reinecke , Sagi Grimberg Date: Mon, 24 Feb 2014 22:05:11 -0800 In-Reply-To: <530B1DAF.7010702@redhat.com> References: <1393219950-18613-1-git-send-email-nab@daterainc.com> <530B1DAF.7010702@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2014-02-24 at 11:23 +0100, Paolo Bonzini wrote: > Il 24/02/2014 06:32, Nicholas A. Bellinger ha scritto: > > AFAICT up until this point the ->prio field has been unused, but > > I'm certainly open to better ways of signaling (to vhost) that some > > number of metadata iovs are to be expected.. Any thoughts..? > > Hi nab, > > the virtio-scsi side of the patch is nice and readable. As requested, > here are my thoughts on how to add it to the standard. > > The ->prio field is there to mimic SAM's command priority field (8.7 in > my copy of the standard). I'd rather leave it alone; I understand this > is the main reason why this patch is RFC. Yes. ;) > > Since we have a new feature bit, we can add a new element before the > cdb. It could be a count of scatter/gather list like you did here, or > it could be a byte count. Even better, we can add _two_ new fields, one > for protection data out and one for protection data in. > Having two 16-bit fields for data out / data in protection count in the command header should be fine. So that said, adding a new virtio_scsi_cmd_req_pi definition per your recommendation, and will update the series to use this when the VIRTIO_SCSI_F_T10_PI feature bit has been negotiated on both ends. > Also, do we need an equivalent of the residual field, but for metadata? > Mmm, at least for PI I don't think a residual field is necessary. Any time the metadata is not fully read on outgoing WRITEs, or written on incoming READs the next hop performing a VERIFY operation will end up failing with a GUARD or REFERENCE TAG failure. MKP..? > Finally, any reason why you put the data sg elements before the metadata > sg elements? Nope, no particular reason for this. > I would have thought that processing is a bit simpler if > either the metadata comes first, or you store in the command header the > data count (either sg or byte). Because the virtio buffers form a > linked list, it's a bit backwards to put metadata last, and store > metadata count in the command header; it prevents you from processing > the buffers online because you don't know when the metadata starts. > Even though the Linux virtio layer always gives you a buffer count, this > need not be the case in general. > No objection here. Updating the patch series to place protection information ahead of the actual data payload. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/