⚙ D30919 Automagically apply quirks on USB storage devices

Can you explain these SCSI commands? 0x1b does not look like test_unit_ready to me?!

static uint8_t scsi_quirk_no_test_unit_ready[] = { 0x1b, 0, 0, 0, 0, 0 };
static uint8_t scsi_quirk_no_sync_cache[] = { 0x22, 0, 0, 0, 0, 0 };

Can you return ownership of this change? I have another revision of this change that doesn't add scsi_quirk_no_test_unit_ready[] and scsi_quirk_no_sync_cache[], and the option (in Phabricator) to attach an update to an existing Revision is no longer available.

In the selection menu in the bottom submit window "Add action ..." select "Commander revision" and it will be back to you!

vidwer_fbsdbugs_gmail.com commandeered this revision.Jan 17 2022, 2:59 PM

vidwer_fbsdbugs_gmail.com updated this revision to Diff 101535.

vidwer_fbsdbugs_gmail.com edited reviewers, added: • hselasky; removed: vidwer_fbsdbugs_gmail.com.

What's left to land this?

• hselasky added inline comments.Jan 21 2022, 8:16 AM

sys/dev/usb/usb_msctest.c
842	When err is zero, it means the command was successful, and then you apply the quirk? That will end up applying the TEST UNIT READY quirk for _all_ USB mass storage devices? Please explain the logic here. I don't get it! --HPS

@adrian : I see several issues with this patch.

One unrelated issue, is that the usb_quirk_str cannot be referred by usb.ko without a dependency on usb_quirk.ko . Else you get a linker issue.

For now, don't use the usb_quirk_str. Just stringify the enum.

vidwer_fbsdbugs_gmail.com updated this revision to Diff 101747.Jan 21 2022, 5:03 PM

vidwer_fbsdbugs_gmail.com updated this revision to Diff 101761.Jan 21 2022, 6:53 PM

@hselasky also, there should be a (c) line added to sys/dev/usb/quirk/usb_quirk.h, sys/dev/usb/quirk/usb_quirk.c and sys/dev/usb/usb_device.c I think.

Not needed for such small changes.

BTW: How many different memory stick vendors have you tested this new code with?

I'm worried that sending those commands may trigger a fault in faulty devices and make it unrecoverable by means of re-enumeration.

