use_10_for_ms revisited?

use_10_for_ms revisited?

[Jeff Garzik]

I was reading the specs just now, and I see that the inquiry page 
defines how ATAPI and USB devices indicate they are compliant with MMC-4.

And, MMC-4 does not define read/write/modesen/modesel 6-byte commands at 
all.

My conclusion is that we should notice up front when MMC-4 is supported, 
and simply adhere to the spec by always sending 10-byte commands for 
that device.  I do not think that translation of 6-to-10 byte commands 
is answer, which is what happens now.  Our SCSI layer (sr.c, etc.) 
should, IMO, be compliant with MMC-4 and simply send the right commands.

This also implies that use_10_for_ms simply need not exist -- "first try 
10 byte commands for this device" is false...  for some devices we 
should be unconditionally sending 10 byte commands, no ifs, ands, or buts :)

I'll see if I can work up a patch, if noone objects...

	Jeff, debugging atapi support in his ata-scsi driver

Re: use_10_for_ms revisited?

[Matthew Dharm]

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

On Sun, Jun 29, 2003 at 02:30:28AM -0400, Jeff Garzik wrote:
> I was reading the specs just now, and I see that the inquiry page 
> defines how ATAPI and USB devices indicate they are compliant with MMC-4.

It does?  I guess I've missed that.

If you're talking about the INQUIRY command, the data better be within the
first 36 bytes, or it's likely to not actually be there.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

Umm, these aren't the droids you're looking for.
					-- Bill Gates
User Friendly, 11/14/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: use_10_for_ms revisited?

[Jeff Garzik]

Matthew Dharm wrote:
> On Sun, Jun 29, 2003 at 02:30:28AM -0400, Jeff Garzik wrote:
> 
>>I was reading the specs just now, and I see that the inquiry page 
>>defines how ATAPI and USB devices indicate they are compliant with MMC-4.
> 
> 
> It does?  I guess I've missed that.

I'm looking at MMC-4 revision 2a (whatever was on t10.org tonight).

Immediately after table 286 (standard inquiry data format), it describes 
the information I believe we can use to determine that 10-byte commands 
(only) need to be sent.  peripheral qualifier, peripheral device type, 
version, and interface dependent field #1.

These fields can be used to differentiate atapi/usb devices from older 
scsi devices.  One concrete example is that atapi reports zero in the 
version field, even though it's mmc-4 compliant.  But the Linux scsi 
stack treats that instead as the most ancient of ancient scsi devices :) 
  So, to counteract this, the ide-scsi module and other places will 
patch the inquiry page data to indicate a higher scsi revision.

Other examples are several places where 6-byte commands are converted 
into 10-byte commands.  The right answer is to simply know when to send 
10-byte commands in the first place, thus eliminating the need for any 
translation.


> If you're talking about the INQUIRY command, the data better be within the
> first 36 bytes, or it's likely to not actually be there.

Certainly.

	Jeff

Re: use_10_for_ms revisited?

[Matthew Dharm]

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

I'm somewhat concerned that, just as we're finding something that works,
we're going to go off in another direction.

I certainly wouldn't mind removing the code that manipulates the SCSI
revision number.

But, the 6-to-10 conversion code (at least for USB) is all about to go
away, once we settle on how sr.c (and friends) should handle use_10_for_ms
-- so one of your reasons to change all this is already moot.

I guess, in part, I'm hesitant to rely on fields in the INQUIRY data that
nobody has checked to verify that they are actually useful.  Most USB
devices are made on ultra-low testing, and commonly have bad bits in any
field that isn't used by Windows or MacOS.

Matt

