From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53343) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHwbR-0001Rg-8h for qemu-devel@nongnu.org; Tue, 28 Jun 2016 13:15:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHwbN-00066M-1O for qemu-devel@nongnu.org; Tue, 28 Jun 2016 13:15:12 -0400 Received: from mail-wm0-x242.google.com ([2a00:1450:400c:c09::242]:35913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHwbM-00065r-M7 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 13:15:08 -0400 Received: by mail-wm0-x242.google.com with SMTP id c82so7563823wme.3 for ; Tue, 28 Jun 2016 10:15:08 -0700 (PDT) Sender: Paolo Bonzini References: <20160628161945.GA2022@ks392938.kimsufi.com> From: Paolo Bonzini Message-ID: <70f633c5-c7d6-fcf9-7489-05e820e22ac8@redhat.com> Date: Tue, 28 Jun 2016 19:14:55 +0200 MIME-Version: 1.0 In-Reply-To: <20160628161945.GA2022@ks392938.kimsufi.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] scsi-bus: Add support for SCSI scanners List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jarkko Lavinen , qemu-devel@nongnu.org > + case INQUIRY: This is wrong, because INQUIRY's byte 3 is defined to be part of the length in modern SCSI standards. Perhaps you can move it to scsi_req_scanner_length if really necessary? > case MODE_SENSE: > + case MODE_SELECT: > + case REQUEST_SENSE: > + cmd->xfer = buf[4]; > + break; This is unnecessary, because scsi_cdb_xfer should get it right (the commands are respectively 15h for MODE SELECT and 03h for REQUEST SENSE; the existing entry for MODE SENSE is also unnecessary). > + case MODE_SENSE_10: > + case MODE_SELECT_10: > + cmd->xfer = (buf[7] << 8) | buf[8]; > break; Same here, scsi_cdb_xfer should handle it. > > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c > index 045594a..2f8da49 100644 > --- a/hw/scsi/scsi-bus.c > +++ b/hw/scsi/scsi-bus.c > @@ -1139,6 +1139,32 @@ static int scsi_req_medium_changer_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8 > return 0; > } > > +static int scsi_req_scanner_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) > +{ > + switch (buf[0]) { > + /* Scanner commands */ > + case OBJECT_POSITION: > + cmd->xfer = 0; > + break; > + case GET_DATA_BUFFER_STATUS: > + cmd->xfer = buf[8] | (buf[7] << 8); > + break; > + case SCAN: > + cmd->xfer = buf[4]; > + break; > + case READ_10: > + case SEND: > + case GET_WINDOW: > + case SET_WINDOW: > + cmd->xfer = buf[8] | (buf[7] << 8) | (buf[6] << 16); > + break; > + default: > + return scsi_req_xfer(cmd, dev, buf); > + } Looks good. > + return 0; > +} > + > static void scsi_cmd_xfer_mode(SCSICommand *cmd) > { > if (!cmd->xfer) { > @@ -1184,6 +1210,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd) > case SEND_DVD_STRUCTURE: > case PERSISTENT_RESERVE_OUT: > case MAINTENANCE_OUT: > + case SET_WINDOW: > + case SCAN: SCAN conflicts with START_STOP. Add a comment please saying that START_STOP has cmd->xfer set to 0 in scsi_req_xfer for non-scanner devices. Everything else looks good. Thanks, Paolo > cmd->mode = SCSI_XFER_TO_DEV; > break; > case ATA_PASSTHROUGH_12: > @@ -1264,6 +1292,9 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) > case TYPE_MEDIUM_CHANGER: > rc = scsi_req_medium_changer_xfer(cmd, dev, buf); > break; > + case TYPE_SCANNER: > + rc = scsi_req_scanner_length(cmd, dev, buf); > + break; > default: > rc = scsi_req_xfer(cmd, dev, buf); > break; > diff --git a/include/block/scsi.h b/include/block/scsi.h > index a311341..8b966d7 100644 > --- a/include/block/scsi.h > +++ b/include/block/scsi.h > @@ -48,13 +48,17 @@ > #define ERASE 0x19 > #define MODE_SENSE 0x1a > #define LOAD_UNLOAD 0x1b > +#define SCAN 0x1b > #define START_STOP 0x1b > #define RECEIVE_DIAGNOSTIC 0x1c > #define SEND_DIAGNOSTIC 0x1d > #define ALLOW_MEDIUM_REMOVAL 0x1e > +#define SET_WINDOW 0x24 > #define READ_CAPACITY_10 0x25 > +#define GET_WINDOW 0x25 > #define READ_10 0x28 > #define WRITE_10 0x2a > +#define SEND 0x2a > #define SEEK_10 0x2b > #define LOCATE_10 0x2b > #define POSITION_TO_ELEMENT 0x2b > @@ -62,10 +66,12 @@ > #define VERIFY_10 0x2f > #define SEARCH_HIGH 0x30 > #define SEARCH_EQUAL 0x31 > +#define OBJECT_POSITION 0x31 > #define SEARCH_LOW 0x32 > #define SET_LIMITS 0x33 > #define PRE_FETCH 0x34 > #define READ_POSITION 0x34 > +#define GET_DATA_BUFFER_STATUS 0x34 > #define SYNCHRONIZE_CACHE 0x35 > #define LOCK_UNLOCK_CACHE 0x36 > #define INITIALIZE_ELEMENT_STATUS_WITH_RANGE 0x37 > -- > 2.1.4 >