Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
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_picmd_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_
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, 2014-04-07 at 11:03 +0300, Sagi Grimberg wrote:
> 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_picmd_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?
>
Without the initiator providing a protection buffer, there is not much
else that can be done here.
So for the special case where the protect CDB is set, but no protection
buffer is provided, the WRITE_INSERT / READ_STRIP emulation should kick
in on the target side.
--nab
> > + 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);
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Sun, Apr 06, 2014 at 09:32:09PM +, 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
OK but we need to document the new interface in the spec
(and incidentially, this will be useful to verify the assumptions
made here and on the host side).
Could you please submit this proposal to the OASIS Virtio TC
for inclusion into the next spec draft?
Ideally as a patch against the tex source, but a prose
description would do as well.
The TC meets on a bi-weekly basis, we should be able to ratify
this quickly.
> ---
> 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_picmd_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))
> + 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))
> + 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,
> -
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > OK but we need to document the new interface in the spec > (and incidentially, this will be useful to verify the assumptions > made here and on the host side). > Could you please submit this proposal to the OASIS Virtio TC > for inclusion into the next spec draft? > Ideally as a patch against the tex source, but a prose > description would do as well. Most certainly. Please give me a bit to follow up on this, as the next couple of days are going to be hellishly busy.. > The TC meets on a bi-weekly basis, we should be able to ratify > this quickly. > Aside from that, please consider ACK'ing the vhost specific changes so these can make it into v3.15-rc1 code. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > OK but we need to document the new interface in the spec > > (and incidentially, this will be useful to verify the assumptions > > made here and on the host side). > > Could you please submit this proposal to the OASIS Virtio TC > > for inclusion into the next spec draft? > > Ideally as a patch against the tex source, but a prose > > description would do as well. > > Most certainly. Please give me a bit to follow up on this, as the next > couple of days are going to be hellishly busy.. > > > The TC meets on a bi-weekly basis, we should be able to ratify > > this quickly. > > > > Aside from that, please consider ACK'ing the vhost specific changes so > these can make it into v3.15-rc1 code. > > --nab > Hmm but what if the TC wants to change the interface somewhat? I guess we'll still be able to fix it after the merge window - (or worst case, revert the change) is this what you are suggesting? -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, 2014-04-07 at 12:02 +0300, Michael S. Tsirkin wrote: > On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > > > OK but we need to document the new interface in the spec > > > (and incidentially, this will be useful to verify the assumptions > > > made here and on the host side). > > > Could you please submit this proposal to the OASIS Virtio TC > > > for inclusion into the next spec draft? > > > Ideally as a patch against the tex source, but a prose > > > description would do as well. > > > > Most certainly. Please give me a bit to follow up on this, as the next > > couple of days are going to be hellishly busy.. > > > > > The TC meets on a bi-weekly basis, we should be able to ratify > > > this quickly. > > > > > > > Aside from that, please consider ACK'ing the vhost specific changes so > > these can make it into v3.15-rc1 code. > > > > --nab > > > > Hmm but what if the TC wants to change the interface somewhat? I don't have a objection to changing the interface post-merge. > I guess we'll still be able to fix it after the merge window - > (or worst case, revert the change) is this what you are suggesting? > Yes, but I would think that it is actually fixable. ;) Otherwise, a v3.16 merge is an option as well. It's really your + Paolo's call here. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
Il 07/04/2014 05:13, Nicholas A. Bellinger ha scritto: > I guess we'll still be able to fix it after the merge window - > (or worst case, revert the change) is this what you are suggesting? > Yes, but I would think that it is actually fixable. ;) Otherwise, a v3.16 merge is an option as well. It's really your + Paolo's call here. With s/niov/nbytes/, I think the interface is fine. If you can do it this week, it is your call as target maintainer... I don't have to do much more than give my acked-by for the changes to the virtio-scsi drivers, it's up to you whether to use it for 3.15 or 3.16. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > OK but we need to document the new interface in the spec > > (and incidentially, this will be useful to verify the assumptions > > made here and on the host side). > > Could you please submit this proposal to the OASIS Virtio TC > > for inclusion into the next spec draft? > > Ideally as a patch against the tex source, but a prose > > description would do as well. > > Most certainly. Please give me a bit to follow up on this, as the next > couple of days are going to be hellishly busy.. Ping. We really need to get this moving to have the interface reviewed for the next merge window. > > The TC meets on a bi-weekly basis, we should be able to ratify > > this quickly. > > > > Aside from that, please consider ACK'ing the vhost specific changes so > these can make it into v3.15-rc1 code. > > --nab > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Wed, 2014-05-07 at 12:13 +0300, Michael S. Tsirkin wrote: > On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > > > OK but we need to document the new interface in the spec > > > (and incidentially, this will be useful to verify the assumptions > > > made here and on the host side). > > > Could you please submit this proposal to the OASIS Virtio TC > > > for inclusion into the next spec draft? > > > Ideally as a patch against the tex source, but a prose > > > description would do as well. > > > > Most certainly. Please give me a bit to follow up on this, as the next > > couple of days are going to be hellishly busy.. > > Ping. > We really need to get this moving to have the interface reviewed for > the next merge window. > Hi MST, So I've finally got some cycles to get back to this code, and wanted to verify the outstanding items you had previously raised: - Convert vhost-scsi to be independent of IOV layout using memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi operation..?) - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of iovecs. - Ensure virtio_scsi_cmd_req_pi is naturally aligned - Figure out why QEMU is not acking (any) vhost-scsi feature bits Is there anything else that you'd like to see for an initial merge, or other issues that need to be addressed with the above..? --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, May 19, 2014 at 12:07:03PM -0700, Nicholas A. Bellinger wrote: > On Wed, 2014-05-07 at 12:13 +0300, Michael S. Tsirkin wrote: > > On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > > > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > > > > > OK but we need to document the new interface in the spec > > > > (and incidentially, this will be useful to verify the assumptions > > > > made here and on the host side). > > > > Could you please submit this proposal to the OASIS Virtio TC > > > > for inclusion into the next spec draft? > > > > Ideally as a patch against the tex source, but a prose > > > > description would do as well. > > > > > > Most certainly. Please give me a bit to follow up on this, as the next > > > couple of days are going to be hellishly busy.. > > > > Ping. > > We really need to get this moving to have the interface reviewed for > > the next merge window. > > > > Hi MST, > > So I've finally got some cycles to get back to this code, and wanted to > verify the outstanding items you had previously raised: > > - Convert vhost-scsi to be independent of IOV layout using > memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi > operation..?) Ideally yes. > - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. > - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of > iovecs. > - Ensure virtio_scsi_cmd_req_pi is naturally aligned It turns out other virtio scsi commands aren't aligned? If true we don't need to make an exception here. > - Figure out why QEMU is not acking (any) vhost-scsi feature bits > > Is there anything else that you'd like to see for an initial merge, or > other issues that need to be addressed with the above..? > > --nab FYI Paolo sent a spec patch for the feature. Have you seen it? -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, 2014-05-19 at 22:15 +0300, Michael S. Tsirkin wrote: > On Mon, May 19, 2014 at 12:07:03PM -0700, Nicholas A. Bellinger wrote: > > On Wed, 2014-05-07 at 12:13 +0300, Michael S. Tsirkin wrote: > > > On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > > > > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > > > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > > > > > > > OK but we need to document the new interface in the spec > > > > > (and incidentially, this will be useful to verify the assumptions > > > > > made here and on the host side). > > > > > Could you please submit this proposal to the OASIS Virtio TC > > > > > for inclusion into the next spec draft? > > > > > Ideally as a patch against the tex source, but a prose > > > > > description would do as well. > > > > > > > > Most certainly. Please give me a bit to follow up on this, as the next > > > > couple of days are going to be hellishly busy.. > > > > > > Ping. > > > We really need to get this moving to have the interface reviewed for > > > the next merge window. > > > > > > > Hi MST, > > > > So I've finally got some cycles to get back to this code, and wanted to > > verify the outstanding items you had previously raised: > > > > - Convert vhost-scsi to be independent of IOV layout using > > memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi > > operation..?) > > Ideally yes. Er, so changing vhost-scsi to be independent of IOV layout will have the side effect of breaking existing non PI virtio-scsi logic..? > > > - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. > > - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of > > iovecs. > > - Ensure virtio_scsi_cmd_req_pi is naturally aligned > > It turns out other virtio scsi commands aren't aligned? Correct, virtio_scsi_cmd_req is currently 51 bytes. > If true we don't need to make an exception here. > > > - Figure out why QEMU is not acking (any) vhost-scsi feature bits > > > > Is there anything else that you'd like to see for an initial merge, or > > other issues that need to be addressed with the above..? > > > > --nab > > FYI Paolo sent a spec patch for the feature. > Have you seen it? > Yep, looks fine. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Mon, May 19, 2014 at 01:54:50PM -0700, Nicholas A. Bellinger wrote: > On Mon, 2014-05-19 at 22:15 +0300, Michael S. Tsirkin wrote: > > On Mon, May 19, 2014 at 12:07:03PM -0700, Nicholas A. Bellinger wrote: > > > On Wed, 2014-05-07 at 12:13 +0300, Michael S. Tsirkin wrote: > > > > On Mon, Apr 07, 2014 at 01:56:59AM -0700, Nicholas A. Bellinger wrote: > > > > > On Mon, 2014-04-07 at 11:45 +0300, Michael S. Tsirkin wrote: > > > > > > On Sun, Apr 06, 2014 at 09:32:09PM +, 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 > > > > > > > > > > > > OK but we need to document the new interface in the spec > > > > > > (and incidentially, this will be useful to verify the assumptions > > > > > > made here and on the host side). > > > > > > Could you please submit this proposal to the OASIS Virtio TC > > > > > > for inclusion into the next spec draft? > > > > > > Ideally as a patch against the tex source, but a prose > > > > > > description would do as well. > > > > > > > > > > Most certainly. Please give me a bit to follow up on this, as the > > > > > next > > > > > couple of days are going to be hellishly busy.. > > > > > > > > Ping. > > > > We really need to get this moving to have the interface reviewed for > > > > the next merge window. > > > > > > > > > > Hi MST, > > > > > > So I've finally got some cycles to get back to this code, and wanted to > > > verify the outstanding items you had previously raised: > > > > > > - Convert vhost-scsi to be independent of IOV layout using > > > memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi > > > operation..?) > > > > Ideally yes. > > Er, so changing vhost-scsi to be independent of IOV layout will have the > side effect of breaking existing non PI virtio-scsi logic..? Sorry I didn't make myself clear. I merely think that all code should do memcpy_fromiovecend etc. If done peoperly guests will not notice unless they test VIRTIO_F_ANY_LAYOUT. > > > > > - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. > > > - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of > > > iovecs. > > > - Ensure virtio_scsi_cmd_req_pi is naturally aligned > > > > It turns out other virtio scsi commands aren't aligned? > > Correct, virtio_scsi_cmd_req is currently 51 bytes. > > > If true we don't need to make an exception here. > > > > > > > > - Figure out why QEMU is not acking (any) vhost-scsi feature bits > > > > > > Is there anything else that you'd like to see for an initial merge, or > > > other issues that need to be addressed with the above..? > > > > > > --nab > > > > FYI Paolo sent a spec patch for the feature. > > Have you seen it? > > > > Yep, looks fine. > > --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
Il 19/05/2014 21:07, Nicholas A. Bellinger ha scritto: Hi MST, So I've finally got some cycles to get back to this code, and wanted to verify the outstanding items you had previously raised: - Convert vhost-scsi to be independent of IOV layout using memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi operation..?) - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of iovecs. This is the only item really required, since the bytes field is what will be in VIRTIO 1.0. The other would be nice to have, but not a blocker for PI support. - Ensure virtio_scsi_cmd_req_pi is naturally aligned mst already commented on this. - Figure out why QEMU is not acking (any) vhost-scsi feature bits This is a separate bug, isn't it? It need not block PI support. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] virtio-scsi: Enable DIF/DIX modes in SCSI host LLD
On Tue, 2014-05-20 at 10:35 +0200, Paolo Bonzini wrote: > Il 19/05/2014 21:07, Nicholas A. Bellinger ha scritto: > > Hi MST, > > > > So I've finally got some cycles to get back to this code, and wanted to > > verify the outstanding items you had previously raised: > > > > - Convert vhost-scsi to be independent of IOV layout using > > memcpy_fromiovecend. (Does this effect existing non PI virtio-scsi > > operation..?) > > - Report VIRTIO_F_ANY_LAYOUT feature bit to userspace. > > - Convert virtio_scsi_cmd_req_pi to bytes field instead of number of > > iovecs. > > This is the only item really required, since the bytes field is what > will be in VIRTIO 1.0. The other would be nice to have, but not a > blocker for PI support. > > > - Ensure virtio_scsi_cmd_req_pi is naturally aligned > > mst already commented on this. > > > - Figure out why QEMU is not acking (any) vhost-scsi feature bits > > This is a separate bug, isn't it? It need not block PI support. > Thanks for the feedback. I'll get the series updated to use bytes instead of number of iovecs in virtio_scsi_cmd_req_pi, along with a simple memcpy_fromiovecend conversion. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