On Sun, Jun 29, 2003 at 02:47:23AM -0400, Jeff Garzik wrote:
> Matthew Dharm wrote:
> > On Sun, Jun 29, 2003 at 02:30:28AM -0400, Jeff Garzik wrote:
> > 
> >>I was reading the specs just now, and I see that the inquiry page 
> >>defines how ATAPI and USB devices indicate they are compliant with MMC-4.
> > 
> > 
> > It does?  I guess I've missed that.
> 
> I'm looking at MMC-4 revision 2a (whatever was on t10.org tonight).
> 
> Immediately after table 286 (standard inquiry data format), it describes 
> the information I believe we can use to determine that 10-byte commands 
> (only) need to be sent.  peripheral qualifier, peripheral device type, 
> version, and interface dependent field #1.
> 
> These fields can be used to differentiate atapi/usb devices from older 
> scsi devices.  One concrete example is that atapi reports zero in the 
> version field, even though it's mmc-4 compliant.  But the Linux scsi 
> stack treats that instead as the most ancient of ancient scsi devices :) 
>   So, to counteract this, the ide-scsi module and other places will 
> patch the inquiry page data to indicate a higher scsi revision.
> 
> Other examples are several places where 6-byte commands are converted 
> into 10-byte commands.  The right answer is to simply know when to send 
> 10-byte commands in the first place, thus eliminating the need for any 
> translation.
> 
> 
> > If you're talking about the INQUIRY command, the data better be within the
> > first 36 bytes, or it's likely to not actually be there.
> 
> Certainly.
> 
> 	Jeff
> 
> 
> 

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

You were using cheat codes too.  You guys suck.
					-- Greg to General Studebaker
User Friendly, 12/16/1997

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: use_10_for_ms revisited?

[Jeff Garzik]

Matthew Dharm wrote:
> I'm somewhat concerned that, just as we're finding something that works,
> we're going to go off in another direction.
> 
> I certainly wouldn't mind removing the code that manipulates the SCSI
> revision number.

I'm talking about code in the scsi core that does stuff like ("if 
scsi_level <= 2" ...), which is wrong for MMC, where version==0 is a 
valid value on modern atapi and usb devices.


> But, the 6-to-10 conversion code (at least for USB) is all about to go
> away, once we settle on how sr.c (and friends) should handle use_10_for_ms
> -- so one of your reasons to change all this is already moot.

Not really, see below.


> I guess, in part, I'm hesitant to rely on fields in the INQUIRY data that
> nobody has checked to verify that they are actually useful.  Most USB
> devices are made on ultra-low testing, and commonly have bad bits in any
> field that isn't used by Windows or MacOS.

MMC-4 is the culmination of the stuff that's in the field now.

Really, sr wants to know "are you mmc-compliant?"

If the answer is yes, it should only use MMC-defined commands (mode 
sense 10, etc.).

use_10_for_ms is a subset of that.

	Jeff

Re: use_10_for_ms revisited?

[Andries Brouwer]

On Sun, Jun 29, 2003 at 02:30:28AM -0400, Jeff Garzik wrote:

> I was reading the specs just now, and I see that the inquiry page 
> defines how ATAPI and USB devices indicate they are compliant with MMC-4.
> 
> And, MMC-4 does not define read/write/modesen/modesel 6-byte commands at 
> all.
> 
> My conclusion is that we should notice up front when MMC-4 is supported, 
> and simply adhere to the spec by always sending 10-byte commands for 
> that device.  I do not think that translation of 6-to-10 byte commands 
> is answer, which is what happens now.  Our SCSI layer (sr.c, etc.) 
> should, IMO, be compliant with MMC-4 and simply send the right commands.
> 
> This also implies that use_10_for_ms simply need not exist -- "first try 
> 10 byte commands for this device" is false...  for some devices we 
> should be unconditionally sending 10 byte commands, no ifs, ands, or buts :)
> 
> I'll see if I can work up a patch, if noone objects...

Jeff, let me object.

You are an optimist - it is as if I hear Andre, who has just read
the latest ATA draft.

The firmware of existing devices is not magically updated when
a new draft standard is released. So reading yesterdays version
gives very little information on the real world.

Andries


[And no, none of my USB devices announces MMC-4 compliance.]

Re: use_10_for_ms revisited?

[Alan Cox]

On Sul, 2003-06-29 at 08:27, Jeff Garzik wrote:
> Really, sr wants to know "are you mmc-compliant?"
> 
> If the answer is yes, it should only use MMC-defined commands (mode 
> sense 10, etc.).
> 
> use_10_for_ms is a subset of that.

