Re: [Qemu-devel] scsi-bus: Add support for SCSI scanners

[Jarkko Lavinen <jlavi@iki.fi>]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

On Tue, Jun 28, 2016 at 07:14:55PM +0200, Paolo Bonzini wrote:
> This is wrong, because INQUIRY's byte 3 is defined to be part of the
> length in modern SCSI standards.

> This is wrong, because INQUIRY's byte 3 is defined to be part of the
> length in modern SCSI standards.
Ok. I was using outdated ANSI spec too pedantically. The first patch is
not needed at all.

> 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.
Done.

I also removed this:

    case GET_DATA_BUFFER_STATUS:
        cmd->xfer = buf[8] | (buf[7] << 8);

Since it is again unneedes and handled by scsi_cdb_xfer.

I added longer sense buffer in Request Sense command for scanners.
Multi Pro has at least one sense bit in byte 18 and Minolta's own SW
uses 20 bytes as an allocation size in Request Sense command.

The extra sense bit seems to be a kind of internal busy/film holder
moving status. After having this sense bit available scanning programs 
have been very stable and every feature seems to be working.

Jarkko

[-- Attachment #2: 0001-scsi-bus-Add-SCSI-scanner-support.patch --]
[-- Type: text/x-diff, Size: 3413 bytes --]

>From 0eb0f4ef920973f62ad3029f5b9ac726cfe610cf Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen@iki.fi>
Date: Tue, 28 Jun 2016 21:51:52 +0300
Subject: [PATCH 1/2] scsi-bus: Add SCSI scanner support

Add support for missing scanner specific SCSI commands and their xfer
lenghts as per ANSI spec section 15.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@iki.fi>
---
 hw/scsi/scsi-bus.c   | 30 ++++++++++++++++++++++++++++++
 include/block/scsi.h |  6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ad6f398..e0fbaee 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1132,6 +1132,31 @@ 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 SCAN:
+        /* SCAN conflicts with START_STOP which has cmd->xfer set to 0 for
+           non-scanner devices */
+        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:
+        /* GET_DATA_BUFFER_STATUS xfer handled by scsi_req_xfer */
+        return scsi_req_xfer(cmd, dev, buf);
+    }
+
+    return 0;
+}
 
 static void scsi_cmd_xfer_mode(SCSICommand *cmd)
 {
@@ -1178,6 +1203,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:
         cmd->mode = SCSI_XFER_TO_DEV;
         break;
     case ATA_PASSTHROUGH_12:
@@ -1258,6 +1285,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


[-- Attachment #3: 0002-scsi-bus-Use-longer-sense-buffer-with-scanners.patch --]
[-- Type: text/x-diff, Size: 1886 bytes --]

>From dd85d2aac375db97e860a0908e5bfd5192d0dc61 Mon Sep 17 00:00:00 2001
From: Jarkko Lavinen <jarkko.lavinen@iki.fi>
Date: Wed, 29 Jun 2016 03:11:46 +0300
Subject: [PATCH 2/2] scsi-bus: Use longer sense buffer with scanners

Scanners can provide additional sense bytes beyond 18 bytes.
VueScan uses 32 bytes alloc length with Request Sense command.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@iki.fi>
---
 hw/scsi/scsi-bus.c     | 10 +++++++++-
 include/hw/scsi/scsi.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index e0fbaee..367c5c2 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -461,6 +461,14 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     return true;
 }
 
+static size_t scsi_sense_len(SCSIRequest *req)
+{
+    if (req->dev->type == TYPE_SCANNER)
+        return SCSI_SENSE_LEN_XTRA;
+    else
+        return SCSI_SENSE_LEN;
+}
+
 static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
 {
     SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
@@ -477,7 +485,7 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
         }
         break;
     case REQUEST_SENSE:
-        scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
+        scsi_target_alloc_buf(&r->req, scsi_sense_len(req));
         r->len = scsi_device_get_sense(r->req.dev, r->buf,
                                        MIN(req->cmd.xfer, r->buf_len),
                                        (req->cmd.buf[1] & 1) == 0);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 8acd3fa..76d121d 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -10,6 +10,7 @@
 
 #define SCSI_CMD_BUF_SIZE     16
 #define SCSI_SENSE_LEN      18
+#define SCSI_SENSE_LEN_XTRA 32
 #define SCSI_INQUIRY_LEN    36
 
 typedef struct SCSIBus SCSIBus;
-- 
2.1.4