Date: Thu, 3 Mar 2022 14:41:48 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: cf912859d228 - stable/13 - usb(4): Automagically apply all quirks for USB mass storage devices. Message-ID: <202203031441.223Efmm3018346@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by hselasky: URL: https://cgit.FreeBSD.org/src/commit/?id=cf912859d228b41ef62b4233cc198a1a9e4ece31 commit cf912859d228b41ef62b4233cc198a1a9e4ece31 Author: Hans Petter Selasky <hselasky@FreeBSD.org> AuthorDate: 2022-02-21 08:24:28 +0000 Commit: Hans Petter Selasky <hselasky@FreeBSD.org> CommitDate: 2022-03-03 14:41:12 +0000 usb(4): Automagically apply all quirks for USB mass storage devices. Currently there are five quirks the USB stack tries to automagically detect: - UQ_MSC_NO_PREVENT_ALLOW - UQ_MSC_NO_SYNC_CACHE - UQ_MSC_NO_TEST_UNIT_READY - UQ_MSC_NO_GETMAXLUN - UQ_MSC_NO_START_STOP If any of the quirks above are set, no further quirks will be probed. If any of the USB mass storage tests fail, the USB device is re-enumerated as a last resort to clear any error states from the device. Then the USB stack will try to probe and attach the umass<N> device passing the detected quirks. While at it give more details in dmesg about what is going on. Tested by: several Submitted by: Idwer Vollering <vidwer_fbsdbugs@gmail.com> Differential Revision: https://reviews.freebsd.org/D30919 Sponsored by: NVIDIA Networking (cherry picked from commit 7520b88860d7a79432e12ffcc47056844518bb62) --- sys/dev/usb/storage/umass.c | 7 +++ sys/dev/usb/usb_device.c | 5 +- sys/dev/usb/usb_msctest.c | 133 +++++++++++++++++++++++++++----------------- sys/dev/usb/usb_msctest.h | 4 +- 4 files changed, 96 insertions(+), 53 deletions(-) diff --git a/sys/dev/usb/storage/umass.c b/sys/dev/usb/storage/umass.c index 8260226e5d12..b8b502494264 100644 --- a/sys/dev/usb/storage/umass.c +++ b/sys/dev/usb/storage/umass.c @@ -2294,6 +2294,13 @@ umass_cam_action(struct cam_sim *sim, union ccb *ccb) xpt_done(ccb); goto done; } + } else if (sc->sc_transfer.cmd_data[0] == START_STOP_UNIT) { + if (sc->sc_quirks & NO_START_STOP) { + ccb->csio.scsi_status = SCSI_STATUS_OK; + ccb->ccb_h.status = CAM_REQ_CMP; + xpt_done(ccb); + goto done; + } } umass_command_start(sc, dir, ccb->csio.data_ptr, ccb->csio.dxfer_len, diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c index 322e4f5401ba..8d0e7961f675 100644 --- a/sys/dev/usb/usb_device.c +++ b/sys/dev/usb/usb_device.c @@ -2047,13 +2047,16 @@ repeat_set_config: } #if USB_HAVE_MSCTEST if (set_config_failed == 0 && config_index == 0 && + usb_test_quirk(&uaa, UQ_MSC_NO_START_STOP) == 0 && + usb_test_quirk(&uaa, UQ_MSC_NO_PREVENT_ALLOW) == 0 && usb_test_quirk(&uaa, UQ_MSC_NO_SYNC_CACHE) == 0 && + usb_test_quirk(&uaa, UQ_MSC_NO_TEST_UNIT_READY) == 0 && usb_test_quirk(&uaa, UQ_MSC_NO_GETMAXLUN) == 0) { /* * Try to figure out if there are any MSC quirks we * should apply automatically: */ - err = usb_msc_auto_quirk(udev, 0); + err = usb_msc_auto_quirk(udev, 0, &uaa); if (err != 0) { set_config_failed = 1; diff --git a/sys/dev/usb/usb_msctest.c b/sys/dev/usb/usb_msctest.c index 0fffd99a73c4..5dcf8d151119 100644 --- a/sys/dev/usb/usb_msctest.c +++ b/sys/dev/usb/usb_msctest.c @@ -2,7 +2,8 @@ /*- * SPDX-License-Identifier: BSD-2-Clause-FreeBSD * - * Copyright (c) 2008,2011 Hans Petter Selasky. All rights reserved. + * Copyright (c) 2008-2022 Hans Petter Selasky. + * Copyright (c) 2021-2022 Idwer Vollering. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -29,9 +30,6 @@ /* * The following file contains code that will detect USB autoinstall * disks. - * - * TODO: Potentially we could add code to automatically detect USB - * mass storage quirks for not supported SCSI commands! */ #ifdef USB_GLOBAL_INCLUDE_FILE @@ -97,7 +95,8 @@ enum { static uint8_t scsi_test_unit_ready[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; static uint8_t scsi_inquiry[] = { 0x12, 0x00, 0x00, 0x00, SCSI_INQ_LEN, 0x00 }; static uint8_t scsi_rezero_init[] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; -static uint8_t scsi_start_stop_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x02, 0x00 }; +static uint8_t scsi_start_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x01, 0x00 }; +static uint8_t scsi_stop_unit[] = { 0x1b, 0x00, 0x00, 0x00, 0x02, 0x00 }; static uint8_t scsi_ztestor_eject[] = { 0x85, 0x01, 0x01, 0x01, 0x18, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00, 0x00 }; static uint8_t scsi_cmotech_eject[] = { 0xff, 0x52, 0x44, 0x45, 0x56, 0x43, @@ -759,28 +758,49 @@ usb_msc_get_max_lun(struct usb_device *udev, uint8_t iface_index) return (buf); } +#define USB_ADD_QUIRK(udev, any, which) do { \ + if (usb_get_manufacturer(udev) != NULL && usb_get_product(udev) != NULL) { \ + DPRINTFN(0, #which " set for USB mass storage device %s %s (0x%04x:0x%04x)\n", \ + usb_get_manufacturer(udev), \ + usb_get_product(udev), \ + UGETW(udev->ddesc.idVendor), \ + UGETW(udev->ddesc.idProduct)); \ + } else { \ + DPRINTFN(0, #which " set for USB mass storage device, 0x%04x:0x%04x\n", \ + UGETW(udev->ddesc.idVendor), \ + UGETW(udev->ddesc.idProduct)); \ + } \ + usbd_add_dynamic_quirk(udev, which); \ + any = 1; \ +} while (0) + usb_error_t -usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index) +usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index, + const struct usb_attach_arg *uaa) { struct bbb_transfer *sc; uint8_t timeout; uint8_t is_no_direct; uint8_t sid_type; + uint8_t any_quirk; int err; sc = bbb_attach(udev, iface_index, UICLASS_MASS); if (sc == NULL) return (0); + any_quirk = 0; + /* * Some devices need a delay after that the configuration * value is set to function properly: */ usb_pause_mtx(NULL, hz); - if (usb_msc_get_max_lun(udev, iface_index) == 0) { + if (usb_test_quirk(uaa, UQ_MSC_NO_GETMAXLUN) == 0 && + usb_msc_get_max_lun(udev, iface_index) == 0) { DPRINTF("Device has only got one LUN.\n"); - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_GETMAXLUN); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_GETMAXLUN); } is_no_direct = 1; @@ -807,37 +827,40 @@ usb_msc_auto_quirk(struct usb_device *udev, uint8_t iface_index) goto done; } - err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, - &scsi_test_unit_ready, sizeof(scsi_test_unit_ready), - USB_MS_HZ); + if (usb_test_quirk(uaa, UQ_MSC_NO_TEST_UNIT_READY) == 0) { + err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0, + &scsi_test_unit_ready, sizeof(scsi_test_unit_ready), + USB_MS_HZ); - if (err != 0) { - if (err != ERR_CSW_FAILED) - goto error; - DPRINTF("Test unit ready failed\n"); + if (err != 0) { + if (err != ERR_CSW_FAILED) + goto error; + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_TEST_UNIT_READY); + } } - err = bbb_command_start(sc, DIR_OUT, 0, NULL, 0, - &scsi_prevent_removal, sizeof(scsi_prevent_removal), - USB_MS_HZ); - - if (err == 0) { - err = bbb_command_start(sc, DIR_OUT, 0, NULL, 0, - &scsi_allow_removal, sizeof(scsi_allow_removal), + if (usb_test_quirk(uaa, UQ_MSC_NO_PREVENT_ALLOW) == 0) { + err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0, + &scsi_prevent_removal, sizeof(scsi_prevent_removal), USB_MS_HZ); - } - if (err != 0) { - if (err != ERR_CSW_FAILED) - goto error; - DPRINTF("Device doesn't handle prevent and allow removal\n"); - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_PREVENT_ALLOW); + if (err == 0) { + err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0, + &scsi_allow_removal, sizeof(scsi_allow_removal), + USB_MS_HZ); + } + + if (err != 0) { + if (err != ERR_CSW_FAILED) + goto error; + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_PREVENT_ALLOW); + } } timeout = 1; retry_sync_cache: - err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, + err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0, &scsi_sync_cache, sizeof(scsi_sync_cache), USB_MS_HZ); @@ -845,9 +868,7 @@ retry_sync_cache: if (err != ERR_CSW_FAILED) goto error; - DPRINTF("Device doesn't handle synchronize cache\n"); - - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_SYNC_CACHE); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE); } else { /* * Certain Kingston memory sticks fail the first @@ -872,11 +893,7 @@ retry_sync_cache: if (timeout--) goto retry_sync_cache; - DPRINTF("Device most likely doesn't " - "handle synchronize cache\n"); - - usbd_add_dynamic_quirk(udev, - UQ_MSC_NO_SYNC_CACHE); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE); } else { if (err != ERR_CSW_FAILED) goto error; @@ -884,6 +901,18 @@ retry_sync_cache: } } + if (usb_test_quirk(uaa, UQ_MSC_NO_START_STOP) == 0) { + err = bbb_command_start(sc, DIR_NONE, 0, NULL, 0, + &scsi_start_unit, sizeof(scsi_start_unit), + USB_MS_HZ); + + if (err != 0) { + if (err != ERR_CSW_FAILED) + goto error; + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_START_STOP); + } + } + /* clear sense status of any failed commands on the device */ err = bbb_command_start(sc, DIR_IN, 0, sc->buffer, @@ -907,24 +936,28 @@ retry_sync_cache: if (err != ERR_CSW_FAILED) goto error; } + goto done; +error: + /* Apply most quirks */ + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_SYNC_CACHE); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_PREVENT_ALLOW); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_TEST_UNIT_READY); + USB_ADD_QUIRK(udev, any_quirk, UQ_MSC_NO_START_STOP); done: bbb_detach(sc); - return (0); -error: - bbb_detach(sc); - - DPRINTF("Device did not respond, enabling all quirks\n"); - - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_SYNC_CACHE); - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_PREVENT_ALLOW); - usbd_add_dynamic_quirk(udev, UQ_MSC_NO_TEST_UNIT_READY); + if (any_quirk) { + /* Unconfigure device, to clear software data toggle. */ + usbd_set_config_index(udev, USB_UNCONFIG_INDEX); - /* Need to re-enumerate the device */ - usbd_req_re_enumerate(udev, NULL); + /* Need to re-enumerate the device to clear its state. */ + usbd_req_re_enumerate(udev, NULL); + return (USB_ERR_STALLED); + } - return (USB_ERR_STALLED); + /* No quirks were added, continue as usual. */ + return (0); } usb_error_t @@ -944,7 +977,7 @@ usb_msc_eject(struct usb_device *udev, uint8_t iface_index, int method) USB_MS_HZ); DPRINTF("Test unit ready status: %s\n", usbd_errstr(err)); err = bbb_command_start(sc, DIR_IN, 0, NULL, 0, - &scsi_start_stop_unit, sizeof(scsi_start_stop_unit), + &scsi_stop_unit, sizeof(scsi_stop_unit), USB_MS_HZ); break; case MSC_EJECT_REZERO: diff --git a/sys/dev/usb/usb_msctest.h b/sys/dev/usb/usb_msctest.h index 6b5d3283738b..ba4e094bab60 100644 --- a/sys/dev/usb/usb_msctest.h +++ b/sys/dev/usb/usb_msctest.h @@ -2,7 +2,7 @@ /*- * SPDX-License-Identifier: BSD-2-Clause-FreeBSD * - * Copyright (c) 2008 Hans Petter Selasky. All rights reserved. + * Copyright (c) 2008-2022 Hans Petter Selasky. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -44,7 +44,7 @@ int usb_iface_is_cdrom(struct usb_device *udev, usb_error_t usb_msc_eject(struct usb_device *udev, uint8_t iface_index, int method); usb_error_t usb_msc_auto_quirk(struct usb_device *udev, - uint8_t iface_index); + uint8_t iface_index, const struct usb_attach_arg *uaa); usb_error_t usb_msc_read_10(struct usb_device *udev, uint8_t iface_index, uint32_t lba, uint32_t blocks, void *buffer);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202203031441.223Efmm3018346>