From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagi Grimberg Subject: Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD Date: Mon, 07 Apr 2014 11:03:16 +0300 Message-ID: <53425BC4.4040707@dev.mellanox.co.il> References: <1396819929-29687-1-git-send-email-nab@daterainc.com> <1396819929-29687-7-git-send-email-nab@daterainc.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f48.google.com ([74.125.82.48]:43145 "EHLO mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752977AbaDGIDV (ORCPT ); Mon, 7 Apr 2014 04:03:21 -0400 Received: by mail-wg0-f48.google.com with SMTP id l18so6363166wgh.31 for ; Mon, 07 Apr 2014 01:03:20 -0700 (PDT) In-Reply-To: <1396819929-29687-7-git-send-email-nab@daterainc.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: "Nicholas A. Bellinger" , target-devel Cc: linux-scsi , "Michael S. Tsirkin" , Paolo Bonzini , "Martin K. Petersen" , Sagi Grimberg , Christoph Hellwig , Hannes Reinecke , "H. Peter Anvin" , Nicholas Bellinger On 4/7/2014 12:32 AM, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > This patch updates virtscsi_probe() to setup necessary Scsi_Host > level protection resources. (currently hardcoded to 1) > > It changes virtscsi_add_cmd() to attach outgoing / incoming > protection SGLs preceeding the data payload, and is using the > new virtio_scsi_cmd_req_pi->d[oi],pi_niv field to signal > to signal to vhost/scsi how many prot_sgs to expect. > > v3 changes: > - Use VIRTIO_SCSI_F_T10_PI to determine PI or non PI header (Paolo) > > v2 changes: > - Make protection buffer come before data buffer (Paolo) > - Enable virtio_scsi_cmd_req_pi usage (Paolo) > > Cc: Paolo Bonzini > Cc: Michael S. Tsirkin > Cc: Martin K. Petersen > Cc: Christoph Hellwig > Cc: Hannes Reinecke > Cc: Sagi Grimberg > Cc: H. Peter Anvin > Signed-off-by: Nicholas Bellinger > --- > drivers/scsi/virtio_scsi.c | 78 ++++++++++++++++++++++++++++++++++---------- > 1 file changed, 60 insertions(+), 18 deletions(-) > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c > index 16bfd50..68d8d1b 100644 > --- a/drivers/scsi/virtio_scsi.c > +++ b/drivers/scsi/virtio_scsi.c > @@ -37,6 +37,7 @@ struct virtio_scsi_cmd { > struct completion *comp; > union { > struct virtio_scsi_cmd_req cmd; > + struct virtio_scsi_cmd_req_pi cmd_pi; > struct virtio_scsi_ctrl_tmf_req tmf; > struct virtio_scsi_ctrl_an_req an; > } req; > @@ -440,7 +441,7 @@ static int virtscsi_add_cmd(struct virtqueue *vq, > size_t req_size, size_t resp_size, gfp_t gfp) > { > struct scsi_cmnd *sc = cmd->sc; > - struct scatterlist *sgs[4], req, resp; > + struct scatterlist *sgs[6], req, resp; > struct sg_table *out, *in; > unsigned out_num = 0, in_num = 0; > > @@ -458,16 +459,24 @@ static int virtscsi_add_cmd(struct virtqueue *vq, > sgs[out_num++] = &req; > > /* Data-out buffer. */ > - if (out) > + if (out) { > + /* Place WRITE protection SGLs before Data OUT payload */ > + if (scsi_prot_sg_count(sc)) Didn't we agree that the LLD should not discard the scmnd prot_op? Martin? > + sgs[out_num++] = scsi_prot_sglist(sc); > sgs[out_num++] = out->sgl; > + } > > /* Response header. */ > sg_init_one(&resp, &cmd->resp, resp_size); > sgs[out_num + in_num++] = &resp; > > /* Data-in buffer */ > - if (in) > + if (in) { > + /* Place READ protection SGLs before Data IN payload */ > + if (scsi_prot_sg_count(sc)) Same here. > + sgs[out_num + in_num++] = scsi_prot_sglist(sc); > sgs[out_num + in_num++] = in->sgl; > + } > > return virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, gfp); > } > @@ -492,12 +501,36 @@ static int virtscsi_kick_cmd(struct virtio_scsi_vq *vq, > return err; > } > > +static void virtio_scsi_init_hdr(struct virtio_scsi_cmd_req *cmd, > + struct scsi_cmnd *sc) > +{ > + cmd->lun[0] = 1; > + cmd->lun[1] = sc->device->id; > + cmd->lun[2] = (sc->device->lun >> 8) | 0x40; > + cmd->lun[3] = sc->device->lun & 0xff; > + cmd->tag = (unsigned long)sc; > + cmd->task_attr = VIRTIO_SCSI_S_SIMPLE; > + cmd->prio = 0; > + cmd->crn = 0; > +} > + > +static void virtio_scsi_init_hdr_pi(struct virtio_scsi_cmd_req_pi *cmd_pi, > + struct scsi_cmnd *sc) > +{ > + virtio_scsi_init_hdr((struct virtio_scsi_cmd_req *)cmd_pi, sc); > + > + if (sc->sc_data_direction == DMA_TO_DEVICE) > + cmd_pi->do_pi_niov = scsi_prot_sg_count(sc); > + else if (sc->sc_data_direction == DMA_FROM_DEVICE) > + cmd_pi->di_pi_niov = scsi_prot_sg_count(sc); > +} > + > static int virtscsi_queuecommand(struct virtio_scsi *vscsi, > struct virtio_scsi_vq *req_vq, > struct scsi_cmnd *sc) > { > struct virtio_scsi_cmd *cmd; > - int ret; > + int ret, req_size; > > struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev); > BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize); > @@ -515,22 +548,20 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi, > > memset(cmd, 0, sizeof(*cmd)); > cmd->sc = sc; > - cmd->req.cmd = (struct virtio_scsi_cmd_req){ > - .lun[0] = 1, > - .lun[1] = sc->device->id, > - .lun[2] = (sc->device->lun >> 8) | 0x40, > - .lun[3] = sc->device->lun & 0xff, > - .tag = (unsigned long)sc, > - .task_attr = VIRTIO_SCSI_S_SIMPLE, > - .prio = 0, > - .crn = 0, > - }; > > BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE); > - memcpy(cmd->req.cmd.cdb, sc->cmnd, sc->cmd_len); > > - if (virtscsi_kick_cmd(req_vq, cmd, > - sizeof cmd->req.cmd, sizeof cmd->resp.cmd, > + if (virtio_has_feature(vscsi->vdev, VIRTIO_SCSI_F_T10_PI)) { > + virtio_scsi_init_hdr_pi(&cmd->req.cmd_pi, sc); > + memcpy(cmd->req.cmd_pi.cdb, sc->cmnd, sc->cmd_len); > + req_size = sizeof(cmd->req.cmd_pi); > + } else { > + virtio_scsi_init_hdr(&cmd->req.cmd, sc); > + memcpy(cmd->req.cmd.cdb, sc->cmnd, sc->cmd_len); > + req_size = sizeof(cmd->req.cmd); > + } > + > + if (virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd), > GFP_ATOMIC) == 0) > ret = 0; > else > @@ -871,7 +902,7 @@ static int virtscsi_probe(struct virtio_device *vdev) > { > struct Scsi_Host *shost; > struct virtio_scsi *vscsi; > - int err; > + int err, host_prot; > u32 sg_elems, num_targets; > u32 cmd_per_lun; > u32 num_queues; > @@ -921,6 +952,16 @@ static int virtscsi_probe(struct virtio_device *vdev) > shost->max_id = num_targets; > shost->max_channel = 0; > shost->max_cmd_len = VIRTIO_SCSI_CDB_SIZE; > + > + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_T10_PI)) { > + host_prot = SHOST_DIF_TYPE1_PROTECTION | SHOST_DIF_TYPE2_PROTECTION | > + SHOST_DIF_TYPE3_PROTECTION | SHOST_DIX_TYPE1_PROTECTION | > + SHOST_DIX_TYPE2_PROTECTION | SHOST_DIX_TYPE3_PROTECTION; > + > + scsi_host_set_prot(shost, host_prot); > + scsi_host_set_guard(shost, SHOST_DIX_GUARD_CRC); > + } > + > err = scsi_add_host(shost, &vdev->dev); > if (err) > goto scsi_add_host_failed; > @@ -990,6 +1031,7 @@ static struct virtio_device_id id_table[] = { > static unsigned int features[] = { > VIRTIO_SCSI_F_HOTPLUG, > VIRTIO_SCSI_F_CHANGE, > + VIRTIO_SCSI_F_T10_PI, > }; > > static struct virtio_driver virtio_scsi_driver = {