PATCH: exclude certain commands from emulated SCSI hosts
Matthew Dharm
emulated host.
A source-code survey shows that usb-storage, sbp2, ide-scsi, and the 3Ware
driver are the only 'emulated' hosts.
If a host is emulated, we (a) don't ask for EVPD data, as it's likely not
there, (b) assume that we have only 36-bytes of INQUIRY data, and (c) don't
ask disks for their cache-type (assume write-through).
All three of these things have been observed in the field to choke emulated
devices, thus it makes sense to cut them off at the SCSI layer. In an
interesting side-note, this appears to be the only code which is
conditional on a host being emulated or not.
Linus, please apply to your 2.5.x tree.
Matthew Dharm
# This is a BitKeeper generated patch for the following project:
# Project Name: greg k-h's linux 2.5 USB kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.697 -> 1.698
# drivers/scsi/sd.c 1.47 -> 1.48
# drivers/scsi/scsi_scan.c 1.26 -> 1.27
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/03/22 mdh...@zen.san.one-eyed-alien.net 1.698
# Emulated hosts shouldn't get certain commands.
# --------------------------------------------
#
diff -Nru a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c Sat Mar 22 19:19:22 2003
+++ b/drivers/scsi/scsi_scan.c Sat Mar 22 19:19:22 2003
@@ -514,6 +514,10 @@
unsigned char scsi_cmd[MAX_COMMAND_SIZE];
int max_lgth = 255;
+ /* emulated devices don't like the request for EVPD page */
+ if (sdev->host->hostt->emulated)
+ return NULL;
+
retry:
evpd_page = kmalloc(max_lgth, GFP_ATOMIC |
(sdev->host->unchecked_isa_dma) ?
@@ -851,6 +855,10 @@
return 0;
}
+ /* emulated devices do not react well to this sort of thing */
+ if (sdev->host->hostt->emulated)
+ goto leave;
+
memset(scsi_cmd, 0, MAX_COMMAND_SIZE);
scsi_cmd[0] = INQUIRY;
scsi_cmd[1] = 0x01;
@@ -1033,6 +1041,10 @@
*/
BUG_ON(bflags == NULL);
*bflags = scsi_get_device_flags(&inq_result[8], &inq_result[16]);
+
+ /* emulated devices should all be treated as blacklisted */
+ if (sdev->host->hostt->emulated)
+ *bflags |= BLIST_INQUIRY_36;
possible_inq_resp_len = (unsigned char) inq_result[4] + 5;
if (BLIST_INQUIRY_36 & *bflags)
diff -Nru a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c Sat Mar 22 19:19:21 2003
+++ b/drivers/scsi/sd.c Sat Mar 22 19:19:22 2003
@@ -1152,6 +1152,15 @@
const int dbd = 0x08; /* DBD */
const int modepage = 0x08; /* current values, cache page */
+ /* emulated devices likely do not support this type of mode sense */
+ if (sdkp->device->host->hostt->emulated) {
+ printk(KERN_NOTICE "%s: emulated, assuming drive cache: %s\n",
+ diskname, "write through");
+ sdkp->WCE = 0;
+ sdkp->RCD = 0;
+ return;
+ }
+
/* cautiously ask */
res = sd_do_mode_sense6(sdp, SRpnt, dbd, modepage, buffer, 4);
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
Sir, for the hundreth time, we do NOT carry 600-round boxes of belt-fed
suction darts!
-- Salesperson to Greg
User Friendly, 12/30/1997
Linus Torvalds
On Sat, 22 Mar 2003, Matthew Dharm wrote:
>
> This patch changes how devices a probed on a SCSI bus if they are on an
> emulated host.
I really think this is wrong. I'd much much rather get _rid_ of that
stupid "emulated" flag, instead of adding meaning to it.
> If a host is emulated, we (a) don't ask for EVPD data, as it's likely not
> there, (b) assume that we have only 36-bytes of INQUIRY data, and (c) don't
> ask disks for their cache-type (assume write-through).
..and thus anything that wants to emulate SCSI has to never be able to be
write-back again.
Now, I'd say that it is stupid to call yourself a SCSI disk if you aren't
one _anyway_ (the raw block device interfaces are simpler and faster), but
if you do, then I think it's doubly stupid to put in arbitrary
restrictions like this.
If there are known commands that devices have trouble with (whether
emulated or not), maybe we could have helper routines to do some default
filtering, and have the queuecommand function check those. But this just
looks ugly and hacky to me.
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Dharm
>
> On Sat, 22 Mar 2003, Matthew Dharm wrote:
> >
> > This patch changes how devices a probed on a SCSI bus if they are on an
> > emulated host.
>
> I really think this is wrong. I'd much much rather get _rid_ of that
> stupid "emulated" flag, instead of adding meaning to it.
Well, you could just delete it. It's not used anywhere.
> > If a host is emulated, we (a) don't ask for EVPD data, as it's likely not
> > there, (b) assume that we have only 36-bytes of INQUIRY data, and (c) don't
> > ask disks for their cache-type (assume write-through).
>
> ..and thus anything that wants to emulate SCSI has to never be able to be
> write-back again.
Okay, I'll admit that could be a problem.
> Now, I'd say that it is stupid to call yourself a SCSI disk if you aren't
> one _anyway_ (the raw block device interfaces are simpler and faster), but
> if you do, then I think it's doubly stupid to put in arbitrary
> restrictions like this.
The problem is this: usb-storage interfaces via SCSI, but not just for
disks. Tape, CD, etc. are all handled with the same protocol. Heck, ATAPI
disk/cd/tape are handled with these code paths.
> If there are known commands that devices have trouble with (whether
> emulated or not), maybe we could have helper routines to do some default
> filtering, and have the queuecommand function check those. But this just
> looks ugly and hacky to me.
Well, there are lots of 'known commands' that fall in to this category. If
we had centralized helper functions, that would be great. But, as it
stands, right now all the low-level drivers have to do all that separately,
and badly.
Right now, we've got nothing, which leaves low-level driver folks out in
the cold.
As I see it, SCSI commands break down into two basic categories: common
and uncommon. Common things (basic read and write, 36-byte INQUIRY, eject,
etc.) are all fine, but the 'uncommon' things (checking cache type,
255-byte INQUIRY, etc) cause problems. I'm trying to find a way to choke
off the problematic commands without having to write code to recognize what
is being sent (and choke it off) based on what is in the command bytes.
I'm open to suggestions.
Matt
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
It was a new hope.
-- Dust Puppy
User Friendly, 12/25/1998
Linus Torvalds
How about making the SCSI stuff pass a "common" flag (or "required") down
with the command? Then, a emulated thing could just decide to punt all
commands with an immediate failure if they aren't marked "required".
That still _allows_ the driver to implement it if it wants to, unlike your
previous approach.
Matthew Dharm
> The problem with this type of approach is that there's no unified list
> of "known good" commands that actually let you operate a device. The
> SCSI and ATAPI standards have been gradually deprecating the commands
> that SCSI-1 (and sometimes even SCSI-2) regard as mandatory
> (READ_6/WRITE_6 springs to mind).
Actually, there is such a list. It's the commands that the 'popular OS'
uses, and I have a pretty good idea exactly what those are. That's why my
original approach was to just cut out the commands that fell outside that
definition.
Matt
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
I want my GPFs!!!
-- Stef
User Friendly, 11/9/1998
James Bottomley
> Actually, there is such a list. It's the commands that the 'popular OS'
> uses, and I have a pretty good idea exactly what those are. That's why my
> original approach was to just cut out the commands that fell outside that
> definition.
Well, if you want to write a helper for the mid layer that checks the
commands and returns a Check Condition with sense Illegal Reguest on the
bad ones, that sounds like the best approach. You can then call the
helper function in the queuecommand of the problem emulated drivers.
Can you publish a rough list of these?
James
James Bottomley
> The problem is this: usb-storage interfaces via SCSI, but not just for
> disks. Tape, CD, etc. are all handled with the same protocol. Heck, ATAPI
> disk/cd/tape are handled with these code paths.
ATAPI isn't really "emulated" in the sense that ATAPI devices understand
a subset of SCSI commands (albeit according to their own rules).
I'll give you the six byte mode sense, since ATAPI devices only do the
ten byte version, but they nevertheless understand it.
> > If there are known commands that devices have trouble with (whether
> > emulated or not), maybe we could have helper routines to do some default
> > filtering, and have the queuecommand function check those. But this just
> > looks ugly and hacky to me.
>
> Well, there are lots of 'known commands' that fall in to this category. If
> we had centralized helper functions, that would be great. But, as it
> stands, right now all the low-level drivers have to do all that separately,
> and badly.
The problem with this type of approach is that there's no unified list
of "known good" commands that actually let you operate a device. The
SCSI and ATAPI standards have been gradually deprecating the commands
that SCSI-1 (and sometimes even SCSI-2) regard as mandatory
(READ_6/WRITE_6 springs to mind).
Given this, the upper level drivers have to try playing a bit to see
what the device will actually support.
> Right now, we've got nothing, which leaves low-level driver folks out in
> the cold.
Well, for truly "emulated" (meaning the command is interpreted inside
the driver and executed there instead of simply being repackaged and
passed on to the device), a library of helper functions would be
acceptable to place in the mid-layer if you want to write such a thing.
> As I see it, SCSI commands break down into two basic categories: common
> and uncommon. Common things (basic read and write, 36-byte INQUIRY, eject,
> etc.) are all fine, but the 'uncommon' things (checking cache type,
> 255-byte INQUIRY, etc) cause problems. I'm trying to find a way to choke
> off the problematic commands without having to write code to recognize what
> is being sent (and choke it off) based on what is in the command bytes.
Under any definition of "common", a device that can't accept variable
length inquiry commands is broken (by every known standard, even).
However, lets not rehash old sore points.
Could you elaborate on what the "problem" is. There seem to be two
separate issues:
1. True devices which have incomplete state models for SCSI inside the
real device and thus react badly to certain commands. For this one, I'm
happy to have a helper library containing the elements of the SCSI state
model for the devices to use, but I do believe that the drivers are
broken and need fixing.
2. Certain real SCSI (or ATAPI) devices which have broken internal SCSI
processing models. For these we need to not send certain commands which
annoy them. This is much harder, since I don't believe we'll get a
common definition of annoying commands that will work for every device.
For ATAPI, we can probably get pretty far with a "no six byte commands"
flag. This should be fairly easy to implement since every LLD tends to
know when it is repackaging for an ATAPI device. What other rules would
you need for this type of thing?
Linus Torvalds
On Sun, 23 Mar 2003, Matthew Dharm wrote:
> On Sun, Mar 23, 2003 at 07:26:09PM -0600, James Bottomley wrote:
> > The problem with this type of approach is that there's no unified list
> > of "known good" commands that actually let you operate a device. The
> > SCSI and ATAPI standards have been gradually deprecating the commands
> > that SCSI-1 (and sometimes even SCSI-2) regard as mandatory
> > (READ_6/WRITE_6 springs to mind).
>
> Actually, there is such a list. It's the commands that the 'popular OS'
> uses, and I have a pretty good idea exactly what those are. That's why my
> original approach was to just cut out the commands that fell outside that
> definition.
Now _this_ is actually a valid approach, but then I would suggest you do
what Andries Brouwer has done a few times: just make the Linux SCSI layer
not use those commands _at_all_ - and instead use the sequences of
commands that "that other OS" uses. That fixed a lot of the smartmedia
problems I used to have - doing simple things like _not_ trying to spin up
devices that reported themselves to be already active etc.
This isn't an "emulated vs native" thing, btw, it's a much more generic
"don't use commands that haven't gotten much testing" approach, and should
probably be done both on emulated devices _and_ on "real" SCSI devices.
After all, "real" devices have problems too - I bet a lot of our
historical SCSI black-lists have been due to exactly the same issues you
see on emulated USB, with devices just reacting badly to command sequences
that they didn't expect and that hadn't been tested by the manufacturer.
Linus
Matthew Dharm
anymore...
EVPD INQUIRY, INQUIRY for bytes != 36, and MODE_SENSE are big offenders.
But, EVPD INQUIRY is something that's a problem. Getting rid of it isn't
easy -- someone put it in there because they wanted/needed the data.
INQUIRY != 36 bytes has been discussed before. Since low-level drivers
snoop the data past 36-bytes, we can't completely get rid of this -- tho I
think this really means that the LLDD need fixing.
MODE_SENSE is something that Linux uses to check for write-protect status.
I could chuck the entire thing, but then we would assume write-enabled for
all disk-like media.
Note that not all of these fall into the 'not-well tested' category.
Certainly, INQUIRY != 36 is something that we blacklist (for having a long
history of problems). MODE_SENSE doesn't have any historical problems that
I know about. EVPD certainly hasn't had much testing.... but can I just
get rid of it then?
Matt
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
I'm seen in many forms. Now open your mouth. It's caffeine time.
-- Cola Man to Greg
User Friendly, 10/28/1998
Matthew Dharm
INQUIRY (for only 36 bytes -- nothing else!)
TEST_UNIT_READY
REQUEST_SENSE
ALLOW_MEDIUM_REMOVAL (mostly for eject purposes)
For disk-like media:
READ_10
WRITE_10
I'd have to go back to my notes for tape and CD media. But those aren't my
biggest problem areas -- the common probe and the disk driver are my
biggest headaches.
Note that MODE_SENSE isn't on this list. How does the 'popular OS' test
for write-protect, you ask? It tries to write and then looks for a
failure, AFAICT.
Note that this data comes from observations about what commands all devices
seem to support.
I'd be willing to write a helper, but I'm a bit out of my element here...
can someone at least suggest a good place to put such a helper (or
volunteer to mock one up for me)?
Matt
On Sun, Mar 23, 2003 at 07:39:01PM -0600, James Bottomley wrote:
> On Sun, 2003-03-23 at 19:37, Matthew Dharm wrote:
> > Actually, there is such a list. It's the commands that the 'popular OS'
> > uses, and I have a pretty good idea exactly what those are. That's why my
> > original approach was to just cut out the commands that fell outside that
> > definition.
>
> Well, if you want to write a helper for the mid layer that checks the
> commands and returns a Check Condition with sense Illegal Reguest on the
> bad ones, that sounds like the best approach. You can then call the
> helper function in the queuecommand of the problem emulated drivers.
>
> Can you publish a rough list of these?
>
> James
>
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
Dudes! May the Open Source be with you.
-- Eric S. Raymond
User Friendly, 12/3/1998
James Bottomley
> Well, here's my list of what the 'popular OS' uses for all devices:
>
> INQUIRY (for only 36 bytes -- nothing else!)
> TEST_UNIT_READY
> REQUEST_SENSE
> ALLOW_MEDIUM_REMOVAL (mostly for eject purposes)
>
> For disk-like media:
>
> READ_10
> WRITE_10
We do about the best we can for read and write. For sd, we gauge the
size of the command from the size of the medium: <1Gb=> six byte, from
1Gb to 2Tb 10 byte, over 2Tb 16 byte, so I think this should all be
fine.
> I'd have to go back to my notes for tape and CD media. But those aren't my
> biggest problem areas -- the common probe and the disk driver are my
> biggest headaches.
>
> Note that MODE_SENSE isn't on this list. How does the 'popular OS' test
> for write-protect, you ask? It tries to write and then looks for a
> failure, AFAICT.
We use MODE_SENSE on CDs to probe for capabilities: this is required
behaviour and we've been doing for a long time.
sd also uses MODE_SENSE to probe the cache type as well as the write
protect state. Cache type certainly can't be obtained any other way,
and I'm not sure allowing writes to read only media wouldn't cause us
more problems in the long run.
> I'd be willing to write a helper, but I'm a bit out of my element here...
> can someone at least suggest a good place to put such a helper (or
> volunteer to mock one up for me)?
OK, I can do this: A simple one with either a blacklist (reject these
commands) or whitelist (only accept these commands) going by the first
command byte OK?
James
Linus Torvalds
On 24 Mar 2003, James Bottomley wrote:
> >
> > For disk-like media:
> >
> > READ_10
> > WRITE_10
>
> We do about the best we can for read and write. For sd, we gauge the
> size of the command from the size of the medium: <1Gb=> six byte, from
> 1Gb to 2Tb 10 byte, over 2Tb 16 byte, so I think this should all be
> fine.
Really? The code was _supposed_ to always start off with READ/WRITE_10's,
and then fall back to the old READ/WRITE_6 if it gets errors from that. Do
we really have some broken random-number generator semantic still in teh
SCSI layer? That sounds like a piece of crock.
Linus
Jens Axboe
>
> On 24 Mar 2003, James Bottomley wrote:
> > >
> > > For disk-like media:
> > >
> > > READ_10
> > > WRITE_10
> >
> > We do about the best we can for read and write. For sd, we gauge the
> > size of the command from the size of the medium: <1Gb=> six byte, from
> > 1Gb to 2Tb 10 byte, over 2Tb 16 byte, so I think this should all be
> > fine.
>
> Really? The code was _supposed_ to always start off with READ/WRITE_10's,
> and then fall back to the old READ/WRITE_6 if it gets errors from that. Do
> we really have some broken random-number generator semantic still in teh
> SCSI layer? That sounds like a piece of crock.
It's not true, ->ten is set unconditionally and we only fall back to 6
byte cdb's if we see an ILLEGAL_REQUEST on a READ_10/WRITE_10.
So the logic is, always assume 10-byte commands. If an incoming request
cannot be addressed with 10-byte commands, use 16.
--
Jens Axboe
James Bottomley
> It's not true, ->ten is set unconditionally and we only fall back to 6
> byte cdb's if we see an ILLEGAL_REQUEST on a READ_10/WRITE_10.
>
> So the logic is, always assume 10-byte commands. If an incoming request
> cannot be addressed with 10-byte commands, use 16.
Sorry, that was me misreading the fallback logic in sd.c
Ten bytes it is unless we have problems.
James
James Bottomley
> Really? The code was _supposed_ to always start off with READ/WRITE_10's,
> and then fall back to the old READ/WRITE_6 if it gets errors from that. Do
> we really have some broken random-number generator semantic still in teh
> SCSI layer? That sounds like a piece of crock.
Well, OK, the whole story is that sd does size the command according to
the request, as I said before. However, if the six byte command fails
with an illegal request sense then we'll retry it at ten bytes and lock
the device to accept only ten byte commands using the ten flag in the
struct scsi_device.
We never do sixteen byte commands unless a region of the device >2Tb is
accessed (in which case the device must support sixteen byte commands).
I think this behaviour is reasonably optimal. I suspect more ancient
SCSI-1 devices would have issues with READ/WRITE_10 than modern devices
that don't do READ_6 but also are unable to return the correct error
code.
Matthew Dharm
> On Mon, 2003-03-24 at 01:04, Matthew Dharm wrote:
> > Note that MODE_SENSE isn't on this list. How does the 'popular OS' test
> > for write-protect, you ask? It tries to write and then looks for a
> > failure, AFAICT.
>
> We use MODE_SENSE on CDs to probe for capabilities: this is required
> behaviour and we've been doing for a long time.
>
> sd also uses MODE_SENSE to probe the cache type as well as the write
> protect state. Cache type certainly can't be obtained any other way,
> and I'm not sure allowing writes to read only media wouldn't cause us
> more problems in the long run.
CD is fine, I was referring to MODE_SENSE in sd.c
I think allowing write to read-only media is the only way to go. I don't
see another way to get write-protect status.
> > I'd be willing to write a helper, but I'm a bit out of my element here...
> > can someone at least suggest a good place to put such a helper (or
> > volunteer to mock one up for me)?
>
> OK, I can do this: A simple one with either a blacklist (reject these
> commands) or whitelist (only accept these commands) going by the first
> command byte OK?
Well, you need to go by more than the first command byte -- ex. INQUIRY is
okay, unless length != 36 or EVPD.
I think a blacklist is probably in order, but with a BIG COMMENT mentioning
that if someone adds new commands into the code paths they should at least
consider if they belong in the blacklist.
Matt
--
Matthew Dharm Home: mdhar...@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
Type "format c:" That should fix everything.
-- Greg
User Friendly, 12/18/1997