On Wed, Jan 4, 2012 at 1:03 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 4 Jan 2012, Oliver Neukum wrote:
>
>> Looking at the following code from scsi.c:
>>
>> int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>> {
>
> ...
>
>> /*
>> * If SCSI-2 or lower, store the LUN value in cmnd.
>> */
>> if (cmd->device->scsi_level <= SCSI_2 &&
>> cmd->device->scsi_level != SCSI_UNKNOWN) {
>> cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
>> (cmd->device->lun << 5 & 0xe0);
>>
>>
>> I am really unsure whether this change is really safe.
>> In particular I doubt it is material for the stable tree.
>
> Well, this change just makes us more compatible with what devices are
> supposed to expect. But if you think it's too chancy, it can be kept
> out of the stable trees.
>
> Matt, what do you think? The change Oliver refers to removes the
> code that forces every USB device to have its scsi_level set to 2, so
> now we'll use the actual level reported by the device. At the same
> time, it adds a new mechanism to prevent the use of REPORT LUNS, which
> was the reason for forcing the level to 2 in the first place.
This is actually a bit of a complicated issue.
Historically, there have been at least 3 observed behavior modes for
the reported SCSI level:
1) Reasonably correct (i.e. 1 or 2)
2) Reports 0 (which, when -1'ed, yields a rev of 0xff). I actually
had an argument with a vendor who insisted this was "correct" because
the device didn't really support all the required SCSI commands; it's
difficult to say he was wrong.
3) Reports 0xff (yada yada to 0xfe).
When this 'fixup' code to force rev 2 was added, there were actually 3
separate issues:
a) REPORT_LUNS command sent to devices that didn't support it, causing
device firmare crash
b) 16-byte command usage used to be keyed off of this, causing
firmware crash (since changed, I *THINK* -- needs verification)
c) Confusing userspace tools (cdrecord, I think, maybe others) into
doing (a) or (b)
If we're pretty certain that the only two things that SCSI revision
are used for is REPORT_LUNS and determining where to stuff the LUN in
the command bytes, that still leaves issue (c); we could always call
that a case of "bad userspace", but users will complain.
The real problem, however, is the number of devices which exhibit bad
behavior (2) or (3). The vast majority of them need the LUN stuffed
into the command bytes (i.e. SCSI-2 behavior). How do be guarantee
functionality to those devices?
Related question: How do we identify those devices? This bad
behavior was so common in the earlier era of usb-storage that we never
created a separate UNUSUAL_DEV flag for it; we just overrode the value
and went on with our lives. Maybe we write it such that the SCSI
level is set to 2 if the reported value is 0 or obviously stupid (i.e.
>3).
I don't *recall* any devices reporting SCSI-3 that needed to be set to
SCSI-2.... but it's been many years since I wrote this bit.
Curious observation: I wonder how this sort of SCSI-3 multi-LUN device
fares on WinXP? As far as I know, WinXP also assumes SCSI-2 when
sending commands to USB devices...
So, in short, I think Alan's patch is a Bad Idea(tm). While it fixes
the problem described in the comments, there are other issues lurking
out there. As I think about it, I kinda like my "force rev to 2 if
obviously wrong" approach... what do you guys think?
Matt
--
Matthew Dharm
Maintainer, USB Mass Storage driver for Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
