RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Timothy Thelin]

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


> On Mon, 7 Nov 2005, James Bottomley wrote:
> 
> > > It's quite possible that usb-storage no longer needs to force the 
> > > scsi-level to 2.  No one has recently tested what would 
> happen without 
> > > it.  Matt probably has the best selection of devices for 
> testing...
> > > 
> > > There is one problem we have with devices that report 
> themselves as SCSI-3 
> > > or SCSI-4 but hang when they receive a REPORT LUNS 
> command.  That's easily 
> > > handled by making usb-storage set the NOREPORTLUN flag.  
> Maybe that's all 
> > > we need to do.
> > 
> > If you could try this out, I'd be grateful.
> 
> I tried the patch below on several combinations of SCSI 
> levels and device 
> types.  It seems to do exactly what we want; for reads I always saw 
> READ(10) and for MODE SENSE I saw either the 6- or 10-byte 
> form, depending 
> on the device type and USB subclass value.
> 
> The question remains whether pass-thru will now work and 
> whether the patch
> will mess up some existing non-compliant device that 
> currently works okay.  
> The only way to know is by trying it out.  Matt and Timothy, 
> that's up to
> you.
> 
> Alan Stern
> 

I did some initial testing on Alan's patch with a sandisk cruzer
mini thumb drive (level 2), a memorex thumb drive (level 0),
and that cypress part (level 0), and I haven't seen any IO
problems.

After that I made another patch (below and attached because
of wrapping) you can apply after Alan's that disables cdb mangling
on level 0 devices. It was generated against Linus' git repo after
applying Alan's patch.

With both patches applied I can now do passthru cdbs on the
cypress part without issue, and I still don't see any IO issues.

Anyone out there have a better selection of hardware to test with?


Signed-off-by: Timothy Thelin <timothy.thelin@wdc.com>

---

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -569,9 +569,10 @@ int scsi_dispatch_cmd(struct scsi_cmnd *
 	}
 
 	/* 
-	 * If SCSI-2 or lower, store the LUN value in cmnd.
+	 * If SCSI-2 or lower (except SCSI-0), store the LUN value in cmnd.
 	 */
