USB storage devices and SAT
USB storage devices that support SAT (the T10 SCSI to ATA translation standard) are beginning to appear.
SAT enables tools like smartmontools to access SMART data on a ATA disk in a USB enclosure. We have run into a problem. It seems that the usb storage subsystem is wedded to the idea of sense data that is no longer than 18 bytes. ** That doesn't play well with SAT which uses descriptor format sense data that is made up of an 8 byte header plus zero or more descriptors. SAT uses a 14 byte "ATA return" (sense) descriptor to yield the ATA registers. That means the sense data length is 22 bytes when an ATA return descriptor is required.
Alan Stern has already noted to another smartmontools developer that such a change is likely to break some USB storage devices. Perhaps the maximum sense buffer size could be optionally specified per usb storage device. Alternatively the usb mass storage logic could make some dynamic decisions itself. For example if the (disk) device responds successfully to either SCSI ATA PASS_THROUGH (12 or 16 byte) command then it will be capable of (at least) 22 byte sense data. A T10 vendor identification field in a standard INQUIRY response of
"ATA " is another indication of a device that supports
SAT.
** the 18 byte sense data limit comes from SCSI-2 (circa 1992). That was upped to 252 bytes around 10 years ago. Some SCSI disk vendors have been using the extra bytes (above 18) for some time. Then in SPC-3 (standard in 2005) descriptor format sense data was introduced and SAT uses it. There are other uses: any SCSI (virtual) disk that needs more than 32 bits to represent all its LBAs should use descriptor format for MEDIUM ERRORs. The reason is that MEDIUM ERRORs should include the LBA of the first failure and the "info" field in the older "fixed" sense format is only 4 bytes.
Doug Gilbert
James Bottomley
maintainerQuoted 2 lines · Douglas Gilbert
USB storage devices that support SAT (the T10 SCSI to ATA translation standard) are beginning to appear.
Um, confused. The USB devices that were ATA under the covers already have to do SAT since the USB storage transport mandates SCSI.
You mean they're now supporting ATA-12/16 pass through? And will this become standard for non-ATA USB storage (like flash)?
James
Quoted 6 lines · James Bottomley
Quoted 2 lines · Douglas Gilbert
USB storage devices that support SAT (the T10 SCSI to ATA translation standard) are beginning to appear.
Um, confused. The USB devices that were ATA under the covers already have to do SAT since the USB storage transport mandates SCSI.
The enclosures today translate a small subset set of SCSI commands to ATA/ATAPI commands.
SAT would allow arbitrary ATA commands to be sent to target devices. In theory. I've only seen a few products that support ATA passthrough, and they all did it in a vendor-proprietary manner.
Matt
Signature · 6 lines
Matthew Dharm Home: [email protected] Maintainer, Linux USB Mass Storage Driver I could always suspend a few hundred accounts and watch what happens. -- Tanya User Friendly, 7/31/1998
Quoted 5 lines · Douglas Gilbert
Alan Stern has already noted to another smartmontools developer that such a change is likely to break some USB storage devices. Perhaps the maximum sense buffer size could be optionally specified per usb storage device. Alternatively the usb mass storage logic could make some dynamic decisions itself.
To clarify: A great many devices choke (fatally) if asked for sense data other than 18 bytes. Since the first TEST_UNIT_READY will likely require sense data, almost every device sees REQUEST_SENSE.
Personally, I hate having to make dynamic decisions in usb-storage. The more we try to do there, the more likely we are to get it wrong.
If you've got an app that is sending a command, and you KNOW that command should produce >18 bytes of sense data, then there should be a way to specify to the SCSI core (and thus get passed to usb-storage) that sense data of >18 bytes should be requested.
Matt
Signature · 6 lines
Matthew Dharm Home: [email protected] 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
Quoted 11 lines · Matthew Dharm
Quoted 5 lines · Douglas Gilbert
Alan Stern has already noted to another smartmontools developer that such a change is likely to break some USB storage devices. Perhaps the maximum sense buffer size could be optionally specified per usb storage device. Alternatively the usb mass storage logic could make some dynamic decisions itself.
To clarify: A great many devices choke (fatally) if asked for sense data other than 18 bytes. Since the first
TEST_UNIT_READYwill likely require sense data, almost every device seesREQUEST_SENSE.
Quoted 8 lines
Personally, I hate having to make dynamic decisions in usb-storage. The more we try to do there, the more likely we are to get it wrong.
If you've got an app that is sending a command, and you KNOW that command should produce >18 bytes of sense data, then there should be a way to specify to the SCSI core (and thus get passed to usb-storage) that sense data of >18 bytes should be requested.
What?!! The number 18 comes totally from USB land at:drivers/usb/storage/transport.c:601 via call to scsi_eh_prep_cmnd Otherwise the entire kernel including scsi-ml and all user applications request 96 bytes of sense, always.
So there is nothing the SCSI core or application can do about it. It is USB fix time I'm afraid.
Quoted 2 lines
Matt
The SAT layer will have to override transport.c/usb_stor_invoke_transport somehow and make do for bigger REQUEST_SENSE. Or a sense_size param per host, or per command.
Boaz
Quoted 10 lines · Boaz Harrosh
Quoted 4 lines · Matthew Dharm
If you've got an app that is sending a command, and you KNOW that command should produce >18 bytes of sense data, then there should be a way to specify to the SCSI core (and thus get passed to usb-storage) that sense data of >18 bytes should be requested.
What?!! The number 18 comes totally from USB land at:
drivers/usb/storage/transport.c:601via call toscsi_eh_prep_cmndOtherwise the entire kernel including scsi-ml and all user applications request 96 bytes of sense, always.
Well, yes. I think the value 96 must be (relatively) new... I think the value 18 originally came about by copying it from somewhere else, since it's not passed as a parameter from the scsi-ml and needed to be coded directly into usb-storage to support auto-sense.
Perhaps you mis-interpreted my use of "should"? I mean "we should add a way" (not "a way should already exist").
Supporting auto-sense was a requirement since there are transport/protocol combinations where it's not really optional. And, once issued REQUEST_SENSE will clear the sense data, so it we needed to keep the data to put into the auto-sense buffer at those times when sense data needed to be passed to the scsi-ml.
Quoted 2 lines
So there is nothing the SCSI core or application can do about it. It is USB fix time I'm afraid.
Right now, there is nothing that SCSI core or an app can do about it. What I'm suggesting is that there be a parameter (in the scsi_host structure, perhaps?) that allows the core or an app to control this.
Matt
Signature · 6 lines
Matthew Dharm Home: [email protected] Maintainer, Linux USB Mass Storage Driver I'm a pink gumdrop! How can anything be worse?!! -- Erwin User Friendly, 10/4/1998
Quoted 14 lines · Douglas Gilbert
USB storage devices that support SAT (the T10 SCSI to ATA translation standard) are beginning to appear.
SAT enables tools like smartmontools to access SMART data on a ATA disk in a USB enclosure. We have run into a problem. It seems that the usb storage subsystem is wedded to the idea of sense data that is no longer than 18 bytes. ** That doesn't play well with SAT which uses descriptor format sense data that is made up of an 8 byte header plus zero or more descriptors. SAT uses a 14 byte "ATA return" (sense) descriptor to yield the ATA registers. That means the sense data length is 22 bytes when an ATA return descriptor is required.
Quoted 11 lines
Alan Stern has already noted to another smartmontools developer that such a change is likely to break some USB storage devices. Perhaps the maximum sense buffer size could be optionally specified per usb storage device. Alternatively the usb mass storage logic could make some dynamic decisions itself. For example if the (disk) device responds successfully to either SCSI ATA
PASS_THROUGH(12 or 16 byte) command then it will be capable of (at least) 22 byte sense data. A T10 vendor identification field in a standard INQUIRY response of
"ATA " is another indication of a device that supports
SAT.
If either of these techniques could be used for dynamically detecting when a device can reliably support 22-byte REQUEST SENSE, they would be okay.
I tend to agree with Matt that it would be best to have the higher layers tell usb-storage exactly how much sense data they want to get. The problem is that these higher layers would not know about the 18-byte restriction on many earlier USB devices, so usb-storage would probably end up needing to make its own dynamic decisions anyway.
Alan Stern
Quoted 5 lines · Alan Stern
I tend to agree with Matt that it would be best to have the higher layers tell usb-storage exactly how much sense data they want to get. The problem is that these higher layers would not know about the 18-byte restriction on many earlier USB devices, so usb-storage would probably end up needing to make its own dynamic decisions anyway.
Why not handle this like other fields in the struct scsi_device? Let the core initialize it with a value, then override it in the slave_configure() routine via unusual_devs.h flag OR via userspace command to the SCSI core?
Matt
Signature · 7 lines
Matthew Dharm Home: [email protected] 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
Boaz Harrosh
Reviewed-byQuoted 7 lines · Matthew Dharm
Quoted 5 lines · Alan Stern
I tend to agree with Matt that it would be best to have the higher layers tell usb-storage exactly how much sense data they want to get. The problem is that these higher layers would not know about the 18-byte restriction on many earlier USB devices, so usb-storage would probably end up needing to make its own dynamic decisions anyway.
Quoted 6 lines
Why not handle this like other fields in the struct
scsi_device? Let the core initialize it with a value, then override it in theslave_configure()routine viaunusual_devs.hflag OR via userspace command to the SCSI core?Matt
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2
Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what sense_size to fetch. It is a parameter to the scsi_eh_prep_cmnd() call. So no need for slave_configure(), default value, and all that loop.
Matthieu castet <[email protected]> wrote ...
Quoted 5 lines
we have got report from 2 users that scsi sat (to do ata passthrough) was not fully \
working with some usb bridge (maxtor and western digital). Everything works except \
the sense result was incomplete [1].
Quoted 8 lines
After some investigation, they need a sense size different of 18 (at least 22). [2]
Because some devices can crash if the sense size is different of 18, in order to not \
break these devices a flag that is set either by unusual entries, either for spc3 or \
latter devices is used.Signed-off-by Matthieu CASTET <[email protected]>
Quoted 76 lines
[1]
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5613[2]
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
===================================================================drivers/usb/storage/scsiglue.c +4
-0@@ -170,6 +170,10 @@if (us->fflags &US_FL_CAPACITY_HEURISTICS ) sdev->guess_capacity =1 ; +/* assume SPC3 or latter device support sense size different of 18 */ +if (sdev->scsi_level >SCSI_SPC_2 ) + us->fflags |=US_FL_SANE_SENSE ; +/* Some devices report a SCSI revision level above 2 but are * unable to handle the REPORT LUNS command (for which * support is mandatory at level 3). Since we already have Index:
linux-2.6/drivers/usb/storage/transport.c
===================================================================--- linux-2.6.orig/drivers/usb/storage/transport.c 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/transport.c2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +6
-1@@ -595,10 +595,15 @@if (need_auto_sense) {int temp_result;struct scsi_eh_save ses; +int sense_size =US_SENSE_SIZE ; + +/* device support and need bigger sense buffer */ +if (us->fflags &US_FL_SANE_SENSE ) + sense_size = ~0 ;US_DEBUGP ("Issuing auto-REQUEST_SENSE\n" ); -scsi_eh_prep_cmnd (srb, &ses,NULL ,0 ,US_SENSE_SIZE ); +scsi_eh_prep_cmnd (srb, &ses,NULL ,0 , sense_size);/* FIXME: we must do the protocol translation here */ if (us->subclass == US_SC_RBC || us->subclass == US_SC_SCSI ||Index:
linux-2.6/drivers/usb/storage/unusual_devs.h
===================================================================--- linux-2.6.orig/drivers/usb/storage/unusual_devs.h 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/unusual_devs.h2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +12
-0@@ -1791,6 +1791,18 @@ US_SC_DEVICE, US_PR_DEVICE, NULL, US_FL_CAPACITY_HEURISTICS), +UNUSUAL_DEV (0x0d49 ,0x7310 ,0x0000 ,0x9999 , +"Maxtor" , +"usb sata bridge" , +US_SC_DEVICE ,US_PR_DEVICE ,NULL , +US_FL_SANE_SENSE ), + +UNUSUAL_DEV (0x1058 ,0x0704 ,0x0000 ,0x9999 , +"Western Digital" , +"External HDD" , +US_SC_DEVICE ,US_PR_DEVICE ,NULL , +US_FL_SANE_SENSE ), +/* Control/Bulk transport for all SubClass values */ USUAL_DEV (US_SC_RBC ,US_PR_CB ,USB_US_TYPE_STOR ), USUAL_DEV(US_SC_8020, US_PR_CB, USB_US_TYPE_STOR),Index:
linux-2.6/include/linux/usb_usual.h
===================================================================
include/linux/usb_usual.h line 60
Quoted 10 lines
include/linux/usb_usual.h +3
-1@@ -52,7 +52,9 @@US_FLAG (MAX_SECTORS_MIN ,0x00002000 ) \ /* Sets max_sectors to arch min */ \US_FLAG (BULK_IGNORE_TAG ,0x00004000 ) \ - /* Ignore tag mismatch in bulk operations */ + /* Ignore tag mismatch in bulk operations */ \ +US_FLAG (SANE_SENSE ,0x00008000 ) \ + /* Need a sense size different of 18 for some cmd (SAT) */#define US_FLAG (name, value) US_FL_##name = value ,
I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Boaz
Quoted 32 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Quoted 22 lines
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
===================================================================drivers/usb/storage/scsiglue.c +4
-0@@ -170,6 +170,10 @@if (us->fflags &US_FL_CAPACITY_HEURISTICS ) sdev->guess_capacity =1 ; +/* assume SPC3 or latter device support sense size different of 18 */ +if (sdev->scsi_level >SCSI_SPC_2 ) + us->fflags |=US_FL_SANE_SENSE ; +/* Some devices report a SCSI revision level above 2 but are * unable to handle the REPORT LUNS command (for which * support is mandatory at level 3). Since we already have Index:
linux-2.6/drivers/usb/storage/transport.c
===================================================================--- linux-2.6.orig/drivers/usb/storage/transport.c 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/transport.c2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +0
-0@@ -595,10 +595,15 @@
Quoted 8 lines
drivers/usb/storage/scsiglue.c line 602
Quoted 8 lines
drivers/usb/storage/scsiglue.c +5
-0if (need_auto_sense) {int temp_result;struct scsi_eh_save ses; +int sense_size =US_SENSE_SIZE ; + +/* device support and need bigger sense buffer */ +if (us->fflags &US_FL_SANE_SENSE ) + sense_size = ~0 ;
This looks highly suspicious at best. Shouldn't sense_size be set to a real value, like 22 or 255?
Quoted 4 lines
I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Changing it to a blacklist would be bad -- in fact it would be a regression, because all the deficient devices which used to work would stop working until they were identified and added to the blacklist.
Apart from the one nit above, I agree that this patch looks sensible.
Alan Stern
Quoted 37 lines · Alan Stern
Quoted 35 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Quoted 25 lines
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
===================================================================drivers/usb/storage/scsiglue.c +4
-0@@ -170,6 +170,10 @@if (us->fflags &US_FL_CAPACITY_HEURISTICS ) sdev->guess_capacity =1 ; +/* assume SPC3 or latter device support sense size different of 18 */ +if (sdev->scsi_level >SCSI_SPC_2 ) + us->fflags |=US_FL_SANE_SENSE ; +/* Some devices report a SCSI revision level above 2 but are * unable to handle the REPORT LUNS command (for which * support is mandatory at level 3). Since we already have Index:
linux-2.6/drivers/usb/storage/transport.c
===================================================================--- linux-2.6.orig/drivers/usb/storage/transport.c 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/transport.c2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +0
-0@@ -595,10 +595,15 @@if (need_auto_sense) {int temp_result;struct scsi_eh_save ses;
Quoted 9 lines
Quoted 5 lines
drivers/usb/storage/scsiglue.c line 602
Quoted 5 lines
drivers/usb/storage/scsiglue.c +5
-0+int sense_size =US_SENSE_SIZE ; + +/* device support and need bigger sense buffer */ +if (us->fflags &US_FL_SANE_SENSE ) + sense_size = ~0 ;This looks highly suspicious at best. Shouldn't
sense_sizebe set to a real value, like 22 or 255?
No this is the regular way to signal from a driver that we support whatever the scsi-ml or application decided. So ~0 means, effectively, scsi's default which is 96. But can change in future with no changes to drivers.
(BTW ~0 is used everywhere in Kernel but here)
Don't just hard code what ever you want. the maximum sense_size mandated by scsi standard is 260. The current midlayer uses 96 every where. If the driver has specific needs, like 18 with broken devices, do so but please don't hard code to arbitrary value.
Quoted 6 lines
Quoted 5 lines
I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Quoted 8 lines
Changing it to a blacklist would be bad -- in fact it would be a regression, because all the deficient devices which used to work would stop working until they were identified and added to the blacklist.
Apart from the one nit above, I agree that this patch looks sensible.
Alan Stern
Thanks
Boaz
Quoted 37 lines · Alan Stern
Quoted 35 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Quoted 25 lines
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
===================================================================drivers/usb/storage/scsiglue.c +4
-0@@ -170,6 +170,10 @@if (us->fflags &US_FL_CAPACITY_HEURISTICS ) sdev->guess_capacity =1 ; +/* assume SPC3 or latter device support sense size different of 18 */ +if (sdev->scsi_level >SCSI_SPC_2 ) + us->fflags |=US_FL_SANE_SENSE ; +/* Some devices report a SCSI revision level above 2 but are * unable to handle the REPORT LUNS command (for which * support is mandatory at level 3). Since we already have Index:
linux-2.6/drivers/usb/storage/transport.c
===================================================================--- linux-2.6.orig/drivers/usb/storage/transport.c 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/transport.c2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +0
-0@@ -595,10 +595,15 @@if (need_auto_sense) {int temp_result;struct scsi_eh_save ses;
Quoted 8 lines
Quoted 5 lines
drivers/usb/storage/scsiglue.c line 602
Quoted 5 lines
drivers/usb/storage/scsiglue.c +5
-0+int sense_size =US_SENSE_SIZE ; + +/* device support and need bigger sense buffer */ +if (us->fflags &US_FL_SANE_SENSE ) + sense_size = ~0 ;This looks highly suspicious at best. Shouldn't
sense_sizebe set to a real value, like 22 or 255?
The "correct" maximum value (SPC-3 and draft SPC-4) is 252. Since SPC-3 the recommended maximum length for the basic SCSI commands that have a 1 byte allocation length field was altered from 255 to 252. This is to be a little friendlier to transports that move data in 4 byte units across their transport. Guessing a bit here but SATA, SAS and FCP fall into that group of transports.
At some stage the allocation field length of an INQUIRY command was changed from 1 to 2 bytes. So to pick up VPD pages on older devices an INQUIRY's maximum allocation length of 252 may be prudent. For example, choosing a value of 260 (0x104) may give a surprising result if the device only expects a 1 byte allocation length field.
Quoted 8 lines
Quoted 4 lines
I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Changing it to a blacklist would be bad -- in fact it would be a regression, because all the deficient devices which used to work would stop working until they were identified and added to the blacklist.
Apart from the one nit above, I agree that this patch looks sensible.
Boaz,
I didn't test the above patch (as I don't have the external USB devices that need it) but a gentleman who did, tells me that he used a very similar patch and it solved his problem.
Doug Gilbert
Quoted 14 lines · Douglas Gilbert
Quoted 13 lines · Alan Stern
Quoted 11 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Quoted 2 lines
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
<snip>
Quoted 10 lines
Quoted 2 lines
This looks highly suspicious at best. Shouldn't
sense_sizebe set to a real value, like 22 or 255?The "correct" maximum value (SPC-3 and draft SPC-4) is 252. Since SPC-3 the recommended maximum length for the basic SCSI commands that have a 1 byte allocation length field was altered from 255 to 252. This is to be a little friendlier to transports that move data in 4 byte units across their transport. Guessing a bit here but SATA, SAS and FCP fall into that group of transports.
252 + 8 bytes header for REQUEST_SENSE command. So total 260 buffer size. (Last I look)
Quoted 7 lines
At some stage the allocation field length of an INQUIRY command was changed from 1 to 2 bytes. So to pick up VPD pages on older devices an INQUIRY's maximum allocation length of 252 may be prudent. For example, choosing a value of 260 (
0x104) may give a surprising result if the device only expects a 1 byte allocation length field.
INQUIRY and VPD pages are different. From what I remember Vendor VPD pages where always be16 length field.
You and the scsi code keep talking about "1 byte allocation length field". But the standard talks about be16 values. For me they are not the same.
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Changing it to a blacklist would be bad -- in fact it would be a regression, because all the deficient devices which used to work would stop working until they were identified and added to the blacklist.
Quoted 7 lines
Quoted 1 line
Apart from the one nit above, I agree that this patch looks sensible.
Boaz,
I didn't test the above patch (as I don't have the external USB devices that need it) but a gentleman who did, tells me that he used a very similar patch and it solved his problem.
Do you know if he used the white-list or if his device returned SPC-3 compliance. The reason I ask is because all the devices I have here from usb sticks to external boxes and converters all work with 96 bytes sense but all report spc-2
Quoted 2 lines
Doug Gilbert
Thanks
Boaz
Quoted 9 lines · Boaz Harrosh
Quoted 7 lines · Douglas Gilbert
The "correct" maximum value (SPC-3 and draft SPC-4) is 252. Since SPC-3 the recommended maximum length for the basic SCSI commands that have a 1 byte allocation length field was altered from 255 to 252. This is to be a little friendlier to transports that move data in 4 byte units across their transport. Guessing a bit here but SATA, SAS and FCP fall into that group of transports.
Quoted 3 lines
252 + 8 bytes header for
REQUEST_SENSEcommand. So total 260 buffer size. (Last I look)
OK you are right 252 allocation length max, specified at REQUEST_SENSE CDB. The sense_buffer structure itself has 252 + 8 restriction which got me confused. In OSD we have large sense payload that can get truncated by these values.
Boaz
Hi,
Quoted 7 lines · Boaz Harrosh
Douglas Gilbert wrote:
Do you know if he used the white-list or if his device returned SPC-3 compliance. The reason I ask is because all the devices I have here from usb sticks to external boxes and converters all work with 96 bytes sense but all report spc-2
He used the white-list. At the moment I don't know if some device report at spc-3, but it could be a easy way to white-list them.
Matthieu
Hi,
Quoted 19 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Matthieu castet <[email protected]> wrote ...
Quoted 7 lines
we have got report from 2 users that scsi sat (to do ata passthrough) was not fully \
working with some usb bridge (maxtor and western digital). Everything works except \
the sense result was incomplete [1].After some investigation, they need a sense size different of 18 (at least 22). [2]
Quoted 9 lines
Quoted 6 lines
Because some devices can crash if the sense size is different of 18, in order to not \
break these devices a flag that is set either by unusual entries, either for spc3 or \
latter devices is used.Signed-off-by Matthieu CASTET <[email protected]>Reviewed-by Boaz Harrosh <[email protected]>
What the status of that.
Should I try to resubmit that patch on linux-usb ?
Quoted 96 lines
Quoted 75 lines
[1]
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5613[2]
http://thread.gmane.org/gmane.linux.utilities.smartmontools/5721
Index:linux-2.6/drivers/usb/storage/scsiglue.c
===================================================================drivers/usb/storage/scsiglue.c +4
-0@@ -170,6 +170,10 @@if (us->fflags &US_FL_CAPACITY_HEURISTICS ) sdev->guess_capacity =1 ; +/* assume SPC3 or latter device support sense size different of 18 */ +if (sdev->scsi_level >SCSI_SPC_2 ) + us->fflags |=US_FL_SANE_SENSE ; +/* Some devices report a SCSI revision level above 2 but are * unable to handle the REPORT LUNS command (for which * support is mandatory at level 3). Since we already have Index:
linux-2.6/drivers/usb/storage/transport.c
===================================================================--- linux-2.6.orig/drivers/usb/storage/transport.c 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/transport.c2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +6
-1@@ -595,10 +595,15 @@if (need_auto_sense) {int temp_result;struct scsi_eh_save ses; +int sense_size =US_SENSE_SIZE ; + +/* device support and need bigger sense buffer */ +if (us->fflags &US_FL_SANE_SENSE ) + sense_size = ~0 ;US_DEBUGP ("Issuing auto-REQUEST_SENSE\n" ); -scsi_eh_prep_cmnd (srb, &ses,NULL ,0 ,US_SENSE_SIZE ); +scsi_eh_prep_cmnd (srb, &ses,NULL ,0 , sense_size);/* FIXME: we must do the protocol translation here */ if (us->subclass == US_SC_RBC || us->subclass == US_SC_SCSI ||Index:
linux-2.6/drivers/usb/storage/unusual_devs.h
===================================================================--- linux-2.6.orig/drivers/usb/storage/unusual_devs.h 2008-08-02 00:07:24.000000000 \ +0200+++
linux-2.6/drivers/usb/storage/unusual_devs.h2008-08-02 00:07:37.000000000 +0200drivers/usb/storage/scsiglue.c +12
-0@@ -1791,6 +1791,18 @@ US_SC_DEVICE, US_PR_DEVICE, NULL, US_FL_CAPACITY_HEURISTICS), +UNUSUAL_DEV (0x0d49 ,0x7310 ,0x0000 ,0x9999 , +"Maxtor" , +"usb sata bridge" , +US_SC_DEVICE ,US_PR_DEVICE ,NULL , +US_FL_SANE_SENSE ), + +UNUSUAL_DEV (0x1058 ,0x0704 ,0x0000 ,0x9999 , +"Western Digital" , +"External HDD" , +US_SC_DEVICE ,US_PR_DEVICE ,NULL , +US_FL_SANE_SENSE ), +/* Control/Bulk transport for all SubClass values */ USUAL_DEV (US_SC_RBC ,US_PR_CB ,USB_US_TYPE_STOR ), USUAL_DEV(US_SC_8020, US_PR_CB, USB_US_TYPE_STOR),Index:
linux-2.6/include/linux/usb_usual.h
===================================================================include/linux/usb_usual.h line 60
Quoted 10 lines
include/linux/usb_usual.h +3
-1@@ -52,7 +52,9 @@US_FLAG (MAX_SECTORS_MIN ,0x00002000 ) \ /* Sets max_sectors to arch min */ \US_FLAG (BULK_IGNORE_TAG ,0x00004000 ) \ - /* Ignore tag mismatch in bulk operations */ + /* Ignore tag mismatch in bulk operations */ \ +US_FLAG (SANE_SENSE ,0x00008000 ) \ + /* Need a sense size different of 18 for some cmd (SAT) */#define US_FLAG (name, value) US_FL_##name = value ,I recommend this patch. It does exactly what's needed with minimum risk it is even maybe over protective, with the white list. Perhaps it should be turned to a black list instead. The old broken devices been an extincting exception.
Boaz
Quoted 26 lines · matthieu castet
Hi,
Quoted 23 lines · Boaz Harrosh
There was a correct and simple patch proposed that fixes this problem the right way:
http://marc.info/?l=linux-usb&m=121762869915609&w=2Doug could you please test this patch to see if it fixes your device?
scsi-core already gives drivers complete control on what
sense_sizeto fetch. It is a parameter to thescsi_eh_prep_cmnd()call. So no need forslave_configure(), default value, and all that loop.Matthieu castet <[email protected]> wrote ...
Quoted 12 lines
we have got report from 2 users that scsi sat (to do ata passthrough) was not fully \
working with some usb bridge (maxtor and western digital). Everything works except \
the sense result was incomplete [1].After some investigation, they need a sense size different of 18 (at least 22). [2]
Because some devices can crash if the sense size is different of 18, in order to not \
break these devices a flag that is set either by unusual entries, either for spc3 or \
latter devices is used.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Matthieu CASTET <[email protected]>Reviewed-by Boaz Harrosh <[email protected]>What the status of that.
Should I try to resubmit that patch on linux-usb ?
I see the To: is me. But I'm not a USB guy at all.
Like you said, I too think the best is to freshen up the patch for latest USB tree and send it to the USB mailing list.
Cheers
Boaz