sr wants to know "are you mmc compliant or one of the odd list". The
scsi blacklist can handle them with luck

Re: use_10_for_ms revisited?

[Jeff Garzik]

Andries Brouwer wrote:
> The firmware of existing devices is not magically updated when
> a new draft standard is released. So reading yesterdays version
> gives very little information on the real world.

I'm not implying otherwise...  I just think a hueristic can be found 
that works with these devices.


> [And no, none of my USB devices announces MMC-4 compliance.]

Would you mind posting the first 8 bytes of any of these USB storage 
devices' INQUIRY pages?

	Jeff

Re: use_10_for_ms revisited?

[Andries Brouwer]

On Sun, Jun 29, 2003 at 12:33:01PM -0400, Jeff Garzik wrote:
> Andries Brouwer wrote:
> >The firmware of existing devices is not magically updated when
> >a new draft standard is released. So reading yesterdays version
> >gives very little information on the real world.
> 
> I'm not implying otherwise...  I just think a hueristic can be found 
> that works with these devices.
> 
> 
> >[And no, none of my USB devices announces MMC-4 compliance.]
> 
> Would you mind posting the first 8 bytes of any of these USB storage 
> devices' INQUIRY pages?
> 

Two that I have right here yield 0 80 2 2 20 0 0 0 and
0 80 0 2 1F 0 0 0, respectively. (All in hex.)

The 20 in the first is a bug - the INQUIRY gives only 36 bytes.

You do not make me happy with wonderful SCSI patches right now.
I like to go slowly.

[I did introduce this use_10_for_ms; it allows a very large
cleanup in the usb-storage code. Submitted the SCSI part of
the patch and waited for it to be applied. That took a while
and when it went in I was a bit short on Linux time. So Matt
got impatient and started doing the rest himself. He patched
sr.c - something I would not have done, I considered the SCSI
part settled - perhaps things broke a bit but James went in and
fixed some things again. So maybe we are now again ready for
the intended large cleanup. I would like to see that done,
and afterwards see this area stable for a few kernel versions.]

Andries

Re: use_10_for_ms revisited?

[Jeff Garzik]

Andries Brouwer wrote:
> Two that I have right here yield 0 80 2 2 20 0 0 0 and
> 0 80 0 2 1F 0 0 0, respectively. (All in hex.)

Ok, you've convinced me I found an ATAPI-only rule that doesn't apply to 
USB :)  Thanks.


> [I did introduce this use_10_for_ms; it allows a very large
> cleanup in the usb-storage code. Submitted the SCSI part of
> the patch and waited for it to be applied. That took a while
> and when it went in I was a bit short on Linux time. So Matt
> got impatient and started doing the rest himself. He patched
> sr.c - something I would not have done, I considered the SCSI
> part settled - perhaps things broke a bit but James went in and
> fixed some things again. So maybe we are now again ready for
> the intended large cleanup. I would like to see that done,
> and afterwards see this area stable for a few kernel versions.]

I would still prefer to detect "MMC", at least in ATAPI's case, and then 
use that flag to trickle down knowledge to use_10_for_ms and similar 
features.  ATAPI is even easier than I was previously thinking:  at 
host-alloc time, we already know the devices will be MMC, and never ever 
want 6-byte commands.

I grant you that ATAPI is not the whole world, here, but it presents 
sufficient cases to warrant a common approach.  Both ide-scsi and usb 
storage (and my ata-scsi driver) have atapi-specific logic in them for 
6-to-10 translation.  We can keep adding hueristic upon hueristic to the 
kernel to handle these things, or we can step back, look at the bigger 
picture, and take advantage of the commonality found.


Stepping back a bit and addressing previous "standards versus reality" 
comments (<grin>), I certainly understand the point, and am used to such 
things in all areas of hardware :)  My main point was that -- at least 
wrt ATAPI -- the scsi MMC standards seem to be moving in the direction 
of codifying _existing practice_, not making up new standards.  IOW, 
they are just writing down the things that have been in the field for years.

	Jeff