I recall having tested (can dig this up in sent e-mail) 8 devices in the past, apart from the ones that I own. Those last two are:
https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L227 https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L406

  idVendor = 0x0718 
  idProduct = 0x061a 
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=-1
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 34
ugen1.2: <TDKMedia Trans-It Drive> at usbus1
umass0 on uhub1
umass0: <TDKMedia Trans-It Drive, class 0/0, rev 2.00/1.00, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0xc100
umass0:2:0: Attached to scbus2
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <TDKMedia Trans-It Drive PMAP> Removable Direct Access SPC-2 SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <TDKMedia Trans-It Drive PMAP> Removable Direct Access SPC-2 SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 3688MB (7554240 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x0781 
  idProduct = 0x556b 
ugen1.2: <TDKMedia Trans-It Drive> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <TDKMedia Trans-It Drive PMAP>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <TDKMedia Trans-It Drive PMAP>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_test_unit_ready (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 27
ugen1.2: <SanDisk Cruzer Edge> at usbus1
umass0 on uhub1
umass0: <SanDisk Cruzer Edge, class 0/0, rev 2.00/1.27, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0x8101
umass0:2:0: Attached to scbus2
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <SanDisk Cruzer Edge 1.27> Removable Direct Access SPC-4 SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <SanDisk Cruzer Edge 1.27> Removable Direct Access SPC-4 SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 7485MB (15330304 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x058f 
  idProduct = 0x6387 
ugen1.2: <SanDisk Cruzer Edge> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <SanDisk Cruzer Edge 1.27>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <SanDisk Cruzer Edge 1.27>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
ugen1.2: <vendor 0x058f Flash Disk> at usbus1
umass0 on uhub1
umass0: <vendor 0x058f Flash Disk, class 0/0, rev 2.00/1.02, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0x4001
umass0: Get Max Lun not supported (USB_ERR_STALLED)
umass0:2:0: Attached to scbus2
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: < Flash Disk 8.07> Removable Direct Access SCSI-2 device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: < Flash Disk 8.07> Removable Direct Access SCSI-2 device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 1900MB (3891200 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
(da0:umass-sim0:0:0:0): PREVENT ALLOW MEDIUM REMOVAL not supported.
  idVendor = 0x1970 
  idProduct = 0x0000 
ugen1.2: <vendor 0x058f Flash Disk> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: < Flash Disk 8.07>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: < Flash Disk 8.07>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=-1
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 34
ugen1.2: <DaneElec Z Mate 8GB> at usbus1
umass0 on uhub1
umass0: <DaneElec Z Mate 8GB, class 0/0, rev 2.00/1.00, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0xc100
umass0:2:0: Attached to scbus2
(probe0:umass-sim0:0:0:0): Down reving Protocol Version from 2 to 0?
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <DaneElec Z Mate 8GB PMAP> Removable Direct Access SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <DaneElec Z Mate 8GB PMAP> Removable Direct Access SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 7872MB (16121856 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x0718 
  idProduct = 0x061a 
ugen1.2: <DaneElec Z Mate 8GB> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <DaneElec Z Mate 8GB PMAP>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <DaneElec Z Mate 8GB PMAP>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=-1
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 34
ugen1.2: <TDKMedia Trans-It Drive> at usbus1
umass0 on uhub1
umass0: <TDKMedia Trans-It Drive, class 0/0, rev 2.00/1.00, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0xc100
umass0:2:0: Attached to scbus2
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <TDKMedia Trans-It Drive PMAP> Removable Direct Access SPC-2 SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <TDKMedia Trans-It Drive PMAP> Removable Direct Access SPC-2 SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 3688MB (7554240 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x05e3 
  idProduct = 0x0727 
ugen1.2: <TDKMedia Trans-It Drive> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <TDKMedia Trans-It Drive PMAP>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <TDKMedia Trans-It Drive PMAP>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_test_unit_ready (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 27
ugen1.2: <vendor 0x05e3 product 0x0727> at usbus1
umass0 on uhub1
umass0: <vendor 0x05e3 product 0x0727, class 0/0, rev 2.00/2.72, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0x8101
umass0:2:0: Attached to scbus2
(probe0:umass-sim0:0:0:0): Down reving Protocol Version from 2 to 0?
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <  0272> Removable Direct Access SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <  0272> Removable Direct Access SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 14909MB (30535679 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x0951 
  idProduct = 0x1666 
ugen1.2: <vendor 0x05e3 product 0x0727> at usbus1 (disconnected)
umass0: at uhub1, port 4, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <  0272>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <  0272>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_test_unit_ready (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=0
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 27
ugen1.2: <Kingston DataTraveler 3.0> at usbus1
umass0 on uhub1
umass0: <Kingston DataTraveler 3.0, class 0/0, rev 3.10/1.10, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0x8101
umass0:2:0: Attached to scbus2
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <Kingston DataTraveler 3.0 PMAP> Removable Direct Access SPC-4 SCSI device
pass2: Serial Number xxx
pass2: 400.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <Kingston DataTraveler 3.0 PMAP> Removable Direct Access SPC-4 SCSI device
da0: Serial Number xxx
da0: 400.000MB/s transfers
da0: 14772MB (30253056 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>
  idVendor = 0x0951 
  idProduct = 0x168a 
ugen1.2: <Kingston DataTraveler 3.0> at usbus1 (disconnected)
umass0: at uhub1, port 2, addr 1 (disconnected)
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <Kingston DataTraveler 3.0 PMAP>  s/n xxx detached
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <Kingston DataTraveler 3.0 PMAP>  s/n xxx detached
(pass2:umass-sim0:0:0:0): Periph destroyed
(da0:umass-sim0:0:0:0): Periph destroyed
umass0: detached
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() | sending bytes for scsi_quirk_no_sync_cache (DIR_IN)
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() err=-1
@ sys/dev/usb/usb_msctest.c:usb_msc_auto_quirk() will call usbd_add_dynamic_quirk() with quirk 34
ugen1.2: <Kingston DT Micro> at usbus1
umass0 on uhub1
umass0: <Kingston DT Micro, class 0/0, rev 2.00/1.00, addr 1> on usbus1
umass0:  SCSI over Bulk-Only; quirks = 0xc100
umass0:2:0: Attached to scbus2
(probe0:umass-sim0:0:0:0): Down reving Protocol Version from 2 to 0?
GEOM: new disk da0
pass2 at umass-sim0 bus 0 scbus2 target 0 lun 0
pass2: <Kingston DT Micro PMAP> Removable Direct Access SCSI device
pass2: Serial Number xxx
pass2: 40.000MB/s transfers
da0 at umass-sim0 bus 0 scbus2 target 0 lun 0
da0: <Kingston DT Micro PMAP> Removable Direct Access SCSI device
da0: Serial Number xxx
da0: 40.000MB/s transfers
da0: 14996MB (30712320 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>
da0: Delete methods: <NONE(*),ZERO>

• hselasky added inline comments.Jan 23 2022, 9:47 PM

sys/dev/usb/quirk/usb_quirk.h
122 ↗	(On Diff #101761)	These chunks are no longer needed? And should be removed?

Rebased, removed leftover lines (extern ...).

vidwer_fbsdbugs_gmail.com marked an inline comment as done.Jan 23 2022, 10:30 PM

There is a couple of other places where zero-length SCSI commands are used, which should be DIR_NONE . Can you fix?

sys/dev/usb/usb_msctest.c
762	You should use a pointer to uaa, and not copy the structure itself.

Please upload full patch context!

In D30919#769780, @hselasky wrote:

There is a couple of other places where zero-length SCSI commands are used, which should be DIR_NONE . Can you fix?

I'm not sure I follow. What do you mean with 'zero-length SCSI commands'?

vidwer_fbsdbugs_gmail.com updated this revision to Diff 101960.Jan 26 2022, 8:54 PM

vidwer_fbsdbugs_gmail.com edited the summary of this revision. (Show Details)Feb 9 2022, 1:52 AM

vidwer_fbsdbugs_gmail.com edited the test plan for this revision. (Show Details)

vidwer_fbsdbugs_gmail.com added a reviewer: avg.

vidwer_fbsdbugs_gmail.com set the repository for this revision to rG FreeBSD src repository.

vidwer_fbsdbugs_gmail.com added a subscriber: Contributor Reviews (src).Feb 9 2022, 2:06 AM

• hselasky added inline comments.Feb 9 2022, 11:55 AM

sys/dev/usb/usb_msctest.c
884	You should also handle timeouts here, not only CSW_FAILED.

@hselasky there seems to exist a trick for outside contributors (those without a commit bit) that I am unaware of, to attach Harbormaster. See https://reviews.freebsd.org/D34220

sys/dev/usb/usb_msctest.c
884	Handle timeouts (with USB_ERR_TIMEOUT) just in this place, or in both places?

Both places.

vidwer_fbsdbugs_gmail.com updated this revision to Diff 102587.Feb 9 2022, 8:31 PM

vidwer_fbsdbugs_gmail.com marked 5 inline comments as done.Feb 10 2022, 11:40 AM

I will try to get some testing done on Monday. Sorry this issue has been taking so long.

I'll defer to hps on the rest, but this looks decently well constructed in its latest form.

sys/dev/usb/usb_msctest.h
6	You may want to drop the All Rights Reserved, here and elsewhere

Test unit ready is already executed at line 811, why not add the quirk there if the command fails? I don't see why you are duplicating tests :-(

The extra printouts you add I agree about.

Let me commander this revision again with what I think you are trying to do.

Could you test this version of the patch with your devices? Does it do what you want?

Sorry this has been taking so long.

• hselasky removed reviewers: adrian, imp, emaste, allanjude, philip, bz, avg.Feb 17 2022, 4:57 PM

Herald added a subscriber: imp. · View Herald TranscriptFeb 17 2022, 4:57 PM

I removed a bunch of reviewers which are not active here.

• hselasky marked 4 inline comments as done.Feb 17 2022, 4:58 PM

adrian added a subscriber: adrian.Feb 17 2022, 4:58 PM

@hselasky I 'm looking at your last implementation at the moment.

vidwer_fbsdbugs_gmail.com added inline comments.Feb 18 2022, 11:31 PM

sys/dev/usb/usb_msctest.c
769	It should read DPRINTFN(), then it will compile.
769	The parameter text 'Applying ...' seems to get lost in both the if and else case. In other words: I don't see per-device quirk details in dmesg.

In D30919#776650, @hselasky wrote:

Could you test this version of the patch with your devices? Does it do what you want?

Except for the dmesg 'noise', as explained above, your implementation works too.

DPRINTF -> DPRINTFN

In D30919#776980, @vidwer_fbsdbugs_gmail.com wrote:

In D30919#776650, @hselasky wrote:

Could you test this version of the patch with your devices? Does it do what you want?

Except for the dmesg 'noise', as explained above, your implementation works too.

That sounds like a small step forward for this patch :-)

Can you show the current dmesg you get?

sys/dev/usb/usb_msctest.c
769	Did you add: options USB_DEBUG to your kernel configuration file?
949–950	Error means we are no longer able to communicate and need to reset the device.

• hselasky added inline comments.Feb 19 2022, 12:24 AM

sys/dev/usb/usb_msctest.c
769	And the code is compiled with: options USB_DEBUG ???

Ehm, the last two iterations only /appeared/ to work because I left in the already present quirk entries @ https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L190

My bad :(

sys/dev/usb/usb_msctest.c
769	USB_DEBUG is set/present in sys/amd64/conf/GENERIC

Can you show me the dmesg with only this patch applied and no quirks? What goes wrong? Does "usbconfig -d X.Y reset" fix the device again?

In D30919#777053, @hselasky wrote:

Can you show me the dmesg with only this patch applied and no quirks? What goes wrong? Does "usbconfig -d X.Y reset" fix the device again?

USB disk one, this one is borrowed and is in no place set up to apply quirks:
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_PREVENT_ALLOW for USB mass storage device SanDisk Extreme (0x0781:0x558b)
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_SYNC_CACHE for USB mass storage device SanDisk Extreme (0x0781:0x558b)
ugen1.2: <SanDisk Extreme> at usbus1
umass0 on uhub0
umass0: <SanDisk Extreme, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 3 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 2 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 1 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 0 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Error 5, Retries exhausted
/* running usbconfig -d x.y reset */
umass0: at uhub0, port 1, addr 1 (disconnected)
umass0: detached
umass0 on uhub0
umass0: <SanDisk Extreme, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: <SanDisk Extreme 1.00> Removable Direct Access SPC-4 SCSI device
da0: Serial Number <redacted>
da0: 400.000MB/s transfers
da0: 59840MB (122552320 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>

USB disk two, https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L406:
ugen1.2: <SanDisk Ultra> at usbus1
umass0 on uhub1
umass0: <SanDisk Ultra, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0x4001
umass0:1:0: Attached to scbus1
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: <SanDisk Ultra 1.00> Removable Direct Access SPC-4 SCSI device
da0: Serial Number <redacted>
da0: 400.000MB/s transfers
da0: 29328MB (60063744 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>

USB disk three, https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L227:
ugen1.2: <vendor 0x13fe USB DISK Pro> at usbus1
umass0 on uhub2
umass0: <vendor 0x13fe USB DISK Pro, class 0/0, rev 2.00/1.10, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0x4000
umass0:1:0: Attached to scbus1
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: < USB DISK Pro PMAP> Removable Direct Access SCSI device
da0: Serial Number <redacted>
da0: 40.000MB/s transfers
da0: 3934MB (8056832 512 byte sectors)
da0: quirks=0x3<NO_SYNC_CACHE,NO_6_BYTE>
da1 at umass-sim0 bus 0 scbus1 target 0 lun 1
da1: < USB DISK Pro PMAP> Removable Direct Access SCSI device
da1: Serial Number <redacted>
da1: 40.000MB/s transfers
da1: 1MB (2880 512 byte sectors)
da1: quirks=0x3<NO_SYNC_CACHE,NO_6_BYTE>

In D30919#777053, @hselasky wrote:

Can you show me the dmesg with only this patch applied and no quirks? What goes wrong? Does "usbconfig -d X.Y reset" fix the device again?

For completeness, here's the output after having removed present quirks.

USB disk one:
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_PREVENT_ALLOW for USB mass storage device SanDisk Extreme (0x0781:0x558b)
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_SYNC_CACHE for USB mass storage device SanDisk Extreme (0x0781:0x558b)
ugen1.2: <SanDisk Extreme> at usbus1
umass0 on uhub0
umass0: <SanDisk Extreme, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 3 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 2 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 1 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 0 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Error 5, Retries exhausted
/* running usbconfig -d x.y reset */
umass0: at uhub0, port 1, addr 1 (disconnected)
umass0: detached
umass0 on uhub0
umass0: <SanDisk Extreme, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: <SanDisk Extreme 1.00> Removable Direct Access SPC-4 SCSI device
da0: Serial Number <redacted>
da0: 400.000MB/s transfers
da0: 59840MB (122552320 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>

USB disk two, https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L406:
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_PREVENT_ALLOW for USB mass storage device SanDisk Ultra (0x0781:0x5581)
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_SYNC_CACHE for USB mass storage device SanDisk Ultra (0x0781:0x5581)
ugen1.2: <SanDisk Ultra> at usbus1
umass0 on uhub0
umass0: <SanDisk Ultra, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 3 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 2 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 1 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 0 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Error 5, Retries exhausted
/* running usbconfig -d x.y reset */
umass0: at uhub0, port 1, addr 1 (disconnected)
umass0: detached
umass0 on uhub0
umass0: <SanDisk Ultra, class 0/0, rev 3.00/1.00, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc100
umass0:1:0: Attached to scbus1
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: <SanDisk Ultra 1.00> Removable Direct Access SPC-4 SCSI device
da0: Serial Number <redacted>
da0: 400.000MB/s transfers
da0: 29328MB (60063744 512 byte sectors)
da0: quirks=0x2<NO_6_BYTE>

USB disk three, https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L227:
sb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_TEST_UNIT_READY for USB mass storage device vendor 0x13fe USB DISK Pro (0x13fe:0x1e20)
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_PREVENT_ALLOW for USB mass storage device vendor 0x13fe USB DISK Pro (0x13fe:0x1e20)
usb_msc_auto_quirk: Applying dynamic quirk UQ_MSC_NO_SYNC_CACHE for USB mass storage device vendor 0x13fe USB DISK Pro (0x13fe:0x1e20)
ugen1.2: <vendor 0x13fe USB DISK Pro> at usbus1
umass0 on uhub0
umass0: <vendor 0x13fe USB DISK Pro, class 0/0, rev 2.00/1.10, addr 1> on usbus1
umass0: SCSI over Bulk-Only; quirks = 0xc001
umass0:1:0: Attached to scbus1
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 3 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 2 more tries remain
(probe0:umass-sim0:0:0:0): INQUIRY. CDB: 12 00 00 00 24 00
(probe0:umass-sim0:0:0:0): CAM status: CCB request completed with an error
(probe0:umass-sim0:0:0:0): Retrying command, 1 more tries remain
da0 at umass-sim0 bus 0 scbus1 target 0 lun 0
da0: < USB DISK Pro PMAP> Removable Direct Access SCSI device
da0: Serial Number <redacted>
da0: 40.000MB/s transfers
da0: 3934MB (8056832 512 byte sectors)
da0: quirks=0x3<NO_SYNC_CACHE,NO_6_BYTE>
da1 at umass-sim0 bus 0 scbus1 target 0 lun 1
da1: < USB DISK Pro PMAP> Removable Direct Access SCSI device
da1: Serial Number <redacted>
da1: 40.000MB/s transfers
da1: 1MB (2880 512 byte sectors)
da1: quirks=0x3<NO_SYNC_CACHE,NO_6_BYTE>

I've now moved that "usbconfig -d xxx reset" into the kernel, so then all devices should work?

In D30919#777108, @hselasky wrote:

I've now moved that "usbconfig -d xxx reset" into the kernel, so then all devices should work?

Yes, now all devices (those three I could test with) work.

Can we now drop or trim the USB MSC quirk table? https://github.com/freebsd/freebsd-src/blob/main/sys/dev/usb/quirk/usb_quirk.c#L190

sys/dev/usb/usb_msctest.c
793	whitespace

OK, good, then I will upstream after a quick local test!

This revision is now accepted and ready to land.Feb 19 2022, 7:55 PM

Update: I tested with a bunch of USB sticks from Kingston and found issues. I will try to resolve those issues and get you an updated patch.

Sorry. This stuff is not easy!

In D30919#777117, @hselasky wrote:

Update: I tested with a bunch of USB sticks from Kingston and found issues. I will try to resolve those issues and get you an updated patch.

Sorry. This stuff is not easy!

What do you mean with 'issues on Kingston devices'?

All SCSI commands time out simply after the added re-enumerate step. I have an idea why.

vidwer_fbsdbugs_gmail.com added inline comments.Feb 20 2022, 10:32 PM

sys/dev/usb/usb_msctest.c
954–957	Is this line now obsolete?

• hselasky marked an inline comment as done.Feb 21 2022, 12:12 AM

• hselasky added inline comments.

sys/dev/usb/usb_msctest.c
954–957	No we still need this.

Using this updated patch all my Kingston USB sticks can be accessed.

Please test with your devices too.

This revision now requires review to proceed.Feb 21 2022, 12:14 AM

@vidwer_fbsdbugs_gmail.com : Can you test this patch again with all your USB mass storage devices? Thank you!

I can do more tests this Saturday.

This change is open for review for over six months now, I think we should attract more testers. Could you write an RFC/RFH to freebsd-usb?

When timeout happens, don't set the single LUN quirk.

• hselasky accepted this revision.Feb 22 2022, 12:44 PM

This revision is now accepted and ready to land.Feb 22 2022, 12:44 PM

Closed by commit rG7520b88860d7: usb(4): Automagically apply all quirks for USB mass storage devices. (authored by • hselasky). · Explain WhyFeb 24 2022, 9:30 AM

This revision was automatically updated to reflect the committed changes.

• hselasky added a commit: rG7520b88860d7: usb(4): Automagically apply all quirks for USB mass storage devices..

• hselasky added a commit: rGcf912859d228: usb(4): Automagically apply all quirks for USB mass storage devices..Mar 3 2022, 2:42 PM

@hselasky do you expect me to write something in RELNOTES?

I don't think you need that, unless there are major regressions.

Automagically apply quirks on USB storage devices
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 103190

sys/dev/usb/storage/umass.c

sys/dev/usb/usb_device.c

sys/dev/usb/usb_msctest.h

sys/dev/usb/usb_msctest.c

Automagically apply quirks on USB storage devicesClosedPublicActions