[PATCH v7 42/46] virtio_scsi: v1.0 support
Note: for consistency, and to avoid sparse errors, convert all fields, even those no longer in use for virtio v1.0. Signed-off-by: Michael S. Tsirkin Acked-by: Paolo Bonzini --- include/linux/virtio_scsi.h | 32 +++- drivers/scsi/virtio_scsi.c | 51 - 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h index de429d1..af44864 100644 --- a/include/linux/virtio_scsi.h +++ b/include/linux/virtio_scsi.h @@ -27,13 +27,15 @@ #ifndef _LINUX_VIRTIO_SCSI_H #define _LINUX_VIRTIO_SCSI_H +#include + #define VIRTIO_SCSI_CDB_SIZE 32 #define VIRTIO_SCSI_SENSE_SIZE 96 /* SCSI command request, followed by data-out */ struct virtio_scsi_cmd_req { u8 lun[8]; /* Logical Unit Number */ - u64 tag;/* Command identifier */ + __virtio64 tag; /* Command identifier */ u8 task_attr; /* Task attribute */ u8 prio;/* SAM command priority field */ u8 crn; @@ -43,20 +45,20 @@ struct virtio_scsi_cmd_req { /* SCSI command request, followed by protection information */ struct virtio_scsi_cmd_req_pi { u8 lun[8]; /* Logical Unit Number */ - u64 tag;/* Command identifier */ + __virtio64 tag; /* Command identifier */ u8 task_attr; /* Task attribute */ u8 prio;/* SAM command priority field */ u8 crn; - u32 pi_bytesout;/* DataOUT PI Number of bytes */ - u32 pi_bytesin; /* DataIN PI Number of bytes */ + __virtio32 pi_bytesout; /* DataOUT PI Number of bytes */ + __virtio32 pi_bytesin; /* DataIN PI Number of bytes */ u8 cdb[VIRTIO_SCSI_CDB_SIZE]; } __packed; /* Response, followed by sense data and data-in */ struct virtio_scsi_cmd_resp { - u32 sense_len; /* Sense data length */ - u32 resid; /* Residual bytes in data buffer */ - u16 status_qualifier; /* Status qualifier */ + __virtio32 sense_len; /* Sense data length */ + __virtio32 resid; /* Residual bytes in data buffer */ + __virtio16 status_qualifier;/* Status qualifier */ u8 status; /* Command completion status */ u8 response;/* Response values */ u8 sense[VIRTIO_SCSI_SENSE_SIZE]; @@ -64,10 +66,10 @@ struct virtio_scsi_cmd_resp { /* Task Management Request */ struct virtio_scsi_ctrl_tmf_req { - u32 type; - u32 subtype; + __virtio32 type; + __virtio32 subtype; u8 lun[8]; - u64 tag; + __virtio64 tag; } __packed; struct virtio_scsi_ctrl_tmf_resp { @@ -76,20 +78,20 @@ struct virtio_scsi_ctrl_tmf_resp { /* Asynchronous notification query/subscription */ struct virtio_scsi_ctrl_an_req { - u32 type; + __virtio32 type; u8 lun[8]; - u32 event_requested; + __virtio32 event_requested; } __packed; struct virtio_scsi_ctrl_an_resp { - u32 event_actual; + __virtio32 event_actual; u8 response; } __packed; struct virtio_scsi_event { - u32 event; + __virtio32 event; u8 lun[8]; - u32 reason; + __virtio32 reason; } __packed; struct virtio_scsi_config { diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index b83846f..d9ec806 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -158,7 +158,7 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf) sc, resp->response, resp->status, resp->sense_len); sc->result = resp->status; - virtscsi_compute_resid(sc, resp->resid); + virtscsi_compute_resid(sc, virtio32_to_cpu(vscsi->vdev, resp->resid)); switch (resp->response) { case VIRTIO_SCSI_S_OK: set_host_byte(sc, DID_OK); @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf) break; } - WARN_ON(resp->sense_len > VIRTIO_SCSI_SENSE_SIZE); + WARN_ON(virtio32_to_cpu(vscsi->vdev, resp->sense_len) > + VIRTIO_SCSI_SENSE_SIZE); if (sc->sense_buffer) { memcpy(sc->sense_buffer, resp->sense, - min_t(u32, resp->sense_len, VIRTIO_SCSI_SENSE_SIZE)); + min_t(u32, +virtio32_to_cpu(vscsi->vdev, resp->sense_len), +VIRTIO_SCSI_SENSE_SIZE)); if (resp->sense_len) set_driver_byte(sc, DRIVER_SENSE); } @@ -323,7 +326,7 @@ static void virtscsi_handle_transport_reset(struct virtio_scsi *vscsi, unsigned int target = event->lun[1]; unsigned int lun = (event->lun[2] << 8) | event->lun[3]; - switch (event->reason) { + switch (v
Re: [PATCH v7 42/46] virtio_scsi: v1.0 support
On Sun, 30 Nov 2014 17:12:47 +0200 "Michael S. Tsirkin" wrote: > Note: for consistency, and to avoid sparse errors, > convert all fields, even those no longer in use > for virtio v1.0. > > Signed-off-by: Michael S. Tsirkin > Acked-by: Paolo Bonzini > --- > include/linux/virtio_scsi.h | 32 +++- > drivers/scsi/virtio_scsi.c | 51 > - > 2 files changed, 49 insertions(+), 34 deletions(-) > > @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi > *vscsi, void *buf) > break; > } > > - WARN_ON(resp->sense_len > VIRTIO_SCSI_SENSE_SIZE); > + WARN_ON(virtio32_to_cpu(vscsi->vdev, resp->sense_len) > > + VIRTIO_SCSI_SENSE_SIZE); Introduce a local variable for this? Might make this statement and the min_t statement below easier to read. > if (sc->sense_buffer) { > memcpy(sc->sense_buffer, resp->sense, > -min_t(u32, resp->sense_len, VIRTIO_SCSI_SENSE_SIZE)); > +min_t(u32, > + virtio32_to_cpu(vscsi->vdev, resp->sense_len), > + VIRTIO_SCSI_SENSE_SIZE)); > if (resp->sense_len) > set_driver_byte(sc, DRIVER_SENSE); > } Otherwise looks good to me. -- 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 v7 42/46] virtio_scsi: v1.0 support
On Mon, Dec 01, 2014 at 01:50:01PM +0100, Cornelia Huck wrote: > On Sun, 30 Nov 2014 17:12:47 +0200 > "Michael S. Tsirkin" wrote: > > > Note: for consistency, and to avoid sparse errors, > > convert all fields, even those no longer in use > > for virtio v1.0. > > > > Signed-off-by: Michael S. Tsirkin > > Acked-by: Paolo Bonzini > > --- > > include/linux/virtio_scsi.h | 32 +++- > > drivers/scsi/virtio_scsi.c | 51 > > - > > 2 files changed, 49 insertions(+), 34 deletions(-) > > > > > @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi > > *vscsi, void *buf) > > break; > > } > > > > - WARN_ON(resp->sense_len > VIRTIO_SCSI_SENSE_SIZE); > > + WARN_ON(virtio32_to_cpu(vscsi->vdev, resp->sense_len) > > > + VIRTIO_SCSI_SENSE_SIZE); > > Introduce a local variable for this? Might make this statement and the > min_t statement below easier to read. I prefer 1:1 code conversions. We can do refactorings on top. Since you mention this line as hard to read, I'll just make it > 80 chars for now, and trivial line splits can come later. OK? > > if (sc->sense_buffer) { > > memcpy(sc->sense_buffer, resp->sense, > > - min_t(u32, resp->sense_len, VIRTIO_SCSI_SENSE_SIZE)); > > + min_t(u32, > > +virtio32_to_cpu(vscsi->vdev, resp->sense_len), > > +VIRTIO_SCSI_SENSE_SIZE)); > > if (resp->sense_len) > > set_driver_byte(sc, DRIVER_SENSE); > > } > > Otherwise looks good to me. -- 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 v7 42/46] virtio_scsi: v1.0 support
On Mon, Dec 01, 2014 at 02:53:05PM +0200, Michael S. Tsirkin wrote: > On Mon, Dec 01, 2014 at 01:50:01PM +0100, Cornelia Huck wrote: > > On Sun, 30 Nov 2014 17:12:47 +0200 > > "Michael S. Tsirkin" wrote: > > > > > Note: for consistency, and to avoid sparse errors, > > > convert all fields, even those no longer in use > > > for virtio v1.0. > > > > > > Signed-off-by: Michael S. Tsirkin > > > Acked-by: Paolo Bonzini > > > --- > > > include/linux/virtio_scsi.h | 32 +++- > > > drivers/scsi/virtio_scsi.c | 51 > > > - > > > 2 files changed, 49 insertions(+), 34 deletions(-) > > > > > > > > @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct > > > virtio_scsi *vscsi, void *buf) > > > break; > > > } > > > > > > - WARN_ON(resp->sense_len > VIRTIO_SCSI_SENSE_SIZE); > > > + WARN_ON(virtio32_to_cpu(vscsi->vdev, resp->sense_len) > > > > + VIRTIO_SCSI_SENSE_SIZE); > > > > Introduce a local variable for this? Might make this statement and the > > min_t statement below easier to read. > > I prefer 1:1 code conversions. We can do refactorings on top. > Since you mention this line as hard to read, I'll just make it > 80 > chars for now, and trivial line splits can come later. > OK? Or maybe I'll keep it as is, since Paolo who wrote this code is happy with it .. > > > if (sc->sense_buffer) { > > > memcpy(sc->sense_buffer, resp->sense, > > > -min_t(u32, resp->sense_len, VIRTIO_SCSI_SENSE_SIZE)); > > > +min_t(u32, > > > + virtio32_to_cpu(vscsi->vdev, resp->sense_len), > > > + VIRTIO_SCSI_SENSE_SIZE)); > > > if (resp->sense_len) > > > set_driver_byte(sc, DRIVER_SENSE); > > > } > > > > Otherwise looks good to me. -- 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 v7 42/46] virtio_scsi: v1.0 support
On Mon, 1 Dec 2014 14:53:05 +0200 "Michael S. Tsirkin" wrote: > On Mon, Dec 01, 2014 at 01:50:01PM +0100, Cornelia Huck wrote: > > On Sun, 30 Nov 2014 17:12:47 +0200 > > "Michael S. Tsirkin" wrote: > > > > > Note: for consistency, and to avoid sparse errors, > > > convert all fields, even those no longer in use > > > for virtio v1.0. > > > > > > Signed-off-by: Michael S. Tsirkin > > > Acked-by: Paolo Bonzini > > > --- > > > include/linux/virtio_scsi.h | 32 +++- > > > drivers/scsi/virtio_scsi.c | 51 > > > - > > > 2 files changed, 49 insertions(+), 34 deletions(-) > > > > > > > > @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct > > > virtio_scsi *vscsi, void *buf) > > > break; > > > } > > > > > > - WARN_ON(resp->sense_len > VIRTIO_SCSI_SENSE_SIZE); > > > + WARN_ON(virtio32_to_cpu(vscsi->vdev, resp->sense_len) > > > > + VIRTIO_SCSI_SENSE_SIZE); > > > > Introduce a local variable for this? Might make this statement and the > > min_t statement below easier to read. > > I prefer 1:1 code conversions. We can do refactorings on top. > Since you mention this line as hard to read, I'll just make it > 80 > chars for now, and trivial line splits can come later. > OK? I guess I don't care strongly enough about this, so go ahead. -- 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