Re: use_10_for_ms revisited?

[Matthew Dharm]

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

On Sun, Jun 29, 2003 at 01:58:04PM -0400, Jeff Garzik wrote:
> I grant you that ATAPI is not the whole world, here, but it presents 
> sufficient cases to warrant a common approach.  Both ide-scsi and usb 
> storage (and my ata-scsi driver) have atapi-specific logic in them for 
> 6-to-10 translation.  We can keep adding hueristic upon hueristic to the 
> kernel to handle these things, or we can step back, look at the bigger 
> picture, and take advantage of the commonality found.

Right now, we can handle this case with a slave_configure() function that
sets use_10_for_ms and use_10_for_rw, like usb-storage does now.  The
heuristic code in usb-storage is on the way out, once sr.c is converted (in
progress).

Devices easily identified, heuristics gone, repeated code very minimal (2
lines) -- sounds like a winner to me.  Actually, I have a feeling that any
algorithm to try to determine MMC or not (complete with it's own heuristic
to handle strange devices) will be much longer than those repeated 2 lines.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

I don't have a left mouse button.  I only have one mouse and it's on my right.
					-- Customer
User Friendly, 2/13/1999

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: use_10_for_ms revisited?

[Jeff Garzik]

Matthew Dharm wrote:
> Right now, we can handle this case with a slave_configure() function that
> sets use_10_for_ms and use_10_for_rw, like usb-storage does now.  The
> heuristic code in usb-storage is on the way out, once sr.c is converted (in
> progress).
> 
> Devices easily identified, heuristics gone, repeated code very minimal (2
> lines) -- sounds like a winner to me.  Actually, I have a feeling that any
> algorithm to try to determine MMC or not (complete with it's own heuristic
> to handle strange devices) will be much longer than those repeated 2 lines.

No argument, I'm sold :)  Sounds like a good plan.

That still doesn't address the issue use_10_for_ms being _too_ 
fine-grained, though:  mode sense is but one part of the larger picture.

We will still have duplicated ATAPI (and other?) logic in USB storage, 
ide-scsi, my driver, and elsewhere for items which are common to MMC. 
For many cases, it is silly to set a whole list of use_10_for_foo 
variables, when they are all unconditionally set to 1 when MMC is 
detected (->slave_configure).

So, a concrete suggestion would be:

Create 'mmc' bit for each scsi_device, set at slave_configure time. 
use_10_for_foo and other code can derive values from this.  i.e. provide 
enough information that can automatically set all those use_10_for_foo 
variables.

	Jeff

Re: use_10_for_ms revisited?

[Andries Brouwer]

On Sun, Jun 29, 2003 at 01:58:04PM -0400, Jeff Garzik wrote:

> I would still prefer to detect "MMC", at least in ATAPI's case, and then 
> use that flag to trickle down knowledge to use_10_for_ms and similar 
> features.  ATAPI is even easier than I was previously thinking:  at 
> host-alloc time, we already know the devices will be MMC, and never ever 
> want 6-byte commands.

And what do they want? 12-byte commands?
What standard are you reading? SFF-8020? ATA-4?

Andries

Re: use_10_for_ms revisited?

[Jeff Garzik]

Andries Brouwer wrote:
> On Sun, Jun 29, 2003 at 01:58:04PM -0400, Jeff Garzik wrote:
> 
> 
>>I would still prefer to detect "MMC", at least in ATAPI's case, and then 
>>use that flag to trickle down knowledge to use_10_for_ms and similar 
>>features.  ATAPI is even easier than I was previously thinking:  at 
>>host-alloc time, we already know the devices will be MMC, and never ever 
>>want 6-byte commands.
> 
> 
> And what do they want? 12-byte commands?
> What standard are you reading? SFF-8020? ATA-4?