-	if (cmd->device->scsi_level <= SCSI_2) {
+	if (cmd->device->scsi_level != SCSI_UNKNOWN &&
+		cmd->device->scsi_level <= SCSI_2) {
 		cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
 			       (cmd->device->lun << 5 & 0xe0);
 	}

[-- Attachment #2: scsi-level-0-aware.patch --]
[-- Type: application/octet-stream, Size: 532 bytes --]

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -569,9 +569,10 @@ int scsi_dispatch_cmd(struct scsi_cmnd *
 	}
 
 	/* 
-	 * If SCSI-2 or lower, store the LUN value in cmnd.
+	 * If SCSI-2 or lower (except SCSI-0), store the LUN value in cmnd.
 	 */
-	if (cmd->device->scsi_level <= SCSI_2) {
+	if (cmd->device->scsi_level != SCSI_UNKNOWN &&
+		cmd->device->scsi_level <= SCSI_2) {
 		cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
 			       (cmd->device->lun << 5 & 0xe0);
 	}

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Timothy Thelin wrote:
>>On Mon, 7 Nov 2005, James Bottomley wrote:
>>
> It was generated against Linus' git repo after
> applying Alan's patch.
> 
> With both patches applied I can now do passthru cdbs on the
> cypress part without issue, and I still don't see any IO issues.
> 
> Anyone out there have a better selection of hardware to test with?
> 

first i need better software, the 2.6.14-git-11 snapshot wont boot, cause
it tries to mount root without detecting partitions before :/

did i miss some patch since this snapshot?

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

thomas schorpp wrote:
> Timothy Thelin wrote:
> 
>>>On Mon, 7 Nov 2005, James Bottomley wrote:
>>>
>>
>>It was generated against Linus' git repo after
>>applying Alan's patch.
>>
>>With both patches applied I can now do passthru cdbs on the
>>cypress part without issue, and I still don't see any IO issues.
>>
>>Anyone out there have a better selection of hardware to test with?
>>
> 
> 
> first i need better software, the 2.6.14-git-11 snapshot wont boot, cause
> it tries to mount root without detecting partitions before :/
> 
> did i miss some patch since this snapshot?
> 

ok, applies against 2.6.14.0. device: cypress -A- bridge.

no change with smartctl:

tom1:/usr/src/sm5/debug# ./smartctl -a -r ioctl,2 -T verypermissive /dev/sda
smartctl version 5.34 [i686-pc-linux-gnu] Copyright (C) 2002-5 Bruce Allen
Home page is http://smartmontools.sourceforge.net/

 [inquiry: 12 00 00 00 24 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=0 milliseconds
  Incoming data, len=36:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30
  status=0x0
Device: IC35L030 AVVN07-0         Version:
 [mode sense: 1a 00 1c 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
 [inquiry: 12 01 80 00 40 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=4 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
  status=0x0
Vital Product Data (VPD) bit ignored in INQUIRY
Device type: disk
 [mode sense: 1a 00 19 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=4 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 e8 30 ef c7  80 35 23 c5 00 00 00 00
 10     70 6a bd c5 a0 84 11 c0  00 01 10 00 00 02 20 00
 20     18 5e 3f d0 20 af 02 c8  00 00 00 00 70 6a bd c5
 30     a0 84 11 c0 00 01 10 00  00 02 20 00 18 be db c5
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
Local Time is: Wed Nov  9 10:34:46 2005 CET
 [test unit ready: 00 00 00 00 00 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=0 milliseconds
  status=0x0
Device does not support SMART [unsupported field in scsi command]
 [log sense: 4d 00 40 00 00 00 00 00 04 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=4:
 00     54 5a 69 66
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 20 00 00 00
 10     00 00
  status=2: sense_key=5 asc=20 ascq=0
Log Sense for supported pages failed [unsupported scsi opcode]
 [request sense: 03 00 00 00 12 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=4 milliseconds
  Incoming data, len=18:
 00     70 00 00 00 00 00 00 0a  00 00 00 00 00 00 00 00
 10     00 00
  status=0x0
 [read defect list(10): 37 00 0c 00 00 00 00 00 04 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=4:
 00     70 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 20 00 00 00
 10     00 00
  status=2: sense_key=5 asc=20 ascq=0
Read defect list (10) Failed: unsupported scsi opcode

Error Counter logging not supported
 [mode sense: 1a 00 0a 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
Device does not support Self Test logging
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug# ./smartctl -a -r ioctl,2 -T verypermissive /dev/sg0
smartctl version 5.34 [i686-pc-linux-gnu] Copyright (C) 2002-5 Bruce Allen
Home page is http://smartmontools.sourceforge.net/

 [inquiry: 12 00 00 00 24 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=0 milliseconds
  Incoming data, len=36:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30
  status=0x0
Device: IC35L030 AVVN07-0         Version:
 [mode sense: 1a 00 1c 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 11 00 00 00  06 00 00 00 00 00 00 00
 30     00 00 00 00 11 00 00 00  07 00 00 00 00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
 [inquiry: 12 01 80 00 40 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=4 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 11 00 00 00  07 00 00 00 00 00 00 00
  status=0x0
Vital Product Data (VPD) bit ignored in INQUIRY
Device type: disk
 [mode sense: 1a 00 19 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=64:
 00     00 00 00 00 1f 00 00 00  49 43 33 35 4c 30 33 30
 10     41 56 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 11 00 00 00  07 00 00 00 00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
Local Time is: Wed Nov  9 10:35:47 2005 CET
 [test unit ready: 00 00 00 00 00 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=0 milliseconds
  status=0x0
Device does not support SMART [unsupported field in scsi command]
 [log sense: 4d 00 40 00 00 00 00 00 04 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=4:
 00     00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 20 00 00 00
 10     00 00
  status=2: sense_key=5 asc=20 ascq=0
Log Sense for supported pages failed [unsupported scsi opcode]
 [request sense: 03 00 00 00 12 00 ]
  scsi_status=0x0, host_status=0x0, driver_status=0x0
  info=0x0  duration=0 milliseconds
  Incoming data, len=18:
 00     70 00 00 00 00 00 00 0a  00 00 00 00 00 00 00 00
 10     00 00
  status=0x0
 [read defect list(10): 37 00 0c 00 00 00 00 00 04 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=4:
 00     70 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 20 00 00 00
 10     00 00
  status=2: sense_key=5 asc=20 ascq=0
Read defect list (10) Failed: unsupported scsi opcode

Error Counter logging not supported
 [mode sense: 1a 00 0a 00 40 00 ]
  scsi_status=0x2, host_status=0x0, driver_status=0x8
  info=0x1  duration=0 milliseconds
  Incoming data, len=64:
 00     70 00 00 00 00 00 00 0a  00 00 00 00 00 00 00 00
 10     00 00 56 4e 30 37 2d 30  20 20 20 20 20 20 20 20
 20     00 30 00 30 00 00 00 00  00 00 00 00 00 00 00 00
 30     00 00 00 00 11 00 00 00  07 00 00 00 00 00 00 00
  >>> Sense buffer, len=18:
 00     70 00 05 00 00 00 00 0a  00 00 00 00 24 00 00 00
 10     00 00
  status=2: sense_key=5 asc=24 ascq=0
Device does not support Self Test logging
tom1:/usr/src/sm5/debug# 

no change with sdparm:

tom1:/usr/src/sm5/debug# sdparm -avl /dev/sda
    /dev/sda: IC35L030  AVVN07-0
Read write error recovery [PS=0] mode page:
  AWRE        0  [cha: n, def:  0, sav:  0]  Automatic write reallocation enabled
  ARRE        0  [cha: n, def:  0, sav:  0]  Automatic read reallocation enabled
  TB          0  [cha: n, def:  0, sav:  0]  Transfer block
  RC          0  [cha: n, def:  0, sav:  0]  Read continuous
  EER         0  [cha: n, def:  0, sav:  0]  Enable early recover
  PER         0  [cha: n, def:  0, sav:  0]  Post error
  DTE         0  [cha: n, def:  0, sav:  0]  Data terminate on error
  DCR         0  [cha: n, def:  0, sav:  0]  Disable correction
  RRC         0  [cha: n, def:  0, sav:  0]  Read retry count
  WRC         0  [cha: n, def:  0, sav:  0]  Write retry count
  RTL         0  [cha: n, def:  0, sav:  0]  Recovery time limit (ms)
>> Format (SBC) mode page not supported
>> Rigid disk (SBC) mode page not supported
>> Verify error recovery (SBC) mode page not supported
Caching (SBC) [PS=0] mode page:
  IC          0  [cha: n, def:  0, sav:  0]  Initiator control
  ABPF        0  [cha: n, def:  0, sav:  0]  Abort pre-fetch
  CAP         0  [cha: n, def:  0, sav:  0]  Caching analysis permitted
  DISC        0  [cha: n, def:  0, sav:  0]  Discontinuity
  SIZE        0  [cha: n, def:  0, sav:  0]  Size (1->CSS valid, 0->NCS valid)
  WCE         0  [cha: n, def:  0, sav:  0]  Write cache enable
  MF          0  [cha: n, def:  0, sav:  0]  Multiplication factor
  RCD         0  [cha: n, def:  0, sav:  0]  Read cache disable
  DRRP        0  [cha: n, def:  0, sav:  0]  Demand read retension priority
  WRP         0  [cha: n, def:  0, sav:  0]  Write retension priority
  DPTL        0  [cha: n, def:  0, sav:  0]  Disable pre-fetch transfer length
  MIPF        0  [cha: n, def:  0, sav:  0]  Minimum pre-fetch
  MAPF        0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch
  MAPFC       0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch ceiling
 >> skipping rest as field position exceeds mode page length=12
>> Control mode page not supported
>> XOR control (SBC) mode page not supported
>> Protocol specific logical unit mode page not supported
>> Protocol specific port mode page not supported
>> Power condition mode page not supported
>> Informational exceptions control mode page not supported
>> Background control (SBC) mode subpage not supported
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug# sdparm -avl /dev/sg0
    /dev/sg0: IC35L030  AVVN07-0
Read write error recovery [PS=0] mode page:
  AWRE        0  [cha: n, def:  0, sav:  0]  Automatic write reallocation enabled
  ARRE        0  [cha: n, def:  0, sav:  0]  Automatic read reallocation enabled
  TB          0  [cha: n, def:  0, sav:  0]  Transfer block
  RC          0  [cha: n, def:  0, sav:  0]  Read continuous
  EER         0  [cha: n, def:  0, sav:  0]  Enable early recover
  PER         0  [cha: n, def:  0, sav:  0]  Post error
  DTE         0  [cha: n, def:  0, sav:  0]  Data terminate on error
  DCR         0  [cha: n, def:  0, sav:  0]  Disable correction
  RRC         0  [cha: n, def:  0, sav:  0]  Read retry count
  WRC         0  [cha: n, def:  0, sav:  0]  Write retry count
  RTL         0  [cha: n, def:  0, sav:  0]  Recovery time limit (ms)
>> Format (SBC) mode page not supported
>> Rigid disk (SBC) mode page not supported
>> Verify error recovery (SBC) mode page not supported
Caching (SBC) [PS=0] mode page:
  IC          0  [cha: n, def:  0, sav:  0]  Initiator control
  ABPF        0  [cha: n, def:  0, sav:  0]  Abort pre-fetch
  CAP         0  [cha: n, def:  0, sav:  0]  Caching analysis permitted
  DISC        0  [cha: n, def:  0, sav:  0]  Discontinuity
  SIZE        0  [cha: n, def:  0, sav:  0]  Size (1->CSS valid, 0->NCS valid)
  WCE         0  [cha: n, def:  0, sav:  0]  Write cache enable
  MF          0  [cha: n, def:  0, sav:  0]  Multiplication factor
  RCD         0  [cha: n, def:  0, sav:  0]  Read cache disable
  DRRP        0  [cha: n, def:  0, sav:  0]  Demand read retension priority
  WRP         0  [cha: n, def:  0, sav:  0]  Write retension priority
  DPTL        0  [cha: n, def:  0, sav:  0]  Disable pre-fetch transfer length
  MIPF        0  [cha: n, def:  0, sav:  0]  Minimum pre-fetch
  MAPF        0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch
  MAPFC       0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch ceiling
 >> skipping rest as field position exceeds mode page length=12
>> Control mode page not supported
>> XOR control (SBC) mode page not supported
>> Protocol specific logical unit mode page not supported
>> Protocol specific port mode page not supported
>> Power condition mode page not supported
>> Informational exceptions control mode page not supported
>> Background control (SBC) mode subpage not supported
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug# sdparm -avl6 /dev/sg0
    /dev/sg0: IC35L030  AVVN07-0
Read write error recovery [PS=0] mode page:
  AWRE        0  [cha: n, def:  0, sav:  0]  Automatic write reallocation enabled
  ARRE        0  [cha: n, def:  0, sav:  0]  Automatic read reallocation enabled
  TB          0  [cha: n, def:  0, sav:  0]  Transfer block
  RC          0  [cha: n, def:  0, sav:  0]  Read continuous
  EER         0  [cha: n, def:  0, sav:  0]  Enable early recover
  PER         0  [cha: n, def:  0, sav:  0]  Post error
  DTE         0  [cha: n, def:  0, sav:  0]  Data terminate on error
  DCR         0  [cha: n, def:  0, sav:  0]  Disable correction
  RRC         0  [cha: n, def:  0, sav:  0]  Read retry count
  WRC         0  [cha: n, def:  0, sav:  0]  Write retry count
  RTL         0  [cha: n, def:  0, sav:  0]  Recovery time limit (ms)
>> Format (SBC) mode page not supported
>> Rigid disk (SBC) mode page not supported
>> Verify error recovery (SBC) mode page not supported
Caching (SBC) [PS=0] mode page:
  IC          0  [cha: n, def:  0, sav:  0]  Initiator control
  ABPF        0  [cha: n, def:  0, sav:  0]  Abort pre-fetch
  CAP         0  [cha: n, def:  0, sav:  0]  Caching analysis permitted
  DISC        0  [cha: n, def:  0, sav:  0]  Discontinuity
  SIZE        0  [cha: n, def:  0, sav:  0]  Size (1->CSS valid, 0->NCS valid)
  WCE         0  [cha: n, def:  0, sav:  0]  Write cache enable
  MF          0  [cha: n, def:  0, sav:  0]  Multiplication factor
  RCD         0  [cha: n, def:  0, sav:  0]  Read cache disable
  DRRP        0  [cha: n, def:  0, sav:  0]  Demand read retension priority
  WRP         0  [cha: n, def:  0, sav:  0]  Write retension priority
  DPTL        0  [cha: n, def:  0, sav:  0]  Disable pre-fetch transfer length
  MIPF        0  [cha: n, def:  0, sav:  0]  Minimum pre-fetch
  MAPF        0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch
  MAPFC       0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch ceiling
 >> skipping rest as field position exceeds mode page length=12
>> Control mode page not supported
>> XOR control (SBC) mode page not supported
>> Protocol specific logical unit mode page not supported
>> Protocol specific port mode page not supported
>> Power condition mode page not supported
>> Informational exceptions control mode page not supported
>> Background control (SBC) mode subpage not supported
tom1:/usr/src/sm5/debug#   

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

thomas schorpp wrote:
> thomas schorpp wrote:
> 
>>Timothy Thelin wrote:
>>
>>
>>>>On Mon, 7 Nov 2005, James Bottomley wrote:
>>>>
>>>
>>>It was generated against Linus' git repo after
>>>applying Alan's patch.
>>>
>>>With both patches applied I can now do passthru cdbs on the
>>>cypress part without issue, and I still don't see any IO issues.
>>>
>>>Anyone out there have a better selection of hardware to test with?
>>>
>>
>>
>>first i need better software, the 2.6.14-git-11 snapshot wont boot, cause
>>it tries to mount root without detecting partitions before :/
>>
>>did i miss some patch since this snapshot?
>>
> 
> 
> ok, applies against 2.6.14.0. device: cypress -A- bridge.
> 
> no change with smartctl:
> 

err, ok, surely not with missing ata-passthru code ;)

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Mark Lord]

 >no change with smartctl:
 >tom1:/usr/src/sm5/debug# ./smartctl -a -r ioctl,2 -T verypermissive /dev/sda

Shouldn't there be a "-d ata" flag on those smartctl commands?

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Mark Lord wrote:
>>no change with smartctl:
>>tom1:/usr/src/sm5/debug# ./smartctl -a -r ioctl,2 -T verypermissive
> /dev/sda
> 
> Shouldn't there be a "-d ata" flag on those smartctl commands?
> -

? i cant find any ata-passthrough in smartctl cvs, the device is treated pure ata then.

ok, will add the (preliminary) passthrough opcodes to linux/scsi.h, 
include it in hdparm, add an option "-d atacb", then try it out there, not?

tried -d ata too. result negative as expected:

tom1:/usr/src/sm5/debug# ./smartctl -a -d ata -r ioctl,2 -T verypermissive /dev/sg0
smartctl version 5.34 [i686-pc-linux-gnu] Copyright (C) 2002-5 Bruce Allen
Home page is http://smartmontools.sourceforge.net/


REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY DEVICE
REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY DEVICE returned -1 errno=1 [Operation not permitted]

===== [IDENTIFY DEVICE] DATA START (BASE-16) =====
000-015: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
016-031: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
032-047: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
048-063: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
064-079: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
080-095: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
096-111: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
112-127: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
128-143: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
144-159: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
160-175: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
176-191: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
192-207: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
208-223: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
224-239: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
240-255: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
256-271: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
272-287: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
288-303: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
304-319: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
320-335: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
336-351: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
352-367: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
368-383: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
384-399: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
400-415: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
416-431: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
432-447: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
448-463: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
464-479: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
480-495: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
496-511: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
===== [IDENTIFY DEVICE] DATA END (512 Bytes) =====


REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY PACKET DEVICE
REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY PACKET DEVICE returned -1 errno=1 [Operation not permitted]

===== [IDENTIFY PACKET DEVICE] DATA START (BASE-16) =====
000-015: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
016-031: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
032-047: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
048-063: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
064-079: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
080-095: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
096-111: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
112-127: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
128-143: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
144-159: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
160-175: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
176-191: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
192-207: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
208-223: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
224-239: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
240-255: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
256-271: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
272-287: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
288-303: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
304-319: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
320-335: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
336-351: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
352-367: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
368-383: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
384-399: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
400-415: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
416-431: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
432-447: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
448-463: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
464-479: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
480-495: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
496-511: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
===== [IDENTIFY PACKET DEVICE] DATA END (512 Bytes) =====

Smartctl: Device Read Identity Failed (not an ATA/ATAPI device)

=== START OF INFORMATION SECTION ===
Device Model:     [No Information Found]
Serial Number:    [No Information Found]
Firmware Version: [No Information Found]
Device is:        Not in smartctl database [for details use: -P showall]
ATA Version is:   1
ATA Standard is:  Exact ATA specification draft version not indicated
Local Time is:    Wed Nov  9 14:53:22 2005 CET
SMART is only available in ATA Version 3 Revision 3 or greater.
We will try to proceed in spite of this.
SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 82-83 don't show if SMART supported.
                  Checking for SMART support by trying SMART ENABLE command.

REPORT-IOCTL: DeviceFD=3 Command=SMART ENABLE
REPORT-IOCTL: DeviceFD=3 Command=SMART ENABLE returned -1 errno=1 [Operation not permitted]
Error SMART Enable failed: Operation not permitted
                  SMART ENABLE failed - this establishes that this device lacks SMART functionality.
SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 85-87 don't show if SMART is enabled.
                  Checking to be sure by trying SMART RETURN STATUS command.

REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS
REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS returned -1 errno=1 [Operation not permitted]
SMART support is: Unavailable


REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS
REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS returned -1 errno=1 [Operation not permitted]
SMART Disabled. Use option -s with argument 'on' to enable it.
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug#
tom1:/usr/src/sm5/debug# smartctl -a -d ata -r ioctl,2 -T verypermissive /dev/sda
smartctl version 5.32 Copyright (C) 2002-4 Bruce Allen
Home page is http://smartmontools.sourceforge.net/


REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY DEVICE
REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY DEVICE returned -1 errno=22 [Invalid argument]

===== [IDENTIFY DEVICE] DATA START (BASE-16) =====
000-015: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
016-031: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
032-047: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
048-063: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
064-079: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
080-095: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
096-111: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
112-127: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
128-143: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
144-159: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
160-175: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
176-191: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
192-207: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
208-223: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
224-239: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
240-255: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
256-271: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
272-287: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
288-303: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
304-319: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
320-335: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
336-351: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
352-367: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
368-383: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
384-399: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
400-415: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
416-431: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
432-447: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
448-463: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
464-479: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
480-495: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
496-511: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
===== [IDENTIFY DEVICE] DATA END (512 Bytes) =====


REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY PACKET DEVICE
REPORT-IOCTL: DeviceFD=3 Command=IDENTIFY PACKET DEVICE returned -1 errno=22 [Invalid argument]

===== [IDENTIFY PACKET DEVICE] DATA START (BASE-16) =====
000-015: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
016-031: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
032-047: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
048-063: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
064-079: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
080-095: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
096-111: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
112-127: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
128-143: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
144-159: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
160-175: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
176-191: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
192-207: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
208-223: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
224-239: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
240-255: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
256-271: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
272-287: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
288-303: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
304-319: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
320-335: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
336-351: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
352-367: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
368-383: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
384-399: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
400-415: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
416-431: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
432-447: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
448-463: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
464-479: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
480-495: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
496-511: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
===== [IDENTIFY PACKET DEVICE] DATA END (512 Bytes) =====

Smartctl: Device Read Identity Failed (not an ATA/ATAPI device)

=== START OF INFORMATION SECTION ===
Device Model:     [No Information Found]
Serial Number:    [No Information Found]
Firmware Version: [No Information Found]
Device is:        Not in smartctl database [for details use: -P showall]
ATA Version is:   1
ATA Standard is:  Exact ATA specification draft version not indicated
Local Time is:    Wed Nov  9 14:53:35 2005 CET
SMART is only available in ATA Version 3 Revision 3 or greater.
We will try to proceed in spite of this.
SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 82-83 don't show if SMART supported.
                  Checking for SMART support by trying SMART ENABLE command.

REPORT-IOCTL: DeviceFD=3 Command=SMART ENABLE
REPORT-IOCTL: DeviceFD=3 Command=SMART ENABLE returned -1 errno=22 [Invalid argument]
Error SMART Enable failed: Invalid argument
                  SMART ENABLE failed - this establishes that this device lacks SMART functionality.
SMART support is: Ambiguous - ATA IDENTIFY DEVICE words 85-87 don't show if SMART is enabled.
                  Checking to be sure by trying SMART RETURN STATUS command.

REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS
REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS returned -1 errno=22 [Invalid argument]
SMART support is: Unavailable


REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS
REPORT-IOCTL: DeviceFD=3 Command=SMART STATUS returned -1 errno=22 [Invalid argument]
SMART Disabled. Use option -s with argument 'on' to enable it.
tom1:/usr/src/sm5/debug# 

[DOC PATCH] T10/04-262r8

[thomas schorpp]

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

thomas schorpp wrote:

> ok, will add the (preliminary) passthrough opcodes to linux/scsi.h, 
> include it in hdparm, add an option "-d atacb", then try it out there, not?
> 

doc patch

Signed-off-by: Thomas Schorpp <t.schorpp@gmx.de>




[-- Attachment #2: T10-04-262r8.patch --]
[-- Type: text/plain, Size: 406 bytes --]

--- linux-git/include/scsi/scsi.h	2005-11-09 15:38:02.000000000 +0100
+++ linux-2.6-dbg/include/scsi/scsi.h	2005-11-09 15:52:27.000000000 +0100
@@ -116,7 +116,7 @@
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
 
-/* Values for T10/04-262r7 */
+/* Values for T10/04-262r8 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */
 #define	ATA_12		      0xa1	/* 12-byte pass-thru */
 

Re: [DOC PATCH] T10/04-262r8

[Jeff Garzik]

thomas schorpp wrote:
> --- linux-git/include/scsi/scsi.h	2005-11-09 15:38:02.000000000 +0100
> +++ linux-2.6-dbg/include/scsi/scsi.h	2005-11-09 15:52:27.000000000 +0100
> @@ -116,7 +116,7 @@
>  /* values for service action in */
>  #define	SAI_READ_CAPACITY_16  0x10
>  
> -/* Values for T10/04-262r7 */
> +/* Values for T10/04-262r8 */
>  #define	ATA_16		      0x85	/* 16-byte pass-thru */
>  #define	ATA_12		      0xa1	/* 12-byte pass-thru */

This is a bit silly.  We should just remove the T10 doc reference, if 
people are going to be continually submitting patches to update this doc id.

	Jeff

Re: [DOC PATCH] T10/04-262r8

[James Bottomley]

> >  #define	ATA_16		      0x85	/* 16-byte pass-thru */
> >  #define	ATA_12		      0xa1	/* 12-byte pass-thru */
> 
> This is a bit silly.  We should just remove the T10 doc reference, if 
> people are going to be continually submitting patches to update this doc id.

Agreed ... especially since 04-262 is just a proposed update to SAT
which is what actually defines these command opcodes, so the comment
isn't even correct.

James

Re: [DOC PATCH] T10/04-262r8

[thomas schorpp]

James Bottomley wrote:
>>> #define	ATA_16		      0x85	/* 16-byte pass-thru */
>>> #define	ATA_12		      0xa1	/* 12-byte pass-thru */
>>

is this all, or do we have a preliminary SATL in kernel yet? cant find.

how about full definitions of the pasthru cdb's in kernel structs and approbiate ioctls?

for hdparm ata-security theres ide taskfile used, can scsi command sequences combined with ide taskfiles 
which use own ioctls?
are taskfiles needed for this in any case?

for smartctl theres sg_cmnd_io() with (user definable?) struct io_hdr. 
so i would try passthru test first with smartctl by adding passthru opcode ATA_12 to smartctl struct scsi_cmd_io io_hdr.
later combining scsi ioparm structs and passthru opcode with usable contents of ide taskfile parm structs in user space.

the passthru cdb is also not clear enough to me:
according to sat-r06 spec byte 0-2 is scsi, 3-9 are ata, 11 is scsi, yes?

thx
tom

Re: [DOC PATCH] T10/04-262r8

[Jeff Garzik]

thomas schorpp wrote:
> James Bottomley wrote:
> 
>>>>#define	ATA_16		      0x85	/* 16-byte pass-thru */
>>>>#define	ATA_12		      0xa1	/* 12-byte pass-thru */
>>>
> 
> is this all, or do we have a preliminary SATL in kernel yet? cant find.

It's been there for more than a year:  drivers/scsi/libata-scsi.c


> how about full definitions of the pasthru cdb's in kernel structs and approbiate ioctls?

upstream as of 2.6.14-git


> for hdparm ata-security theres ide taskfile used, can scsi command sequences combined with ide taskfiles 
> which use own ioctls?
> are taskfiles needed for this in any case?

in libata, the HDIO ioctls are implemented by creating issuing a SCSI 
command (ATA passthru).

	Jeff

Re: [DOC PATCH] T10/04-262r8

[Mark Lord]

thomas schorpp wrote:
>
> for hdparm ata-security theres ide taskfile used, can scsi command sequences combined with ide taskfiles 
> which use own ioctls?

As Jeff said, libata translates some of the HDIO_TASK* functions
into SCSI passthru commands, which then get translated back
into ATA commands at the time of issue to the device.

Once passthru makes it out into the wild (Linus kernels),
I'll update hdparm to have it use SCSI passthru instead
when available/appropriate.

There will have to be about a year or so of full overlap
(hdparm, smartmontools, and kernel), to give distros time
to update/release with all of the affected components.

After that, I imagine Jeff may want to remove the HDIO_TASK*
calls again.

Cheers

(SAT) libata + hdparm/smartctl

[thomas schorpp]

Mark Lord wrote:
> thomas schorpp wrote:
> 
>>
>> for hdparm ata-security theres ide taskfile used, can scsi command
>> sequences combined with ide taskfiles which use own ioctls?
> 
> 
> As Jeff said, libata translates some of the HDIO_TASK* functions
> into SCSI passthru commands, which then get translated back
> into ATA commands at the time of issue to the device.
> 
> Once passthru makes it out into the wild (Linus kernels),
> I'll update hdparm to have it use SCSI passthru instead
> when available/appropriate.
> 
> There will have to be about a year or so of full overlap
> (hdparm, smartmontools, and kernel), to give distros time
> to update/release with all of the affected components.
> 
> After that, I imagine Jeff may want to remove the HDIO_TASK*
> calls again.
> 
> Cheers

ive tried the whole day to make this driver helper libata or parts of it available to the user space
and it didnt work out, sorry, you cant include the header in user-space apps and theres no interface to it.

but if You say so, i'll take a closer look at libatas HDIO_TASKs but this has to be utilized automatic then,
at this time libata.ko does not get loaded or used by usbstorage devices.

when it is a driver library i will use it with drivers, as it should be:

so i look if i can modify sg, sd_mod or/and usb-storage to detect passthru needing devices automatic and to
take action on passthru needing functionality, then none of the user space apps should need to be worked on.

(hopefully) 

y
tom

Re: (SAT) libata + hdparm/smartctl

[Douglas Gilbert]

thomas schorpp wrote:
> Mark Lord wrote:
> 
>>thomas schorpp wrote:
>>
>>
>>>for hdparm ata-security theres ide taskfile used, can scsi command
>>>sequences combined with ide taskfiles which use own ioctls?
>>
>>
>>As Jeff said, libata translates some of the HDIO_TASK* functions
>>into SCSI passthru commands, which then get translated back
>>into ATA commands at the time of issue to the device.
>>
>>Once passthru makes it out into the wild (Linus kernels),
>>I'll update hdparm to have it use SCSI passthru instead
>>when available/appropriate.
>>
>>There will have to be about a year or so of full overlap
>>(hdparm, smartmontools, and kernel), to give distros time
>>to update/release with all of the affected components.
>>
>>After that, I imagine Jeff may want to remove the HDIO_TASK*
>>calls again.
>>
>>Cheers
> 
> 
> ive tried the whole day to make this driver helper libata or parts of it available to the user space
> and it didnt work out, sorry, you cant include the header in user-space apps and theres no interface to it.
> 
> but if You say so, i'll take a closer look at libatas HDIO_TASKs but this has to be utilized automatic then,
> at this time libata.ko does not get loaded or used by usbstorage devices.
> 
> when it is a driver library i will use it with drivers, as it should be:
> 
> so i look if i can modify sg, sd_mod or/and usb-storage to detect passthru needing devices automatic and to
> take action on passthru needing functionality, then none of the user space apps should need to be worked on.
BTW The ATA COMMAND PASS THROUGH (12) SCSI command shares its
opcode (0xa1) with the MMC BLANK command. According to sat-r07
a SAT layer should identify itself by using "ATA     " as
the vendor string of its standard INQUIRY response.



For those not aware in lk 2.6.15-rc1 smartctl (from smartmontools)
works. First you see:

# smartctl -a /dev/sda
smartctl version 5.34 [i686-pc-linux-gnu] Copyright (C) 2002-5 Bruce Allen
Home page is http://smartmontools.sourceforge.net/

Device: ATA      ST380013AS       Version: 3.18

In Linux, SATA disks accessed via libata are not currently supported
by smartmontools. By the time you read this, support may have been added
in recent kernels. Try an additional '-d ata' argument.


Following that advice:

# smartctl -a -d ata /dev/sda
<output just like an (p)ATA disk>


Recent versions of hdparm also work (e.g. "hdparm -I /dev/sda").

Doug Gilbert

Re: [DOC PATCH] T10/04-262r8

[James Bottomley]

On Thu, 2005-11-10 at 07:19 +0100, thomas schorpp wrote:
> how about full definitions of the pasthru cdb's in kernel structs and approbiate ioctls?

Why would we want to do that?  The whole point about pass through is
that it is designed to use the existing SCSI infrastructure
transparently (hence use SG_IO to send an ATA command to a bridge that
understands it---but the key point here is that the device so addressed
must understand the opcodes).

> the passthru cdb is also not clear enough to me:
> according to sat-r06 spec byte 0-2 is scsi, 3-9 are ata, 11 is scsi, yes?

Actually, no; byte 0 is the opcode, so you could call that SCSI and byte
11 (or 15) is the control as defined by SAM; the rest all contain SAT
specific fields.

James

RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Timothy Thelin]

> > 
> > Can we trigger black listing based on the above 
> vendor/product values?
> > i.e. can you check for these values in usb-storage 
> slave_configure code?
> 
> Yes.  The patch below illustrates how.  You'll have to change 
> the name of 
> the sdev flag to something sensible...
> 
> I firmly believe this is the wrong approach, however.  It's a 
> specific 
> solution to a general problem.  I would much prefer to add a 
> new flag to 
> struct request.

I totally agree.

> 
> > Yeh, some other hardware might want lun inihibit in the 
> future, and that
> > could still be added back, but IMHO black list is better.
> 
> I'd say it's inevitable.  And it may even end up being 
> standard-based, not 
> vendor/product based.

Within a year from now, there are going to be USB devices
natively supporting the SAT passthru CDB.  Once this happens,
its going to be CDB based and not vendor/product based on
when to not mangle the CDB; chips from different vendors
will support the CDB.  The blacklist approach really isn't
going to scale, where the flag approch will.  Plus with
a blacklist there would always be a lead time until a
device with passthru supported works correctly under the
kernel (blacklist update), where with a flag approach,
support is automatic.

Regards,
Tim Thelin

RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Timothy Thelin]

> > Okay, what am I missing?
> > 
> > Looking at that patch, it looks to me like if sg.c set a 
> flag in the SCSI
> > command block then scsi_core.c could use that flag in the 
> test to determine
> > if the LUN should be masked-in.
> > 
> > HOWEVER, I keep hearing that the changes will be extensive. 
>  What am I
> > missing?
> 
> Not extensive AFAICT ... 
> 
> The scmd/cdb is not available until we call scsi_get_command() in the
> request function. So you would have to add a field into 
> scsi_request, set
> it in sg (in both sg.c block/scsi_ioctl.c, test via sd and sg 
> when you are
> done), and add the field in scsi_cmnd, and set scmd one in
> scsi_init_cmd_from_req().
> 

I'l sign up as a tester =D

> But as far as black listing, it does seem like a better 
> solution in that
> user apps do not need special code.
> 
> Do the devices that require it (well so far) have useable 
> vendor + model
> strings or usb id's?
> 

Actually, from my point of view its better to do the flag than
use a blacklist.  Blacklists require active maintenence, so
everytime WD releases a new product based on this cypress chip,
the kernel needs yet another entry (and there are a couple more
on our roadmap).  I actually see the flag solution as a more
generic forward looking solution; it's one patch verses one
patch per device. And from the point of view of someone who
creates utilities that need raw access to hard drives, this
flag is trivial to support in userspace and makes logical sense.

The two userspace cases I can think of:
1) Tool is aware of what device its trying to access.  Well since
it knows the device it wants to get access to, it will also
be aware of if it needs that flag or not.
2) Tool is generic (sdparm, hdparm) and tries to access whatever
device it's pointed at.  We'll based off of the cdb it tries to
construct, its going to know if it needs that flag or not.  Both
the ATACB and the SAT passthru CDB would need that flag since
they both have meaningful information in the LUN bits.  For cdbs
that don't have meaningful bits there, they don't have to use the
flag.

So by going the flag route, you need one kernel change, and one
userspace change per tool to be flag aware.  Then future products
that use the same passthru cdbs will get supported for free, unlike
the blacklist approach.


Regards,
Tim Thelin
Software Engineer
Western Digital

RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Timothy Thelin]

> 
> On Thu, Nov 03, 2005 at 11:08:07PM -0500, James Bottomley wrote:
> > On Wed, 2005-11-02 at 15:45 -0800, Matthew Dharm wrote:
> > > On Wed, Nov 02, 2005 at 02:18:52PM -0800, Timothy Thelin wrote:
> > > > 
> > > > If you had time to spare, instead of touching usb-storage,
> > > > it might be better spent resurecting SG_FLAG_LUN_INHIBIT to
> > > > stop the above behavior so that SG_IO cdbs can be passed
> > > > through untouched.
> > > > (SG_FLAG_FUN_INHIBIT was a flag SG_IO used to support a long
> > > > time ago, and I have no idea why it was dropped, but it was)
> > > 
> > > I didn't realize that had been removed.  Anyone that sends a
> > > vendor-specific command to a device needs this flag to 
> make sure it goes
> > > through unmangled.
> > > 
> > > Perhaps someone on linux-scsi can comment on why this was 
> removed and how
> > > we might get it back?
> > 
> > I've no distinct recollection of someone removing this, but if I
> > remember correctly what it used to do, it was a hack to stop us from
> > mangling SCSI-3 CDB's.  We fixed the mid-layer not to 
> require the hack
> > by only setting the CDB[1] lun field for SCSI-1 and SCSI-2 
> devices (as
> > the standards mandate).  What's the actual problem?  No 
> SCSI-1 or SCSI-2
> > device should have any vendor specific CDBs that uses these bits in
> > CDB[1].
> 
> Unfortunately, reality appears to disagree with the last 
> "should".  I've
> personally seen devices with vendor-specific commands that 
> want to control
> CDB[1] in SCSI-2.
> 
> I didn't know it was removed; I only know what Timothy Thelin 
> told me.  Can
> we get the feature back?
> 
> Matt

And for an even more concrete example:
The CY7C68300B cypress bridge board (has various siblings as well
on their site that act very similar) implements SCSI spec 0 (ie it
doesn't claim to support any scsi spec).  Now usb-storage sees
this in inquiry, and decides to export the device as a scsi2 device
since based on the usb-storage devs' experience most usb devices
really want scsi2 cdbs.  So SCSI core thinks this is a scsi2 device
and procedes to mangle the cdbs as they're going through.

The problem then is with vendor specific commands as SCSI core
mangles those as well.  The vendor specific command I had issues
with is the "ATACB" ATA passthru cdb that cypress boards understand.
CDB[1] is a signature byte that must be 0x24.  Needless to say,
that leading 2 gets masked off to a 0, and the drive aborts the
ATACB.  Now cypress could have been smarter about designing their
cdb, but they wern't, and so there really needs to be a way to tell
SCSI core "hands off the cdb".

This is also going to be an issue with USB devices that implement
SAT passthru but state they implement SCSI spec 0, as SAT passthru
CDB[1] can't afford to be mangled either.

I started this conversation a little over a month ago, but it died.
http://marc.theaimsgroup.com/?l=linux-scsi&m=112714917116713&w=2

Regards,
Tim Thelin

RE: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Fri, 2005-11-04 at 10:30 -0800, Timothy Thelin wrote:
> And for an even more concrete example:
> The CY7C68300B cypress bridge board (has various siblings as well
> on their site that act very similar) implements SCSI spec 0 (ie it
> doesn't claim to support any scsi spec).  Now usb-storage sees
> this in inquiry, and decides to export the device as a scsi2 device
> since based on the usb-storage devs' experience most usb devices
> really want scsi2 cdbs.  So SCSI core thinks this is a scsi2 device
> and procedes to mangle the cdbs as they're going through.

What happens if you prevent USB mangling the scsi_level?  I think, for
the most part, we would handle 0 in about the same way as we handle 2.
However, we could gate the if around the CDB[1] mangling as

if (scsi_level != SCSI_UNKNOWN && scsi_level <= SCSI_2)

which should fix your problem, I think.

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Fri, Nov 04, 2005 at 12:58:27PM -0600, James Bottomley wrote:
> On Fri, 2005-11-04 at 10:30 -0800, Timothy Thelin wrote:
> > And for an even more concrete example:
> > The CY7C68300B cypress bridge board (has various siblings as well
> > on their site that act very similar) implements SCSI spec 0 (ie it
> > doesn't claim to support any scsi spec).  Now usb-storage sees
> > this in inquiry, and decides to export the device as a scsi2 device
> > since based on the usb-storage devs' experience most usb devices
> > really want scsi2 cdbs.  So SCSI core thinks this is a scsi2 device
> > and procedes to mangle the cdbs as they're going through.
> 
> What happens if you prevent USB mangling the scsi_level?  I think, for
> the most part, we would handle 0 in about the same way as we handle 2.
> However, we could gate the if around the CDB[1] mangling as
> 
> if (scsi_level != SCSI_UNKNOWN && scsi_level <= SCSI_2)
> 
> which should fix your problem, I think.

A long time ago, usb-storage didn't mangle the scsi_level.  And almost
nothing worked.  Without a SCSI level, 6-byte commands get sent; only
10-byte commands will do.

Matt

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

I see you've been reading alt.sex.chubby.sheep voraciously.
					-- Tanya
User Friendly, 11/24/97

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Fri, 2005-11-04 at 12:30 -0800, Matthew Dharm wrote:
> > What happens if you prevent USB mangling the scsi_level?  I think, for
> > the most part, we would handle 0 in about the same way as we handle 2.
> > However, we could gate the if around the CDB[1] mangling as
> > 
> > if (scsi_level != SCSI_UNKNOWN && scsi_level <= SCSI_2)
> > 
> > which should fix your problem, I think.
> 
> A long time ago, usb-storage didn't mangle the scsi_level.  And almost
> nothing worked.  Without a SCSI level, 6-byte commands get sent; only
> 10-byte commands will do.

That area of the scsi subsystem has been changed significantly ...
mainly for usb storage.  I don't believe your assertion about
SCSI_UNKOWN only producing 6 byte CDBs to be correct, the behaviour is
now governed by two scsi_device flags: use_10_for_rw and use_10_for_ms
which you can control in slave configure.

Can you just try it with a modern kernel and see if anything still
breaks?

Thanks,

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Fri, Nov 04, 2005 at 02:49:55PM -0600, James Bottomley wrote:
> On Fri, 2005-11-04 at 12:30 -0800, Matthew Dharm wrote:
> > > What happens if you prevent USB mangling the scsi_level?  I think, for
> > > the most part, we would handle 0 in about the same way as we handle 2.
> > > However, we could gate the if around the CDB[1] mangling as
> > > 
> > > if (scsi_level != SCSI_UNKNOWN && scsi_level <= SCSI_2)
> > > 
> > > which should fix your problem, I think.
> > 
> > A long time ago, usb-storage didn't mangle the scsi_level.  And almost
> > nothing worked.  Without a SCSI level, 6-byte commands get sent; only
> > 10-byte commands will do.
> 
> That area of the scsi subsystem has been changed significantly ...
> mainly for usb storage.  I don't believe your assertion about
> SCSI_UNKOWN only producing 6 byte CDBs to be correct, the behaviour is
> now governed by two scsi_device flags: use_10_for_rw and use_10_for_ms
> which you can control in slave configure.
> 
> Can you just try it with a modern kernel and see if anything still
> breaks?

I just realized this plan has a problem...

The reported SCSI level of a device is mostly garbage, but not always.
I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
seems independent of what devices have vendor-specific commands (and thus
need the CDB[1] not messed with).

It is an interesting experiment to remove the force-to-SCSI_2 part of the
usb-storage code (on the general principal of "we shouldn't be messing with
the data passed through the driver), but it doesn't solve the original
question of needing a way to pass commands without CDB[1] getting altered.

Matt

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

Pitr!  That's brilliant!  Funded by Microsoft refunds.  What sweet irony!
					-- A.J.
User Friendly, 2/15/1999

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Sat, 2005-11-05 at 15:55 -0800, Matthew Dharm wrote:
> On Fri, Nov 04, 2005 at 02:49:55PM -0600, James Bottomley wrote: 
> > Can you just try it with a modern kernel and see if anything still
> > breaks?
> 
> I just realized this plan has a problem...
> 
> The reported SCSI level of a device is mostly garbage, but not always.
> I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
> seems independent of what devices have vendor-specific commands (and thus
> need the CDB[1] not messed with).
> 
> It is an interesting experiment to remove the force-to-SCSI_2 part of the
> usb-storage code (on the general principal of "we shouldn't be messing with
> the data passed through the driver), but it doesn't solve the original
> question of needing a way to pass commands without CDB[1] getting altered.

Well, that might be a problem if it weren't for the fact that this
LUN_INHIBIT flag was removed in 2002.  If it's taken three years to find
a device that has a problem with it, I don't really think it's a
particularly widespread problem.  And since the device that now shows
the problem is setting the level to 0, it looks like we have a potential
solution that fits all known cases.

Anyway, the goal should be to handle devices in a standards compliant
manner first and then worry about quirk tables when that doesn't
work ... we have an incredibly broad quirk infrastructure in SCSI for
this.

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

James Bottomley wrote:
> On Sat, 2005-11-05 at 15:55 -0800, Matthew Dharm wrote:
> 
>>On Fri, Nov 04, 2005 at 02:49:55PM -0600, James Bottomley wrote: 
>>
>>>Can you just try it with a modern kernel and see if anything still
>>>breaks?
>>
>>I just realized this plan has a problem...
>>
>>The reported SCSI level of a device is mostly garbage, but not always.
>>I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
>>seems independent of what devices have vendor-specific commands (and thus
>>need the CDB[1] not messed with).
>>
>>It is an interesting experiment to remove the force-to-SCSI_2 part of the
>>usb-storage code (on the general principal of "we shouldn't be messing with
>>the data passed through the driver), but it doesn't solve the original
>>question of needing a way to pass commands without CDB[1] getting altered.
> 
> 
> Well, that might be a problem if it weren't for the fact that this
> LUN_INHIBIT flag was removed in 2002.  If it's taken three years to find
> a device that has a problem with it, I don't really think it's a
> particularly widespread problem.  And since the device that now shows
> the problem is setting the level to 0, it looks like we have a potential
> solution that fits all known cases.
> 
> Anyway, the goal should be to handle devices in a standards compliant
> manner first and then worry about quirk tables when that doesn't
> work ... we have an incredibly broad quirk infrastructure in SCSI for
> this.
> 
> James
> 

ok, ive just ordered 2 cypress CY7C68300B samples and will replace the discont'd A type in my device.
datasheet for A says nothing about ATACB and security. sheet for B does guarantee.

cypress windows driver reports no security, too.

so i will replace the chip to have a testing base.

tom

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Sat, Nov 05, 2005 at 06:49:05PM -0600, James Bottomley wrote:
> On Sat, 2005-11-05 at 15:55 -0800, Matthew Dharm wrote:
> > On Fri, Nov 04, 2005 at 02:49:55PM -0600, James Bottomley wrote: 
> > > Can you just try it with a modern kernel and see if anything still
> > > breaks?
> > 
> > I just realized this plan has a problem...
> > 
> > The reported SCSI level of a device is mostly garbage, but not always.
> > I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
> > seems independent of what devices have vendor-specific commands (and thus
> > need the CDB[1] not messed with).
> > 
> > It is an interesting experiment to remove the force-to-SCSI_2 part of the
> > usb-storage code (on the general principal of "we shouldn't be messing with
> > the data passed through the driver), but it doesn't solve the original
> > question of needing a way to pass commands without CDB[1] getting altered.
> 
> Well, that might be a problem if it weren't for the fact that this
> LUN_INHIBIT flag was removed in 2002.  If it's taken three years to find
> a device that has a problem with it, I don't really think it's a
> particularly widespread problem.  And since the device that now shows
> the problem is setting the level to 0, it looks like we have a potential
> solution that fits all known cases.

Was it really removed that long ago?  I guess there are still a lot of
people using 2.4 kernels, then.

I've been recommending this flag to people as recently as 6 months ago
(i.e. the last time someone asked me).   It comes up 2-3 times a year.

The last time it came up, the device set SCSI_2.

Matt

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

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Matthew Dharm wrote:
> On Sat, Nov 05, 2005 at 06:49:05PM -0600, James Bottomley wrote:
> 
>>On Sat, 2005-11-05 at 15:55 -0800, Matthew Dharm wrote:
>>
>>>On Fri, Nov 04, 2005 at 02:49:55PM -0600, James Bottomley wrote: 
>>>
>>>>Can you just try it with a modern kernel and see if anything still
>>>>breaks?
>>>
>>>I just realized this plan has a problem...
>>>
>>>The reported SCSI level of a device is mostly garbage, but not always.
>>>I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
>>>seems independent of what devices have vendor-specific commands (and thus
>>>need the CDB[1] not messed with).
>>>
>>>It is an interesting experiment to remove the force-to-SCSI_2 part of the
>>>usb-storage code (on the general principal of "we shouldn't be messing with
>>>the data passed through the driver), but it doesn't solve the original
>>>question of needing a way to pass commands without CDB[1] getting altered.
>>
>>Well, that might be a problem if it weren't for the fact that this
>>LUN_INHIBIT flag was removed in 2002.  If it's taken three years to find
>>a device that has a problem with it, I don't really think it's a
>>particularly widespread problem.  And since the device that now shows
>>the problem is setting the level to 0, it looks like we have a potential
>>solution that fits all known cases.
> 
> 
> Was it really removed that long ago?  

yes
...
-	if (!(hp->flags & SG_FLAG_LUN_INHIBIT)) {
-		if (sdp->device->scsi_level <= SCSI_2)
-			cmnd[1] = (cmnd[1] & 0x1f) | (sdp->device->lun << 5);
-	}
...
17. 2002-10-15  [RFC PATCH] consolidate SCSI-2 command lun setting    linux-scs Patrick Mansfield 

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Patrick Mansfield]

On Sun, Nov 06, 2005 at 11:28:36PM +0100, thomas schorpp wrote:
> > 
> > 
> > Was it really removed that long ago?  
> 
> yes

Hmmm yeh, here is the original email with patch:

http://marc.theaimsgroup.com/?l=linux-scsi&m=103470224812028&w=2

Black listing would cover more cases than adding back the SG_IO flag.

Black listing USB devices is somewhat bad/tricky, since they often don't
show up with registered vendor + model names, and use somewhat generic
names, for example I have an xd reader that has vendor "Generic", model
"STORAGE DEVICE". 

So either the hardware is stupid, or usb-storage generates those names and
maybe everything is OK (simple search finds nothing in usb storage code).
We do have some of those name in the current scsi devinfo table ... I haven't
looked for details on them or tried to figure out if they hurt any
devices.

Other potential methods besides black list based on vendor + model:

1) If (big if, i.e. unlikely) USB devices in general don't need
the LUN in the cdb, we could black list all USB to skip the storing of it. 

The majority of the (single LUN) USB devices probably never look
at the field, so perhaps we could refine the above to: 

2) Black list all usb devices that don't have multiple LUNs, assuming the
never check the cdb for a LUN value.

I don't know if you can tell (in usb code) if usb mass storage is
multi-lun or not without scanning for the other LUNs (normal SCSI devices
would have to be scanned or sent a REPORT LUNS, and even then, they could
show up with more LUNs in the future).

3) black list via usb storage based on usb id's, kind of like the handling
in usb slave_configure for USB_VENDOR_ID_GENESYS, though a more general
method would be better.

All of the above methods still should have defines in linux scsi devinfo
table and a corresponding new scsi_device->no_cdb_lun_setting field, and
setting of it in scsi_scan.c:scsi_add_lun (calls slave_configure near
the end, so then slave_configure can set no_cdb_lun_setting as required).

The slave_configure can then set sdev specific values, such as those that
might be set in linux scsi core black list, see current driver/usb/storage
scsiglue.c:slave_configure().

-- Patrick Mansfield

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Sun, Nov 06, 2005 at 03:52:13PM -0800, Patrick Mansfield wrote:
> Black listing would cover more cases than adding back the SG_IO flag.
> 
> Black listing USB devices is somewhat bad/tricky, since they often don't
> show up with registered vendor + model names, and use somewhat generic
> names, for example I have an xd reader that has vendor "Generic", model
> "STORAGE DEVICE". 
> 
> So either the hardware is stupid, or usb-storage generates those names and
> maybe everything is OK (simple search finds nothing in usb storage code).

The hardware is stupid.  Really, I'm not that stupid, even on my worst day.

It's worse than that, actually.  I've seen devices where the VID/PID is
that of the USB/IDE bridge vendor, and the INQUIRY gives "Generic".
There's really no way to programmatically distinguish one device from
another with high reliability.

> We do have some of those name in the current scsi devinfo table ... I haven't
> looked for details on them or tried to figure out if they hurt any
> devices.
> 
> Other potential methods besides black list based on vendor + model:
> 
> 1) If (big if, i.e. unlikely) USB devices in general don't need
> the LUN in the cdb, we could black list all USB to skip the storing of it. 

Nope.  It's pretty common to see 5/6/7-in-one media readers which are
multi-LUN.

> The majority of the (single LUN) USB devices probably never look
> at the field, so perhaps we could refine the above to: 
> 
> 2) Black list all usb devices that don't have multiple LUNs, assuming the
> never check the cdb for a LUN value.

Iffy.  Some single-LUN devices look at the LUN field, some don't.  I'd say
it's about a 60/40 split.

> I don't know if you can tell (in usb code) if usb mass storage is
> multi-lun or not without scanning for the other LUNs (normal SCSI devices
> would have to be scanned or sent a REPORT LUNS, and even then, they could
> show up with more LUNs in the future).

For the vast majority of USB storage devices (but not all), you can
determine this pretty well.  BBB devices support a GetMaxLUN command --
usb-storage uses it to set an internal variable, but it could be passed
elsewhere.

> 3) black list via usb storage based on usb id's, kind of like the handling
> in usb slave_configure for USB_VENDOR_ID_GENESYS, though a more general
> method would be better.

See my response to #1 above.  Vendors are, apparently, stupid -- at least
when it comes to making sure devices distinguish themselves in a nice
manner.

Matt

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

We can customize our colonels.
					-- Tux
User Friendly, 12/1/1998

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Sun, Nov 06, 2005 at 03:52:13PM -0800, Patrick Mansfield wrote:
> On Sun, Nov 06, 2005 at 11:28:36PM +0100, thomas schorpp wrote:
> > > 
> > > 
> > > Was it really removed that long ago?  
> > 
> > yes
> 
> Hmmm yeh, here is the original email with patch:
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=103470224812028&w=2

Okay, what am I missing?

Looking at that patch, it looks to me like if sg.c set a flag in the SCSI
command block then scsi_core.c could use that flag in the test to determine
if the LUN should be masked-in.

HOWEVER, I keep hearing that the changes will be extensive.  What am I
missing?

Matt

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

Oh great modem, why hast thou forsaken me?
					-- Dust Puppy
User Friendly, 3/2/1998

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Patrick Mansfield]

On Mon, Nov 07, 2005 at 09:05:25AM -0800, Matthew Dharm wrote:

> Okay, what am I missing?
> 
> Looking at that patch, it looks to me like if sg.c set a flag in the SCSI
> command block then scsi_core.c could use that flag in the test to determine
> if the LUN should be masked-in.
> 
> HOWEVER, I keep hearing that the changes will be extensive.  What am I
> missing?

Not extensive AFAICT ... 

The scmd/cdb is not available until we call scsi_get_command() in the
request function. So you would have to add a field into scsi_request, set
it in sg (in both sg.c block/scsi_ioctl.c, test via sd and sg when you are
done), and add the field in scsi_cmnd, and set scmd one in
scsi_init_cmd_from_req().

But as far as black listing, it does seem like a better solution in that
user apps do not need special code.

Do the devices that require it (well so far) have useable vendor + model
strings or usb id's?

-- Patrick Mansfield

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Mon, 7 Nov 2005, Patrick Mansfield wrote:

> On Mon, Nov 07, 2005 at 09:05:25AM -0800, Matthew Dharm wrote:
> 
> > Okay, what am I missing?
> > 
> > Looking at that patch, it looks to me like if sg.c set a flag in the SCSI
> > command block then scsi_core.c could use that flag in the test to determine
> > if the LUN should be masked-in.
> > 
> > HOWEVER, I keep hearing that the changes will be extensive.  What am I
> > missing?
> 
> Not extensive AFAICT ... 
> 
> The scmd/cdb is not available until we call scsi_get_command() in the
> request function. So you would have to add a field into scsi_request, set
> it in sg (in both sg.c block/scsi_ioctl.c, test via sd and sg when you are
> done), and add the field in scsi_cmnd, and set scmd one in
> scsi_init_cmd_from_req().

Isn't it true that scsi_request is going away?  Everything will have to 
use generic request structures.

> But as far as black listing, it does seem like a better solution in that
> user apps do not need special code.

I don't think blacklisting is a good way to do this.  In principle any USB 
mass storage device -- any SCSI device, in fact -- might have a 
vendor-specific pass-thru needing special handling.  It doesn't have to be 
correlated with the vendor, the product, the SCSI level, the transport, or 
anything else.

Alan Stern

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Alan Stern wrote:
> On Mon, 7 Nov 2005, Patrick Mansfield wrote:
> 
> 

> 
> I don't think blacklisting is a good way to do this.  In principle any USB 
> mass storage device -- any SCSI device, in fact -- might have a 
> vendor-specific pass-thru needing special handling.

no. that would be not industry best practice and uneconomical sw effort.
remember this devices are mainly "designed for windows" and WHQL certification 
is expensive for every single driver. and ms has only one driver for all usb storage 
right now, all boxes and sticks i had here use it.

>  It doesn't have to be 
> correlated with the vendor, the product, the SCSI level, the transport, or 
> anything else.

yes. the sheet for design recommended cypress chips state all ATACB and ATA-Security.
i would be make no sense to implement different behaviuors in chips for the same purpose.

> 
> Alan Stern
> 

tom schorpp

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Mon, 7 Nov 2005, thomas schorpp wrote:

> > I don't think blacklisting is a good way to do this.  In principle any USB 
> > mass storage device -- any SCSI device, in fact -- might have a 
> > vendor-specific pass-thru needing special handling.
> 
> no. that would be not industry best practice and uneconomical sw effort.

Adding vendor-specific codes to SCSI devices isn't industry best practice?  
I can't argue with that, but it is very common nevertheless.

Is it an uneconomical software effort?  Ridiculous -- it's not software at 
all, it's part of the firmware.

> remember this devices are mainly "designed for windows" and WHQL certification 
> is expensive for every single driver. and ms has only one driver for all usb storage 
> right now, all boxes and sticks i had here use it.

I think you have a very limited view of how devices are "designed".  
Besides, I specifically said I was talking about _all_ SCSI devices, not 
just USB ones.

> >  It doesn't have to be 
> > correlated with the vendor, the product, the SCSI level, the transport, or 
> > anything else.
> 
> yes. the sheet for design recommended cypress chips state all ATACB and ATA-Security.
> i would be make no sense to implement different behaviuors in chips for the same purpose.

That's what Cypress does, sure.  But who says that other vendors have to 
copy Cypress?  For the most part they don't.  Neither does T10.

As for whether it makes no sense -- you have your point of view.  Business 
people have a very different point of view; to them it might make 
excellent sense, to help secure a competitive advantage.

Alan Stern

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Alan Stern wrote:
> On Mon, 7 Nov 2005, thomas schorpp wrote:
> 
> 
>>>I don't think blacklisting is a good way to do this.  In principle any USB 
>>>mass storage device -- any SCSI device, in fact -- might have a 
>>>vendor-specific pass-thru needing special handling.
>>
>>no. that would be not industry best practice and uneconomical sw effort.
> 
> 
> Adding vendor-specific codes to SCSI devices isn't industry best practice?  
> I can't argue with that, but it is very common nevertheless.

not for all the cheap noname consumer stuff around. maybe in your scsi-320 server 
world i cant afford.

> 
> Is it an uneconomical software effort?  Ridiculous -- it's not software at 
> all, it's part of the firmware.

i thought were speaking of OS drivers here?

> 
> 
>>remember this devices are mainly "designed for windows" and WHQL certification 
>>is expensive for every single driver. and ms has only one driver for all usb storage 
>>right now, all boxes and sticks i had here use it.
> 
> 
> I think you have a very limited view of how devices are "designed".  
> Besides, I specifically said I was talking about _all_ SCSI devices, not 
> just USB ones.
> 

yes, for the second sentence, this is made clear now.

> 
>>> It doesn't have to be 
>>>correlated with the vendor, the product, the SCSI level, the transport, or 
>>>anything else.
>>
>>yes. the sheet for design recommended cypress chips state all ATACB and ATA-Security.
>>i would be make no sense to implement different behaviuors in chips for the same purpose.
> 
> 
> That's what Cypress does, sure.  But who says that other vendors have to 
> copy Cypress?  For the most part they don't.  Neither does T10.

they dont copy cypress, they want to reduce costs, processes and resulting 
"standards" should be compatible, therefore.

> 
> As for whether it makes no sense -- you have your point of view.  Business 
> people have a very different point of view; to them it might make 
> excellent sense, to help secure a competitive advantage.

:D i'm no business man. i am a generalist and i just want to lock my external hdd.

> 
> Alan Stern
> 
> -

tom schorpp

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Christoph Hellwig]

On Mon, Nov 07, 2005 at 09:24:21AM -0800, Patrick Mansfield wrote:
> The scmd/cdb is not available until we call scsi_get_command() in the
> request function. So you would have to add a field into scsi_request, set
> it in sg (in both sg.c block/scsi_ioctl.c, test via sd and sg when you are
> done), and add the field in scsi_cmnd, and set scmd one in
> scsi_init_cmd_from_req().

Once Mike finally reposts his patchkit scsi_request should be gone
and block SG_IO or sg should send down a struct request.  Which already
has a cmnd pointer embedded.

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Patrick Mansfield wrote:
> On Mon, Nov 07, 2005 at 09:05:25AM -0800, Matthew Dharm wrote:
> 
> 
>>Okay, what am I missing?
>>
>>Looking at that patch, it looks to me like if sg.c set a flag in the SCSI
>>command block then scsi_core.c could use that flag in the test to determine
>>if the LUN should be masked-in.
>>
>>HOWEVER, I keep hearing that the changes will be extensive.  What am I
>>missing?
> 
> 
> Not extensive AFAICT ... 
> 
> The scmd/cdb is not available until we call scsi_get_command() in the
> request function. So you would have to add a field into scsi_request, set
> it in sg (in both sg.c block/scsi_ioctl.c, test via sd and sg when you are
> done), and add the field in scsi_cmnd, and set scmd one in
> scsi_init_cmd_from_req().

i'll wait for maintainers decisions before touching code.

> 
> But as far as black listing, it does seem like a better solution in that
> user apps do not need special code.
> 
> Do the devices that require it (well so far) have useable vendor + model
> strings or usb id's?
> 
> -- Patrick Mansfield
> 
> 

most no name boxes use brigde manu id's although cypress claims on web 
this was not allowed (here the older A-brigde chip, model go-usb2):

Bus 004 Device 003: ID 04b4:6830 Cypress Semiconductor Corp. USB-2.0 IDE Adapter
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x04b4 Cypress Semiconductor Corp.
  idProduct          0x6830 USB-2.0 IDE Adapter
  bcdDevice            0.01
  iManufacturer          56 Cypress Semiconductor
  iProduct               78 USB2.0 Storage Device
  iSerial               100 DEF1092E5AC4
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           32
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0xc0
      Self Powered
    MaxPower                0mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         8 Mass Storage
      bInterfaceSubClass      6 SCSI
      bInterfaceProtocol     80 Bulk (Zip)
      iInterface              0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x88  EP 8 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  bNumConfigurations      1
tom1:~# 

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Patrick Mansfield]

On Mon, Nov 07, 2005 at 06:54:58PM +0100, thomas schorpp wrote:
> Bus 004 Device 003: ID 04b4:6830 Cypress Semiconductor Corp. USB-2.0 IDE Adapter
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0 (Defined at Interface level)
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0        64
>   idVendor           0x04b4 Cypress Semiconductor Corp.
>   idProduct          0x6830 USB-2.0 IDE Adapter

So this is a USB to ATA/ATAPI bridge?

Can we trigger black listing based on the above vendor/product values?
i.e. can you check for these values in usb-storage slave_configure code?

I am ATA ignorant ... how does this thing pass multi-lun values to the ATA
side? Is it just effectively not allowed in ATA?

Per earlier email, datasheet is here:

http://www.cypress.com/portal/server.pt?space=CommunityPage&control=SetCommunity&CommunityID=209&PageID=259&fid=14&rpn=CY7C68320

On page 13 of datasheet:

Sounds like this device has special snooping of cdb[1], so if it sees 0x24
in there it looks for an embedded ATA command, and translates and sends
that to the underlying device, so we can never black list based on the
device that we are using to send the SG_IO ioctl.

Yeh, some other hardware might want lun inihibit in the future, and that
could still be added back, but IMHO black list is better.

-- Patrick Mansfield

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Mon, 7 Nov 2005, Patrick Mansfield wrote:

> On Mon, Nov 07, 2005 at 06:54:58PM +0100, thomas schorpp wrote:
> > Bus 004 Device 003: ID 04b4:6830 Cypress Semiconductor Corp. USB-2.0 IDE Adapter
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               2.00
> >   bDeviceClass            0 (Defined at Interface level)
> >   bDeviceSubClass         0
> >   bDeviceProtocol         0
> >   bMaxPacketSize0        64
> >   idVendor           0x04b4 Cypress Semiconductor Corp.
> >   idProduct          0x6830 USB-2.0 IDE Adapter
> 
> So this is a USB to ATA/ATAPI bridge?
> 
> Can we trigger black listing based on the above vendor/product values?
> i.e. can you check for these values in usb-storage slave_configure code?

Yes.  The patch below illustrates how.  You'll have to change the name of 
the sdev flag to something sensible...

I firmly believe this is the wrong approach, however.  It's a specific 
solution to a general problem.  I would much prefer to add a new flag to 
struct request.

> Yeh, some other hardware might want lun inihibit in the future, and that
> could still be added back, but IMHO black list is better.

I'd say it's inevitable.  And it may even end up being standard-based, not 
vendor/product based.

Alan Stern



Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Index: usb-2.6/drivers/usb/storage/scsiglue.c
===================================================================
--- usb-2.6.orig/drivers/usb/storage/scsiglue.c
+++ usb-2.6/drivers/usb/storage/scsiglue.c
@@ -185,6 +185,13 @@ static int slave_configure(struct scsi_d
 	if (us->flags & US_FL_NOT_LOCKABLE)
 		sdev->lockable = 0;
 
+	/* SCSI-ATA pass-thru whitelist for Cypress chip */
+	if (le16_to_cpu(us->pusb_dev->descriptor.idVendor) == 
+				USB_VENDOR_ID_CYPRESS &&
+			le16_to_cpu(us->pusb_dev->descriptor.idProduct) ==
+				USB_PRODUCT_ID_CYPRESS_CY7C68320)
+		sdev->ata_passthru = 1;
+
 	/* this is to satisfy the compiler, tho I don't think the 
 	 * return code is ever checked anywhere. */
 	return 0;
Index: usb-2.6/drivers/usb/storage/usb.h
===================================================================
--- usb-2.6.orig/drivers/usb/storage/usb.h
+++ usb-2.6/drivers/usb/storage/usb.h
@@ -169,7 +169,10 @@ extern void fill_inquiry_response(struct
 #define scsi_lock(host)		spin_lock_irq(host->host_lock)
 
 
-/* Vendor ID list for devices that require special handling */
+/* Vendor/Product ID list for devices that require special handling */
 #define USB_VENDOR_ID_GENESYS		0x05e3	/* Genesys Logic */
 
+#define USB_VENDOR_ID_CYPRESS		0x04b4	/* Cypress Semiconductor */
+#define USB_PRODUCT_ID_CYPRESS_CY7C68320	0x6830
+
 #endif

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Patrick Mansfield]

On Mon, Nov 07, 2005 at 02:53:34PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2005, Patrick Mansfield wrote:
> > Can we trigger black listing based on the above vendor/product values?
> > i.e. can you check for these values in usb-storage slave_configure code?
> 
> Yes.  The patch below illustrates how.  You'll have to change the name of 
> the sdev flag to something sensible...
> 
> I firmly believe this is the wrong approach, however.  It's a specific 
> solution to a general problem.  I would much prefer to add a new flag to 
> struct request.

We have the specific problem of a device being reported as scsi-2
compliant when it is not.

Stepping back a bit ...

usb-storage should pass through the scsi level, we should not require
special handling (adding back inhibit lun support or a new black list
option) for compliant hardware.

So, do we have a usb-storage problem or non-compliant hardware here? I
did not find the answer in previous emails.

That is, is usb-storage forcing scsi-2 when the device tells us it is
scsi-3 compliant, or is the hardware reporting devices are scsi-2, yet
requiring non-LUN value in cdb[1]?

A method that can reliably and easily set some black list value for the
hardware in question will to lead to fewer questions in the future as to
why an SG_IO command is getting LUN values set in cdb[1].

If this is a hardware problem I'm not opposed to adding back support for
the ioctl and AFAIUI a new struct request flag.

-- Patrick Mansfield

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Mon, 7 Nov 2005, Patrick Mansfield wrote:

> We have the specific problem of a device being reported as scsi-2
> compliant when it is not.
> 
> Stepping back a bit ...
> 
> usb-storage should pass through the scsi level, we should not require
> special handling (adding back inhibit lun support or a new black list
> option) for compliant hardware.
> 
> So, do we have a usb-storage problem or non-compliant hardware here? I
> did not find the answer in previous emails.
> 
> That is, is usb-storage forcing scsi-2 when the device tells us it is
> scsi-3 compliant, or is the hardware reporting devices are scsi-2, yet
> requiring non-LUN value in cdb[1]?

I think we may have both.  However I don't know how this Cypress chip
reports itself.  A system log showing the INQUIRY data would be very
helpful.

It's quite possible that usb-storage no longer needs to force the 
scsi-level to 2.  No one has recently tested what would happen without 
it.  Matt probably has the best selection of devices for testing...

There is one problem we have with devices that report themselves as SCSI-3 
or SCSI-4 but hang when they receive a REPORT LUNS command.  That's easily 
handled by making usb-storage set the NOREPORTLUN flag.  Maybe that's all 
we need to do.

Alan Stern

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Mon, 2005-11-07 at 15:59 -0500, Alan Stern wrote:
> > That is, is usb-storage forcing scsi-2 when the device tells us it is
> > scsi-3 compliant, or is the hardware reporting devices are scsi-2, yet
> > requiring non-LUN value in cdb[1]?
> 
> I think we may have both.  However I don't know how this Cypress chip
> reports itself.  A system log showing the INQUIRY data would be very
> helpful.

We were told in prior emails that it actually reports a level of zero
(i.e. no compliance with any SCSI standard).  My original proposal was
just not to modify the CDB[1] for this case if we could get the INQUIRY
passed through unmangled.

> It's quite possible that usb-storage no longer needs to force the 
> scsi-level to 2.  No one has recently tested what would happen without 
> it.  Matt probably has the best selection of devices for testing...
> 
> There is one problem we have with devices that report themselves as SCSI-3 
> or SCSI-4 but hang when they receive a REPORT LUNS command.  That's easily 
> handled by making usb-storage set the NOREPORTLUN flag.  Maybe that's all 
> we need to do.

If you could try this out, I'd be grateful.

Thanks,

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Mon, 7 Nov 2005, James Bottomley wrote:

> > It's quite possible that usb-storage no longer needs to force the 
> > scsi-level to 2.  No one has recently tested what would happen without 
> > it.  Matt probably has the best selection of devices for testing...
> > 
> > There is one problem we have with devices that report themselves as SCSI-3 
> > or SCSI-4 but hang when they receive a REPORT LUNS command.  That's easily 
> > handled by making usb-storage set the NOREPORTLUN flag.  Maybe that's all 
> > we need to do.
> 
> If you could try this out, I'd be grateful.

I tried the patch below on several combinations of SCSI levels and device 
types.  It seems to do exactly what we want; for reads I always saw 
READ(10) and for MODE SENSE I saw either the 6- or 10-byte form, depending 
on the device type and USB subclass value.

The question remains whether pass-thru will now work and whether the patch
will mess up some existing non-compliant device that currently works okay.  
The only way to know is by trying it out.  Matt and Timothy, that's up to
you.

Alan Stern



Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Index: usb-2.6/drivers/usb/storage/scsiglue.c
===================================================================
--- usb-2.6.orig/drivers/usb/storage/scsiglue.c
+++ usb-2.6/drivers/usb/storage/scsiglue.c
@@ -94,23 +94,6 @@ static int slave_configure(struct scsi_d
 	 * the end, scatter-gather buffers follow page boundaries. */
 	blk_queue_dma_alignment(sdev->request_queue, (512 - 1));
 
-	/* Set the SCSI level to at least 2.  We'll leave it at 3 if that's
-	 * what is originally reported.  We need this to avoid confusing
-	 * the SCSI layer with devices that report 0 or 1, but need 10-byte
-	 * commands (ala ATAPI devices behind certain bridges, or devices
-	 * which simply have broken INQUIRY data).
-	 *
-	 * NOTE: This means /dev/sg programs (ala cdrecord) will get the
-	 * actual information.  This seems to be the preference for
-	 * programs like that.
-	 *
-	 * NOTE: This also means that /proc/scsi/scsi and sysfs may report
-	 * the actual value or the modified one, depending on where the
-	 * data comes from.
-	 */
-	if (sdev->scsi_level < SCSI_2)
-		sdev->scsi_level = SCSI_2;
-
 	/* According to the technical support people at Genesys Logic,
 	 * devices using their chips have problems transferring more than
 	 * 32 KB at a time.  In practice people have found that 64 KB
@@ -159,10 +142,10 @@ static int slave_configure(struct scsi_d
 		/* 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
-		 * a Get-Max-LUN request, we won't lose much by setting the
-		 * revision level down to 2.  The only devices that would be
+		 * a Get-Max-LUN request, we won't lose much by not using
+		 * REPORT LUNS.  The only devices that would be
 		 * affected are those with sparse LUNs. */
-		sdev->scsi_level = SCSI_2;
+		sdev->sdev_target->no_report_lun = 1;
 
 		/* USB-IDE bridges tend to report SK = 0x04 (Non-recoverable
 		 * Hardware Error) when any low-level error occurs,
Index: usb-2.6/include/scsi/scsi_device.h
===================================================================
--- usb-2.6.orig/include/scsi/scsi_device.h
+++ usb-2.6/include/scsi/scsi_device.h
@@ -161,7 +161,8 @@ struct scsi_target {
 	unsigned int		channel;
 	unsigned int		id; /* target id ... replace
 				     * scsi_device.id eventually */
-	unsigned long		create:1; /* signal that it needs to be added */
+	unsigned int		create:1; /* signal that it needs to be added */
+	unsigned int		no_report_lun:1; /* do not use REPORT LUNS */
 	char			scsi_level;
 	void 			*hostdata; /* available to low-level driver */
 	unsigned long		starget_data[0]; /* for the transport */
Index: usb-2.6/drivers/scsi/scsi_scan.c
===================================================================
--- usb-2.6.orig/drivers/scsi/scsi_scan.c
+++ usb-2.6/drivers/scsi/scsi_scan.c
@@ -1078,6 +1078,7 @@ static int scsi_report_lun_scan(struct s
 	 * support more than 8 LUNs.
 	 */
 	if ((bflags & BLIST_NOREPORTLUN) || 
+	     starget->no_report_lun ||
 	     starget->scsi_level < SCSI_2 ||
 	    (starget->scsi_level < SCSI_3 && 
 	     (!(bflags & BLIST_REPORTLUN2) || shost->max_lun <= 8)) )

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Mike Anderson]

Alan Stern <stern@rowland.harvard.edu> wrote:
> ===================================================================
> --- usb-2.6.orig/include/scsi/scsi_device.h
> +++ usb-2.6/include/scsi/scsi_device.h
> @@ -161,7 +161,8 @@ struct scsi_target {
>  	unsigned int		channel;
>  	unsigned int		id; /* target id ... replace
>  				     * scsi_device.id eventually */
> -	unsigned long		create:1; /* signal that it needs to be added */
> +	unsigned int		create:1; /* signal that it needs to be added */
> +	unsigned int		no_report_lun:1; /* do not use REPORT LUNS */
>  	char			scsi_level;
>  	void 			*hostdata; /* available to low-level driver */
>  	unsigned long		starget_data[0]; /* for the transport */

Alan,

Instead of adding a new bit field to scsi_target could we utilize bflags
in the scsi_target. Currently we have sdev_bflags in the scsi_device which
we are not utilizing. We could remove sdev_bflags and add a bflags to
scsi_target

In the usb slave configure we could "or" in BLIST_NOREPORTLUN.

Then in the function scsi_get_device_flags we could reference the
scsi_target bflags instead of sdev_bflags.

The bflags check in scsi_scan.c for BLIST_NOREPORTLUN could stay the same
and we would not have two flags for a similar no report luns check.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Alan Stern]

On Tue, 8 Nov 2005, Mike Anderson wrote:

> Alan,
> 
> Instead of adding a new bit field to scsi_target could we utilize bflags
> in the scsi_target. Currently we have sdev_bflags in the scsi_device which
> we are not utilizing. We could remove sdev_bflags and add a bflags to
> scsi_target
> 
> In the usb slave configure we could "or" in BLIST_NOREPORTLUN.
> 
> Then in the function scsi_get_device_flags we could reference the
> scsi_target bflags instead of sdev_bflags.
> 
> The bflags check in scsi_scan.c for BLIST_NOREPORTLUN could stay the same
> and we would not have two flags for a similar no report luns check.

That's a good idea.  Except that we should keep sdev_bflags while adding 
target_bflags.

I see there's a comment in scsi_probe_lun:

                /*
                 * Get any flags for this device.
                 *
                 * XXX add a bflags to Scsi_Device, and replace the
                 * corresponding bit fields in Scsi_Device, so bflags
                 * need not be passed as an argument.
                 */

Does that need to be updated?

There are several other flags in scsi_device that are duplicates of things
in bflags:

	lockable		BLIST_NOT_LOCKABLE
	use_10_for_ms		BLIST_USE_10_BYTE_MS
	use_192_bytes_for_3f	BLIST_MS_192_BYTES_FOR_3F
	skip_ms_page_8		BLIST_MS_SKIP_PAGE_08
	skip_ms_page_3f		BLIST_MS_SKIP_PAGE_3F
	no_start_on_add		BLIST_NOSTARTONADD
	retry_hwerr		BLIST_RETRY_HWERROR

Maybe I've missed some.  Then there are flags which aren't used any more,
like BLIST_INQUIRY_36 (although for backward compatibility we should try
to keep them).  Note also that some of these flags apply to individual
LUNs while others apply to entire targets.

There's a good reason for storing these flags in the device or target.  
If they're available only in the blacklist database then drivers can't 
adjust them for specific devices.  Ideally, once that structures have been 
set up and the blacklist flags copied into them, there will be no need to 
refer back to the original database entry.

The whole situation is thoroughly disorganized.  If you would like to 
rearrange everything, you've got my support.

Alan Stern

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Sun, 2005-11-06 at 13:58 -0800, Matthew Dharm wrote:
> I've been recommending this flag to people as recently as 6 months ago
> (i.e. the last time someone asked me).   It comes up 2-3 times a year.
> 
> The last time it came up, the device set SCSI_2.

OK ... if you send a patch to add it back  I'll look it over ... but it
needs to be clean;  I'm betting it needs a huge amount of changes that
would thread through both SCSI and block (so Jens will need to ack it
too).

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Patrick Mansfield]

On Sat, Nov 05, 2005 at 03:55:22PM -0800, Matthew Dharm wrote:

> I just realized this plan has a problem...
> 
> The reported SCSI level of a device is mostly garbage, but not always.
> I've seen 0, 1, 2, 3, and 0xff all reported.  HOWEVER, the reported value
> seems independent of what devices have vendor-specific commands (and thus
> need the CDB[1] not messed with).

But it is only a problem for devices that require us to set values in
cdb[1] that conflict with the scsi spec, not for all devices that have
vendor specific commands.

Do the devices in question that don't want the LUN in cdb[1] report as
scsi-3 or later? If so we can still pass through the scsi level and it
would work fine. Yeh, if we later hit scsi-2 devices that want it we
have a problem.

-- Patrick Mansfield

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Andries Brouwer]

On Fri, Nov 04, 2005 at 12:30:04PM -0800, Matthew Dharm wrote:
> On Fri, Nov 04, 2005 at 12:58:27PM -0600, James Bottomley wrote:

> > What happens if you prevent USB mangling the scsi_level?  I think, for
> > the most part, we would handle 0 in about the same way as we handle 2.
> > However, we could gate the if around the CDB[1] mangling as
> > 
> > if (scsi_level != SCSI_UNKNOWN && scsi_level <= SCSI_2)
> > 
> > which should fix your problem, I think.
> 
> A long time ago, usb-storage didn't mangle the scsi_level.  And almost
> nothing worked.  Without a SCSI level, 6-byte commands get sent; only
> 10-byte commands will do.

Yes, that was long ago. Haven't things improved?

(For example, I seem to recall adding stuff like use_10_for_{ms,rw}
to make it possible for usb storage to tell the SCSI layer that it
prefers 10-byte commands. Maybe no cheating with SCSI level is needed
anymore?)

Andries

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Pete Zaitcev]

On Fri, 4 Nov 2005 10:30:44 -0800, "Timothy Thelin" <Timothy.Thelin@wdc.com> wrote:

> The problem then is with vendor specific commands as SCSI core
> mangles those as well.  The vendor specific command I had issues
> with is the "ATACB" ATA passthru cdb that cypress boards understand.
> CDB[1] is a signature byte that must be 0x24.  Needless to say,
> that leading 2 gets masked off to a 0, and the drive aborts the
> ATACB.  Now cypress could have been smarter about designing their
> cdb, but they wern't, and so there really needs to be a way to tell
> SCSI core "hands off the cdb".

If this issue is real, you can use ub with that device for now.

-- Pete

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[thomas schorpp]

Pete Zaitcev wrote:
> On Fri, 4 Nov 2005 10:30:44 -0800, "Timothy Thelin" <Timothy.Thelin@wdc.com> wrote:
> 
> 
>>The problem then is with vendor specific commands as SCSI core
>>mangles those as well.  The vendor specific command I had issues
>>with is the "ATACB" ATA passthru cdb that cypress boards understand.
>>CDB[1] is a signature byte that must be 0x24.  Needless to say,
>>that leading 2 gets masked off to a 0, and the drive aborts the
>>ATACB.  Now cypress could have been smarter about designing their
>>cdb, but they wern't, and so there really needs to be a way to tell
>>SCSI core "hands off the cdb".
> 
> 
> If this issue is real, you can use ub with that device for now.
> 
> -- Pete
> -

hello Pete

nice idea :)

unfortunately it doesnt work with latest smartctl cvs on my usb2 drive, no smart functionality
tom1:/usr/src/sm5# ./smartctl -a -d scsi /dev/uba
smartctl version 5.34 [i686-pc-linux-gnu] Copyright (C) 2002-5 Bruce Allen
Home page is http://smartmontools.sourceforge.net/

Device: IC35L030 AVVN07-0         Version:
Device type: disk
Local Time is: Sat Nov  5 16:49:28 2005 CET
Device does not support SMART

Error Counter logging not supported
Device does not support Self Test logging
tom1:/usr/src/sm5#

hdparm cant work(?). not implemented.

tom1:/usr/src/sm5# sdparm -alv /dev/uba
    /dev/uba: IC35L030  AVVN07-0
Read write error recovery [PS=0] mode page:
  AWRE        0  [cha: n, def:  0, sav:  0]  Automatic write reallocation enabled
  ARRE        0  [cha: n, def:  0, sav:  0]  Automatic read reallocation enabled
  TB          0  [cha: n, def:  0, sav:  0]  Transfer block
  RC          0  [cha: n, def:  0, sav:  0]  Read continuous
  EER         0  [cha: n, def:  0, sav:  0]  Enable early recover
  PER         0  [cha: n, def:  0, sav:  0]  Post error
  DTE         0  [cha: n, def:  0, sav:  0]  Data terminate on error
  DCR         0  [cha: n, def:  0, sav:  0]  Disable correction
  RRC         0  [cha: n, def:  0, sav:  0]  Read retry count
  WRC         0  [cha: n, def:  0, sav:  0]  Write retry count
  RTL         0  [cha: n, def:  0, sav:  0]  Recovery time limit (ms)
>> Format (SBC) mode page not supported
>> Rigid disk (SBC) mode page not supported
>> Verify error recovery (SBC) mode page not supported
Caching (SBC) [PS=0] mode page:
  IC          0  [cha: n, def:  0, sav:  0]  Initiator control
  ABPF        0  [cha: n, def:  0, sav:  0]  Abort pre-fetch
  CAP         0  [cha: n, def:  0, sav:  0]  Caching analysis permitted
  DISC        0  [cha: n, def:  0, sav:  0]  Discontinuity
  SIZE        0  [cha: n, def:  0, sav:  0]  Size (1->CSS valid, 0->NCS valid)
  WCE         0  [cha: n, def:  0, sav:  0]  Write cache enable
  MF          0  [cha: n, def:  0, sav:  0]  Multiplication factor
  RCD         0  [cha: n, def:  0, sav:  0]  Read cache disable
  DRRP        0  [cha: n, def:  0, sav:  0]  Demand read retension priority
  WRP         0  [cha: n, def:  0, sav:  0]  Write retension priority
  DPTL        0  [cha: n, def:  0, sav:  0]  Disable pre-fetch transfer length
  MIPF        0  [cha: n, def:  0, sav:  0]  Minimum pre-fetch
  MAPF        0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch
  MAPFC       0  [cha: n, def:  0, sav:  0]  Maximum pre-fetch ceiling
 >> skipping rest as field position exceeds mode page length=12
>> Control mode page not supported
>> XOR control (SBC) mode page not supported
>> Protocol specific logical unit mode page not supported
>> Protocol specific port mode page not supported
>> Power condition mode page not supported
>> Informational exceptions control mode page not supported
>> Background control (SBC) mode subpage not supported
tom1:/usr/src/sm5#

device is in locked state, ata-security.

hmm, could someone supply me a sdparm commandline and data to test unlock (if supported), pls ?
im not through with all the specs from T10.org yet and they want money for more docs.

thx,
tom

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Wed, Nov 02, 2005 at 02:18:52PM -0800, Timothy Thelin wrote:
> 
> If you had time to spare, instead of touching usb-storage,
> it might be better spent resurecting SG_FLAG_LUN_INHIBIT to
> stop the above behavior so that SG_IO cdbs can be passed
> through untouched.
> (SG_FLAG_FUN_INHIBIT was a flag SG_IO used to support a long
> time ago, and I have no idea why it was dropped, but it was)

I didn't realize that had been removed.  Anyone that sends a
vendor-specific command to a device needs this flag to make sure it goes
through unmangled.

Perhaps someone on linux-scsi can comment on why this was removed and how
we might get it back?

Matt

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

We can customize our colonels.
					-- Tux
User Friendly, 12/1/1998

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Wed, 2005-11-02 at 15:45 -0800, Matthew Dharm wrote:
> On Wed, Nov 02, 2005 at 02:18:52PM -0800, Timothy Thelin wrote:
> > 
> > If you had time to spare, instead of touching usb-storage,
> > it might be better spent resurecting SG_FLAG_LUN_INHIBIT to
> > stop the above behavior so that SG_IO cdbs can be passed
> > through untouched.
> > (SG_FLAG_FUN_INHIBIT was a flag SG_IO used to support a long
> > time ago, and I have no idea why it was dropped, but it was)
> 
> I didn't realize that had been removed.  Anyone that sends a
> vendor-specific command to a device needs this flag to make sure it goes
> through unmangled.
> 
> Perhaps someone on linux-scsi can comment on why this was removed and how
> we might get it back?

I've no distinct recollection of someone removing this, but if I
remember correctly what it used to do, it was a hack to stop us from
mangling SCSI-3 CDB's.  We fixed the mid-layer not to require the hack
by only setting the CDB[1] lun field for SCSI-1 and SCSI-2 devices (as
the standards mandate).  What's the actual problem?  No SCSI-1 or SCSI-2
device should have any vendor specific CDBs that uses these bits in
CDB[1].

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Thu, Nov 03, 2005 at 11:08:07PM -0500, James Bottomley wrote:
> On Wed, 2005-11-02 at 15:45 -0800, Matthew Dharm wrote:
> > On Wed, Nov 02, 2005 at 02:18:52PM -0800, Timothy Thelin wrote:
> > > 
> > > If you had time to spare, instead of touching usb-storage,
> > > it might be better spent resurecting SG_FLAG_LUN_INHIBIT to
> > > stop the above behavior so that SG_IO cdbs can be passed
> > > through untouched.
> > > (SG_FLAG_FUN_INHIBIT was a flag SG_IO used to support a long
> > > time ago, and I have no idea why it was dropped, but it was)
> > 
> > I didn't realize that had been removed.  Anyone that sends a
> > vendor-specific command to a device needs this flag to make sure it goes
> > through unmangled.
> > 
> > Perhaps someone on linux-scsi can comment on why this was removed and how
> > we might get it back?
> 
> I've no distinct recollection of someone removing this, but if I
> remember correctly what it used to do, it was a hack to stop us from
> mangling SCSI-3 CDB's.  We fixed the mid-layer not to require the hack
> by only setting the CDB[1] lun field for SCSI-1 and SCSI-2 devices (as
> the standards mandate).  What's the actual problem?  No SCSI-1 or SCSI-2
> device should have any vendor specific CDBs that uses these bits in
> CDB[1].

Unfortunately, reality appears to disagree with the last "should".  I've
personally seen devices with vendor-specific commands that want to control
CDB[1] in SCSI-2.

I didn't know it was removed; I only know what Timothy Thelin told me.  Can
we get the feature back?

Matt

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

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Fri, 2005-11-04 at 09:28 -0800, Matthew Dharm wrote:
> > I've no distinct recollection of someone removing this, but if I
> > remember correctly what it used to do, it was a hack to stop us from
> > mangling SCSI-3 CDB's.  We fixed the mid-layer not to require the hack
> > by only setting the CDB[1] lun field for SCSI-1 and SCSI-2 devices (as
> > the standards mandate).  What's the actual problem?  No SCSI-1 or SCSI-2
> > device should have any vendor specific CDBs that uses these bits in
> > CDB[1].
> 
> Unfortunately, reality appears to disagree with the last "should".  I've
> personally seen devices with vendor-specific commands that want to control
> CDB[1] in SCSI-2.

Which device and command?  It isn't some vendor who screwed up SCSI-3
compliance and then decided to "fix" it by only claiming SCSI-2, is it?

> I didn't know it was removed; I only know what Timothy Thelin told me.  Can
> we get the feature back?

Not at all easily ... changes to support it would have to thread through
several structures in both the block layer and the scsi subsystem.

James

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[Matthew Dharm]

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

On Fri, Nov 04, 2005 at 12:33:50PM -0600, James Bottomley wrote:
> On Fri, 2005-11-04 at 09:28 -0800, Matthew Dharm wrote:
> > > I've no distinct recollection of someone removing this, but if I
> > > remember correctly what it used to do, it was a hack to stop us from
> > > mangling SCSI-3 CDB's.  We fixed the mid-layer not to require the hack
> > > by only setting the CDB[1] lun field for SCSI-1 and SCSI-2 devices (as
> > > the standards mandate).  What's the actual problem?  No SCSI-1 or SCSI-2
> > > device should have any vendor specific CDBs that uses these bits in
> > > CDB[1].
> > 
> > Unfortunately, reality appears to disagree with the last "should".  I've
> > personally seen devices with vendor-specific commands that want to control
> > CDB[1] in SCSI-2.
> 
> > I didn't know it was removed; I only know what Timothy Thelin told me.  Can
> > we get the feature back?
> 
> Not at all easily ... changes to support it would have to thread through
> several structures in both the block layer and the scsi subsystem.

Would the block layer really have to change much if the commands only come
from SG_IO?

Matt

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

P:  How about "Web Designer"?
DP: I'd like a name that people won't laugh at.
					-- Pitr and Dust Puppy
User Friendly, 12/6/1997

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

Re: [usb-storage] [Merging ATA passthru] on integrating SMART/ATA-Security in usb-storage driver

[James Bottomley]

On Fri, 2005-11-04 at 12:30 -0800, Matthew Dharm wrote:
> Would the block layer really have to change much if the commands only come
> from SG_IO?

Yes, since the CDB[1] addition is done in scsi_dispatch_cmd() all you
really have access to there is the command (and through it the device
and host).  So a blacklist would be not too much work to implement but
actually making the SG_IO flag work would be quite large because it has
to be connected first to the request and then to the command.

James