scsi: don't store LUN bits in CDB[1] for USB mass-storage devices

The SCSI specification requires that the second Command Data Byte
should contain the LUN value in its high-order bits if the recipient
device reports SCSI level 2 or below.  Nevertheless, some USB
mass-storage devices use those bits for other purposes in
vendor-specific commands.  Currently Linux has no way to send such
commands, because the SCSI stack always overwrites the LUN bits.

Testing shows that Windows 7 and XP do not store the LUN bits in the
CDB when sending commands to a USB device.  This doesn't matter if the
device uses the Bulk-Only or UAS transports (which virtually all
modern USB mass-storage devices do), as these have a separate
mechanism for sending the LUN value.

Therefore this patch introduces a flag in the Scsi_Host structure to
inform the SCSI midlayer that a transport does not require the LUN
bits to be stored in the CDB, and it makes usb-storage set this flag
for all devices using the Bulk-Only transport.  (UAS is handled by a
separate driver, but it doesn't really matter because no SCSI-2 or
lower device is at all likely to use UAS.)

The patch also cleans up the code responsible for storing the LUN
value by adding a bitflag to the scsi_device structure.  The test for
whether to stick the LUN value in the CDB can be made when the device
is probed, and stored for future use rather than being made over and
over in the fast path.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Tiziano Bacocco <tiziano.bacocco@gmail.com>
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Acked-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d81f3cc43ff1d3e834fcb6d2dc6460810d167ee8..1423cb17fbfda37e4c944b2e7e60feae14359b04 100644 (file)
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -670,14 +670,10 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
                return SCSI_MLQUEUE_DEVICE_BUSY;
        }
 
-       /*
-        * If SCSI-2 or lower, store the LUN value in cmnd.
-        */
-       if (cmd->device->scsi_level <= SCSI_2 &&
-           cmd->device->scsi_level != SCSI_UNKNOWN) {
+       /* Store the LUN value in cmnd, if needed. */
+       if (cmd->device->lun_in_cdb)
                cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
                               (cmd->device->lun << 5 & 0xe0);
-       }
 
        scsi_log_send(cmd);
 

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56675dbbf681c35318b8a4b9cfa92120187952ef..f84c4018847816ddb5fa2a24cadfd70696cb9e22 100644 (file)
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -736,6 +736,16 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
                sdev->scsi_level++;
        sdev->sdev_target->scsi_level = sdev->scsi_level;
 
+       /*
+        * If SCSI-2 or lower, and if the transport requires it,
+        * store the LUN value in CDB[1].
+        */
+       sdev->lun_in_cdb = 0;
+       if (sdev->scsi_level <= SCSI_2 &&
+           sdev->scsi_level != SCSI_UNKNOWN &&
+           !sdev->host->no_scsi2_lun_in_cdb)
+               sdev->lun_in_cdb = 1;
+
        return 0;
 }
 

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 8b4105a22ac2564b5dc64eef5c816c5df0eaf3a8..85e36f3a55857d32ed6fc9000680646d127bed9c 100644 (file)
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1263,7 +1263,19 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
        sdev->sdev_dev.class = &sdev_class;
        dev_set_name(&sdev->sdev_dev, "%d:%d:%d:%llu",
                     sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
+       /*
+        * Get a default scsi_level from the target (derived from sibling
+        * devices).  This is the best we can do for guessing how to set
+        * sdev->lun_in_cdb for the initial INQUIRY command.  For LUN 0 the
+        * setting doesn't matter, because all the bits are zero anyway.
+        * But it does matter for higher LUNs.
+        */
        sdev->scsi_level = starget->scsi_level;
+       if (sdev->scsi_level <= SCSI_2 &&
+                       sdev->scsi_level != SCSI_UNKNOWN &&
+                       !shost->no_scsi2_lun_in_cdb)
+               sdev->lun_in_cdb = 1;
+
        transport_setup_device(&sdev->sdev_gendev);
        spin_lock_irqsave(shost->host_lock, flags);
        list_add_tail(&sdev->same_target_siblings, &starget->devices);

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index cedb29252a92228119a6afa849c4853e87eb3b4e..bf3f8e2de0467b9d2f761151f540a66bad040672 100644 (file)
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -983,6 +983,14 @@ int usb_stor_probe2(struct us_data *us)
        if (!(us->fflags & US_FL_SCM_MULT_TARG))
                us_to_host(us)->max_id = 1;
 
+       /*
+        * Like Windows, we won't store the LUN bits in CDB[1] for SCSI-2
+        * devices using the Bulk-Only transport (even though this violates
+        * the SCSI spec).
+        */
+       if (us->transport == usb_stor_Bulk_transport)
+               us_to_host(us)->no_scsi2_lun_in_cdb = 1;
+
        /* Find the endpoints and calculate pipe values */
        result = get_pipes(us);
        if (result)

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 1a0d1842962e132488ccc95f4e2bde0508d2d408..27ecee73bd72b35b6ad0029686a278ec3ae26de4 100644 (file)
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -174,6 +174,7 @@ struct scsi_device {
        unsigned wce_default_on:1;      /* Cache is ON by default */
        unsigned no_dif:1;      /* T10 PI (DIF) should be disabled */
        unsigned broken_fua:1;          /* Don't set FUA bit */
+       unsigned lun_in_cdb:1;          /* Store LUN bits in CDB[1] */
 
        atomic_t disk_events_disable_depth; /* disable depth for disk events */
 

diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index cafb260ef2d344deaf8da062afb853fe19b9a1b3..d0f69a3210df5be500d8b4d897e35b1f23ee0079 100644 (file)
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -693,6 +693,9 @@ struct Scsi_Host {
         */
        struct workqueue_struct *tmf_work_q;
 
+       /* The transport requires the LUN bits NOT to be stored in CDB[1] */
+       unsigned no_scsi2_lun_in_cdb:1;
+
        /*
         * Value host_blocked counts down from
         */