MMC-<1234> from t10.org, which if considered over time (i.e. changes 
from 1 -> 2 -> 3 -> 4 are interesting) gives a fairly reasonable picture 
of the subset of scsi that atapi belongs to.  That picture seems to gibe 
with the atapi testing on ancient devices I've done so far.  MMC sorta 
takes over where SFF left off, IMO.  You definitely want the sff docs 
too for the full picture (as you no doubt know already).

	Jeff

Re: use_10_for_ms revisited?

[Matthew Dharm]

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

On Sun, Jun 29, 2003 at 02:17:41PM -0400, Jeff Garzik wrote:
> That still doesn't address the issue use_10_for_ms being _too_ 
> fine-grained, though:  mode sense is but one part of the larger picture.
> 
> We will still have duplicated ATAPI (and other?) logic in USB storage, 
> ide-scsi, my driver, and elsewhere for items which are common to MMC. 
> For many cases, it is silly to set a whole list of use_10_for_foo 
> variables, when they are all unconditionally set to 1 when MMC is 
> detected (->slave_configure).

I'm having difficulty seeing what duplicated code will be there after the
cleanup.... can you give an example?

One bit I see is the code which modifies the INQUIRY page to show SCSI rev
2.  Anything else?

> So, a concrete suggestion would be:
> 
> Create 'mmc' bit for each scsi_device, set at slave_configure time. 
> use_10_for_foo and other code can derive values from this.  i.e. provide 
> enough information that can automatically set all those use_10_for_foo 
> variables.

This sounds pretty good, if there really are more sections of code which
need more use_10_for_foo codes.  Really, (if I understand you correctly)
you're just proposing to replace all the separate use_10_for_foo codes with
a single, unified, 'mmc' code.  Do you think the use_10_for codes should
remain so they can be turned off individually, or should they all just be
replaced with the 'mmc' code?

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

What the hell are you?
					-- Pitr to Dust Puppy 
User Friendly, 12/3/1997

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: use_10_for_ms revisited?

[James Bottomley]

Actually guys, by penguin fiat number:

torvalds@home.transmeta.com|ChangeSet|20030621190214|57300

We have been setting use_10_for_ms to one for every device in .73

Now, our current setup can't handle the 10->6 fallback, but I've fixed
that in the recent BK.

Given that there have been no reported problems (other than from our
inability to interpret 10 by mode sense data), I'm inclined to leave it
this change in.

This rather pre-empts the slave_configure setting of the flags, though.

James

Re: use_10_for_ms revisited?

[Jeff Garzik]

James Bottomley wrote:
> Actually guys, by penguin fiat number:
> 
> torvalds@home.transmeta.com|ChangeSet|20030621190214|57300
> 
> We have been setting use_10_for_ms to one for every device in .73

I noticed this :)


> Now, our current setup can't handle the 10->6 fallback, but I've fixed
> that in the recent BK.
> 
> Given that there have been no reported problems (other than from our
> inability to interpret 10 by mode sense data), I'm inclined to leave it
> this change in.

sr.c is still hardcoded to use mode_sense_6...  even though it's been 
hardcoded to use read/write_10 for years :)

	Jeff

Re: use_10_for_ms revisited?

[Matthew Dharm]

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

On Sun, Jun 29, 2003 at 03:07:11PM -0400, Jeff Garzik wrote:
> sr.c is still hardcoded to use mode_sense_6...  even though it's been 
> hardcoded to use read/write_10 for years :)

James made a patch to fix this, which I intend to test today or tomorrow.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

S:  Another stupid question?
G:  There's no such thing as a stupid question, only stupid people.
					-- Stef and Greg
User Friendly, 7/15/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: use_10_for_ms revisited?

[Jeff Garzik]

Matthew Dharm wrote:
> On Sun, Jun 29, 2003 at 03:07:11PM -0400, Jeff Garzik wrote:
> 
>>sr.c is still hardcoded to use mode_sense_6...  even though it's been 
>>hardcoded to use read/write_10 for years :)
> 
> 
> James made a patch to fix this, which I intend to test today or tomorrow.


I just cloned scsi-misc-2.5 and saw it.  I'll do the same w/ atapi.

	Jeff