| Message ID | 20231006125445.122380-1-gmazyland@gmail.com |
|---|---|
| Headers | show |
| Series | usb-storage,uas,scsi: Support OPAL commands on USB attached devices. | expand |
On Mon, Oct 16, 2023 at 09:25:57AM +0200, Milan Broz wrote: > This patchset adds support for OPAL commands (self-encrypted drives) > through USB-attached storage (usb-storage and UAS drivers). This is version 2 of the proposed patch set, but you didn't include the version number in the email Subject: lines and you didn't include the summary of differences from v1 below the "---" lines of the various patches. Patches 5, 6, and 7 look okay. You can add my Reviewed-by: to each of them. I've got some additional comments on patch 4 (in a separate email). Alan Stern
On 10/16/23 19:33, Alan Stern wrote: > On Mon, Oct 16, 2023 at 09:25:57AM +0200, Milan Broz wrote: >> This patchset adds support for OPAL commands (self-encrypted drives) >> through USB-attached storage (usb-storage and UAS drivers). > > This is version 2 of the proposed patch set, but you didn't include the > version number in the email Subject: lines and you didn't include the > summary of differences from v1 below the "---" lines of the various > patches. Hi, well, the first patchset was RFC, so I sent is as "the first real version". Perhaps not the correct way, sorry for that. Anyway, if you see the discussion about OPAL change on SCSI list, another solution (inside USB storage driver) is needed. So, please ignore patch 6/7, these will be needed, but I have to rewrite SCSI logic to USB glue/UAS driver. But for the generic 64-bit flags (patch 1-5), if you see this useful, please review it. Common requirement is that kernel patch need an user for merge (and my flag is currently no going to be used without rewrite). But that time will come one day, and if I can save people time to reinvent the 64-bit quirks logic, it would be nice to merge it. Thanks, Milan > > Patches 5, 6, and 7 look okay. You can add my Reviewed-by: to each of > them. > > I've got some additional comments on patch 4 (in a separate email). > > Alan Stern
On Mon, Oct 16, 2023 at 09:26:01AM +0200, Milan Broz wrote: > The USB mass storage quirks flags can be stored in driver_info in > a 32-bit integer (unsigned long on 32-bit platforms). > As this attribute cannot be enlarged, we need to use some form > of translation of 64-bit quirk bits. > > This problem was discussed on the USB list > https://lore.kernel.org/linux-usb/f9e8acb5-32d5-4a30-859f-d4336a86b31a@gmail.com/ > > The initial solution to use a static array extensively increased the size > of the kernel module, so I decided to try the second suggested solution: > generate a table by host-compiled program and use bit 31 to indicate > that the value is an index, not the actual value. > > This patch adds a host-compiled program that processes unusual_devs.h > (and unusual_uas.h) and generates files usb-ids.c and usb-ids-uas.c > (for pre-processed USB device table with 32-bit device info). > These files also contain a generated translation table for device_info > to 64-bit values. > > The translation function is used only in usb-storage and uas modules; all > other USB storage modules store flags directly, using only 32-bit integers. > > This translation is unnecessary for a 64-bit system, but I keep it > in place for simplicity in this patch. > > Signed-off-by: Milan Broz <gmazyland@gmail.com> > --- > drivers/usb/storage/Makefile | 25 ++++ > drivers/usb/storage/mkflags.c | 226 +++++++++++++++++++++++++++++ > drivers/usb/storage/uas-detect.h | 4 +- > drivers/usb/storage/uas.c | 20 +-- > drivers/usb/storage/usb-ids.h | 33 +++++ > drivers/usb/storage/usb.c | 10 +- > drivers/usb/storage/usual-tables.c | 23 +-- > 7 files changed, 301 insertions(+), 40 deletions(-) > create mode 100644 drivers/usb/storage/mkflags.c > create mode 100644 drivers/usb/storage/usb-ids.h > > diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile > index 46635fa4a340..612678f108d0 100644 > --- a/drivers/usb/storage/Makefile > +++ b/drivers/usb/storage/Makefile > @@ -45,3 +45,28 @@ ums-realtek-y := realtek_cr.o > ums-sddr09-y := sddr09.o > ums-sddr55-y := sddr55.o > ums-usbat-y := shuttle_usbat.o > + > +# The mkflags host-compiled generator produces usb-ids.c (usb-storage) > +# and usb-ids-uas.c (uas) with USB device tables. > +# These tables include pre-computed 32-bit flags as USB driver device_info s/flags as/flags, as/ Otherwise this seems to say that the 32-bit flags are converted to USB driver device_info values -- an incorrect parsing that makes no sense and will confuse readers. (It confused me at first.) Also, don't you really mean "driver_info" rather than "driver device_info"? That's the name of the field in struct usb_device_id. Same comments apply to each place you used this text. > +# (where the value is stored) can be only 32-bit. > +# The most significant bit means it is index to 64-bit flags table pre-computed > +# by mkflags. Currently used only by mass-storage and UAS driver. > + > +$(obj)/usual-tables.o: $(obj)/usb-ids.c > +$(obj)/uas.o: $(obj)/usb-ids-uas.c > +clean-files := usb-ids.c usb-ids-uas.c > +HOSTCFLAGS_mkflags.o := -I $(srctree)/include/ > +hostprogs += mkflags > + > +quiet_cmd_mkflag_storage = FLAGS $@ > +cmd_mkflag_storage = $(obj)/mkflags storage > $@ > + > +quiet_cmd_mkflag_uas = FLAGS $@ > +cmd_mkflag_uas = $(obj)/mkflags uas > $@ > + > +$(obj)/usb-ids.c: $(obj)/mkflags FORCE > + $(call if_changed,mkflag_storage) > + > +$(obj)/usb-ids-uas.c: $(obj)/mkflags FORCE > + $(call if_changed,mkflag_uas) Hmmm... Where does this say that usb-ids.c and usb-ids-uas.c depend on unusual_devs.h and unusual_uas.h? Is the kbuild system smart enough to figure that out on its own (this seems unlikely)? And do the FORCE things need to be there? After all, the .c files don't need to be rebuilt if nothing has changed since the last build. > diff --git a/drivers/usb/storage/mkflags.c b/drivers/usb/storage/mkflags.c > new file mode 100644 > index 000000000000..2514ffef0154 > --- /dev/null > +++ b/drivers/usb/storage/mkflags.c > @@ -0,0 +1,226 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +/* > + * This is host-compiled generator for usb-ids.c (usb-storage) > + * and usb-ids-uas.c (uas). > + * > + * Generated files contain pre-computed 32-bit flags as USB driver > + * device_info (where the value is stored) can be only 32-bit. > + * The most significant bit means that it is index to 64-bit flags table > + * named usb_stor_di_to_u64 with size stored in usb_stor_di_idx_size > + * (for sanity check). > + * > + * Note that usb-storage driver contains also UAS devices, while UAS > + * driver contains only UAS entries (so there can be duplicities). "duplicates", not "duplicities". ("duplicity" means something else -- basically it means "lying".) > + */ > + > +#include <stdio.h> > +#include <string.h> > + > +/* > + * Cannot use userspace <inttypes.h> as code below > + * processes internal kernel headers > + */ > +#include <linux/types.h> > + > +/* > + * Silence warning for definitions in headers we do not use > + */ > +struct usb_device_id {}; > +struct usb_interface; > + > +#include <linux/usb_usual.h> > + > +struct svals { "svals" is a rather generic and uninformative name. How about calling it "unusual_dev_entry" or something like that, instead? > + unsigned int type; > + > + /*interface */ > + uint8_t bDeviceSubClass; > + uint8_t bDeviceProtocol; > + > + /* device */ > + uint16_t idVendor; > + uint16_t idProduct; > + uint16_t bcdDevice_lo; > + uint16_t bcdDevice_hi; > + > + uint64_t flags; > + unsigned int set; > + unsigned int idx; > +}; > + > +enum { TYPE_DEVICE_STORAGE, TYPE_DEVICE_UAS, TYPE_CLASS }; > +enum { FLAGS_NOT_SET, FLAGS_SET, FLAGS_DUPLICATE }; If you give names to these enums and move their definitions up above definition of svals, then you can declare the .type and .set fields to be of the enumerated types instead of just integers. > +#define FLAGS_END (uint64_t)-1 > + > +#define IS_ENABLED(x) 0 > + > +static struct svals vals[] = { > +#define USUAL_DEV(useProto, useTrans) \ > +{ TYPE_CLASS, useProto, useTrans, 0, 0, 0, 0, 0, FLAGS_NOT_SET, 0 } > + > +#define COMPLIANT_DEV UNUSUAL_DEV > + > +/* USB-atached mass storage */ Spelling error. Also, considering that "UAS" stands for "USB-Attached Storage", you might want to change this to "USB Mass Storage" to make the distinction clearer. Or even change both comments to "usb-storage" and "uas", since the driver names are more distinct than the protocol names. > +#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ > + vendorName, productName, useProtocol, useTransport, \ > + initFunction, flags) \ > +{ TYPE_DEVICE_STORAGE, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 } > +#include "unusual_devs.h" > +#undef UNUSUAL_DEV > + > +/* UAS */ > +#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ > + vendorName, productName, useProtocol, useTransport, \ > + initFunction, flags) \ > +{ TYPE_DEVICE_UAS, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 } > +#include "unusual_uas.h" > +#undef UNUSUAL_DEV > + > +/* Terminating entry */ > +{ .flags = FLAGS_END } > +}; > +#undef USUAL_DEV > +#undef COMPLIANT_DEV > +#undef IS_ENABLED > + > +/* Highest bit indicates it is index to usb_stor_di_to_u64 table */ > +#define HI32 (uint32_t)(1UL << 31) > + > +static unsigned long get_device_info(uint64_t flags, unsigned int idx) > +{ > + if (flags < HI32) > + return (unsigned long)flags; > + > + /* Use index that will be processed in usb_stor_di2flags */ > + return HI32 + idx; > +} > + > +static void print_class(uint8_t bDeviceSubClass, uint8_t bDeviceProtocol) > +{ > + printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO, "); > + printf(".bInterfaceClass = USB_CLASS_MASS_STORAGE, "); > + printf(".bInterfaceSubClass = 0x%x, .bInterfaceProtocol = 0x%x },\n", > + bDeviceSubClass, bDeviceProtocol); > +} > +static void print_type(unsigned int type) > +{ > + for (int i = 0; vals[i].flags != FLAGS_END; i++) { > + if (vals[i].type != type) > + continue; > + > + if (type == TYPE_DEVICE_STORAGE || type == TYPE_DEVICE_UAS) { > + printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE_AND_VERSION, "); > + printf(".idVendor = 0x%04x, .idProduct =0x%04x, " > + ".bcdDevice_lo = 0x%04x, .bcdDevice_hi = 0x%04x, .driver_info = 0x%lx },\n", > + vals[i].idVendor, vals[i].idProduct, > + vals[i].bcdDevice_lo, vals[i].bcdDevice_hi, > + get_device_info(vals[i].flags, vals[i].idx)); > + } else if (type == TYPE_CLASS) > + print_class(vals[i].bDeviceSubClass, vals[i].bDeviceProtocol); > + } > +} > + > +static void print_usb_flags(const char *type) > +{ > + int i, count; > + > + printf("const u64 usb_%s_di_to_u64[] = {\n", type); > + for (i = 0, count = 0; vals[i].flags != FLAGS_END; i++) { > + if (vals[i].set == FLAGS_SET) { > + printf("\t/* 0x%02x */ 0x%llx,\n", vals[i].idx, vals[i].flags); > + count++; > + } > + } > + printf("};\n\n"); > + printf("const unsigned long usb_%s_di_idx_size = %i;\n\n", type, count); > +} > + > +static void print_usb_storage(void) > +{ > + printf("#include <linux/usb.h>\n\n"); > + > + /* conversion table from 32-bit device_flags to 64-bit flags */ > + print_usb_flags("stor"); > + > + /* usb_storage_usb_ids */ > + printf("const struct usb_device_id usb_storage_usb_ids[] = {\n"); > + > + /* USB storage devices */ > + print_type(TYPE_DEVICE_STORAGE); > + > + /* UAS storage devices */ > + printf("#if IS_ENABLED(CONFIG_USB_UAS)\n"); > + print_type(TYPE_DEVICE_UAS); > + printf("#endif\n"); > + > + /* transport subclasses */ > + print_type(TYPE_CLASS); > + > + printf("\t{ } /* Terminating entry */\n};\n"); > + printf("MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids);\n"); > +} > + > +static void print_usb_uas(void) > +{ > + printf("#include <linux/usb.h>\n\n"); > + > + /* conversion table from 32-bit device_flags to 64-bit flags */ > + print_usb_flags("uas"); > + > + /* uas_usb_ids */ > + printf("const struct usb_device_id uas_usb_ids[] = {\n"); > + > + /* UAS storage devices */ > + print_type(TYPE_DEVICE_UAS); > + > + /* transport subclasses */ > + print_class(USB_SC_SCSI, USB_PR_BULK); > + print_class(USB_SC_SCSI, USB_PR_UAS); > + > + printf("\t{ } /* Terminating entry */\n};\n"); > + printf("MODULE_DEVICE_TABLE(usb, uas_usb_ids);\n"); > +} > + > +int main(int argc, char *argv[]) > +{ > + int i, j, idx = 0, idx_old, skip = 0; > + > + if (argc != 2 || (strcmp(argv[1], "storage") && strcmp(argv[1], "uas"))) { > + printf("Please specify output type: storage or uas.\n"); > + return 1; > + } > + > + for (i = 0; vals[i].flags != FLAGS_END; i++) { > + if (vals[i].type == TYPE_CLASS) > + continue; > + skip = 0; > + if (vals[i].flags >= HI32) { > + for (j = 0; j < i; j++) { > + if (vals[j].flags == vals[i].flags && > + vals[j].set == FLAGS_SET) { > + skip = 1; > + idx_old = vals[j].idx; > + break; > + } > + } > + if (skip) { > + vals[i].idx = idx_old; > + vals[i].set = FLAGS_DUPLICATE; > + } else { > + vals[i].idx = idx; > + vals[i].set = FLAGS_SET; > + idx++; > + } > + } > + } > + > + if (!strcmp(argv[1], "storage")) > + print_usb_storage(); > + else if (!strcmp(argv[1], "uas")) > + print_usb_uas(); > + else > + return 1; > + > + return 0; > +} > diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h > index 4d3b49e5b87a..fbe068b138c4 100644 > --- a/drivers/usb/storage/uas-detect.h > +++ b/drivers/usb/storage/uas-detect.h > @@ -54,12 +54,14 @@ static int uas_find_endpoints(struct usb_host_interface *alt, > > static int uas_use_uas_driver(struct usb_interface *intf, > const struct usb_device_id *id, > + const u64 *di_to_u64, > + unsigned long di_idx_size, > u64 *flags_ret) > { > struct usb_host_endpoint *eps[4] = { }; > struct usb_device *udev = interface_to_usbdev(intf); > struct usb_hcd *hcd = bus_to_hcd(udev->bus); > - u64 flags = id->driver_info; > + u64 flags = usb_stor_di2flags(di_to_u64, di_idx_size, id->driver_info); > struct usb_host_interface *alt; > int r; > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index 696bb0b23599..8a1c4449dcc9 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -26,6 +26,7 @@ > #include <scsi/scsi_host.h> > #include <scsi/scsi_tcq.h> > > +#include "usb-ids.h" > #include "uas-detect.h" > #include "scsiglue.h" > > @@ -909,21 +910,7 @@ static const struct scsi_host_template uas_host_template = { > .cmd_size = sizeof(struct uas_cmd_info), > }; > > -#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ > - vendorName, productName, useProtocol, useTransport, \ > - initFunction, flags) \ > -{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \ > - .driver_info = (flags) } > - > -static struct usb_device_id uas_usb_ids[] = { > -# include "unusual_uas.h" > - { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) }, > - { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) }, > - { } > -}; > -MODULE_DEVICE_TABLE(usb, uas_usb_ids); > - > -#undef UNUSUAL_DEV > +#include "usb-ids-uas.c" I think it would make more sense to put this #include immediately after the "usb-ids.h" line. After all, the two are so similar. > > static int uas_switch_interface(struct usb_device *udev, > struct usb_interface *intf) > @@ -990,7 +977,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) > struct usb_device *udev = interface_to_usbdev(intf); > u64 dev_flags; > > - if (!uas_use_uas_driver(intf, id, &dev_flags)) > + if (!uas_use_uas_driver(intf, id, usb_uas_di_to_u64, usb_uas_di_idx_size, > + &dev_flags)) > return -ENODEV; > > if (uas_switch_interface(udev, intf)) > diff --git a/drivers/usb/storage/usb-ids.h b/drivers/usb/storage/usb-ids.h > new file mode 100644 > index 000000000000..8bfd84e07f96 > --- /dev/null > +++ b/drivers/usb/storage/usb-ids.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#ifndef _USB_STOR_IDS_H_ > +#define _USB_STOR_IDS_H_ > + > +#include <linux/types.h> > +#include <linux/bug.h> > + > +/* Conversion of 32-bit quirks flags for 32-bit platforms */ > +extern const unsigned long usb_stor_di_idx_size; > +extern const unsigned long usb_uas_di_idx_size; > +extern const u64 usb_stor_di_to_u64[]; > +extern const u64 usb_uas_di_to_u64[]; > + > +static u64 usb_stor_di2flags(const u64 *di_to_u64, > + unsigned long idx_size, unsigned long idx) I really dislike all the places you've used "di", such as here in "di2flags". Something a little more explicit would be much better, such as "drv_info". Also, idx_size refers to the size of the table, not the size of the index. So a better name would be "table_size". Similarly, "di_to_u64" sounds like a function name, not an array name. You could call it "drv_info_u64_table". > +{ > + u64 flags = 0; > + > + if (idx < (1UL << 31)) > + return idx; > + > + idx -= (1UL << 31); > + > + if (idx < idx_size) > + flags = di_to_u64[idx]; > + else > + WARN_ONCE(true, "usb_stor_di_to_u64 table not updated"); I think Greg KH is against introducing new usages of WARN/WARN_ON/WARN_ONCE. You can change this to pr_warn(). > + > + return flags; > +} > + > +#endif > diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c > index db8c4d2c8d11..bb48ab1bd461 100644 > --- a/drivers/usb/storage/usb.c > +++ b/drivers/usb/storage/usb.c > @@ -56,6 +56,7 @@ > #include "sierra_ms.h" > #include "option_ms.h" > > +#include "usb-ids.h" > #if IS_ENABLED(CONFIG_USB_UAS) > #include "uas-detect.h" > #endif > @@ -589,7 +590,11 @@ static int get_device_info(struct us_data *us, const struct usb_device_id *id, > us->protocol = (unusual_dev->useTransport == USB_PR_DEVICE) ? > idesc->bInterfaceProtocol : > unusual_dev->useTransport; > - us->fflags = id->driver_info; > + if (fflags_use_index) > + us->fflags = usb_stor_di2flags(usb_stor_di_to_u64, usb_stor_di_idx_size, > + id->driver_info); > + else > + us->fflags = id->driver_info; > usb_stor_adjust_quirks(us->pusb_dev, &us->fflags); > > if (us->fflags & US_FL_IGNORE_DEVICE) { > @@ -1091,7 +1096,8 @@ static int storage_probe(struct usb_interface *intf, > > /* If uas is enabled and this device can do uas then ignore it. */ > #if IS_ENABLED(CONFIG_USB_UAS) > - if (uas_use_uas_driver(intf, id, NULL)) > + if (uas_use_uas_driver(intf, id, usb_stor_di_to_u64, > + usb_stor_di_idx_size, NULL)) > return -ENXIO; > #endif > > diff --git a/drivers/usb/storage/usual-tables.c b/drivers/usb/storage/usual-tables.c > index a26029e43dfd..40ef861dbd08 100644 > --- a/drivers/usb/storage/usual-tables.c > +++ b/drivers/usb/storage/usual-tables.c > @@ -13,28 +13,9 @@ > > > /* > - * The table of devices > + * The table of devices is pre-generated in usb-ids.c > */ > -#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ > - vendorName, productName, useProtocol, useTransport, \ > - initFunction, flags) \ > -{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \ > - .driver_info = (kernel_ulong_t)(flags) } > - > -#define COMPLIANT_DEV UNUSUAL_DEV > - > -#define USUAL_DEV(useProto, useTrans) \ > -{ USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) } > - > -const struct usb_device_id usb_storage_usb_ids[] = { > -# include "unusual_devs.h" > - { } /* Terminating entry */ > -}; > -MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids); > - > -#undef UNUSUAL_DEV > -#undef COMPLIANT_DEV > -#undef USUAL_DEV > +#include "usb-ids.c" > > /* > * The table of devices to ignore > -- > 2.42.0 Alan Stern
This patchset adds support for OPAL commands (self-encrypted drives) through USB-attached storage (usb-storage and UAS drivers). 1) Patches 1-4 only add support for 64-bit quirks for USB storage (unfortunately, USB device info can be 32-bit on 32-bit platforms, and we are out of space for flags now). 2) Patches 5-6 enable OPAL commands on USB device and also adds an ATA-12 pass-thru wrapper to support OPAL even on devices that do not support SCSI SECURITY IN/OUT commands. ATA-12 is already used by sedutils directly; this patch makes internal kernel OPAL ioctl work with ATA-12 too. As patch 6 introduced a new USB quirk that overflows 32-bit, I posted all patches together - but logically, these solve two separate issues. More info 1) 64bit USB storage quirk flags The quirks are transferred through the device info value, which is unsigned long (and as a part of USB infrastructure, it cannot be changed). After discussion on USB list, I used high bit as an indicator that the values need to be translated/unpacked to 64bit (while lower values are used directly). This is implemented through a host-compiled program that generates device tables and translation function. As both usb-storage and UAS drivers share a lot of headers and definitions, we need to generate separate files for usb-storage, UAS and flags translation function. (I also tried to use a statically generated array for flags, but this increased the size of drivers significantly, and the code was quite ugly...) 2) Support for OPAL on USB attached storage. The main support for OPAL on USB-attached storage is straightforward. The patch 6 - enables SCSI security flag for USB mass storage and UAS device by default. - adds an optional wrapper to the SCSI layer for the ATA-12 pass-thru command as an alternative if SECURITY IN/OUT is unavailable. During device detection, these steps are then done: 1) USB driver (mass-storage, UAS) enables security driver flag by default if not disabled by quirk 2) SCSI device enumerates SECURITY IN/OUT support. If detected, SECURITY ON/OUT wrapper is used (as in the current code). If not, the new ATA12 pass-thru wrapper is used instead. 3) SED OPAL code tries OPAL discovery command for the device. If it receives correct reply, OPAL is enabled for the device. Enabling support may uncover many issues, as OPAL-locked devices often tend to generate errors on the locked range. Anyway, cryptsetup will soon support OPAL devices, and I think support for USB devices is a nice feature that enables users to unlock drives even if they are attached through USB adapters. But also, there are bugs in firmware, so I added a quirk flag that can disable security commands for particular devices. The last patch uses this quirk for Realtek 9210, which seems to support OPAL commands, but after configuring OPAL locking range, it also sets the write-protected flag for the whole device. This is perhaps a bug in firmware (all versions I tried), and I will report that later to Realtek. Milan Broz (6): usb-storage: remove UNUSUAL_VENDOR_INTF macro usb-storage: make internal quirks flags 64bit usb-storage: use fflags index only in usb-storage driver usb-storage,uas: use host helper to generate driver info usb-storage,uas,scsi: allow to pass through security commands (OPAL) usb-storage,uas: Disable security commands (OPAL) for RT9210 chip family drivers/scsi/sd.c | 33 ++++- drivers/usb/storage/Makefile | 25 ++++ drivers/usb/storage/alauda.c | 2 +- drivers/usb/storage/cypress_atacb.c | 2 +- drivers/usb/storage/datafab.c | 2 +- drivers/usb/storage/ene_ub6250.c | 2 +- drivers/usb/storage/freecom.c | 2 +- drivers/usb/storage/isd200.c | 2 +- drivers/usb/storage/jumpshot.c | 2 +- drivers/usb/storage/karma.c | 2 +- drivers/usb/storage/mkflags.c | 212 ++++++++++++++++++++++++++++ drivers/usb/storage/onetouch.c | 2 +- drivers/usb/storage/realtek_cr.c | 2 +- drivers/usb/storage/scsiglue.c | 4 + drivers/usb/storage/sddr09.c | 2 +- drivers/usb/storage/sddr55.c | 2 +- drivers/usb/storage/shuttle_usbat.c | 2 +- drivers/usb/storage/uas-detect.h | 4 +- drivers/usb/storage/uas.c | 26 ++-- drivers/usb/storage/unusual_devs.h | 11 ++ drivers/usb/storage/unusual_uas.h | 11 ++ drivers/usb/storage/usb.c | 42 +++--- drivers/usb/storage/usb.h | 7 +- drivers/usb/storage/usual-tables.c | 38 +---- include/linux/usb_usual.h | 2 + 25 files changed, 346 insertions(+), 95 deletions(-) create mode 100644 drivers/usb/storage/mkflags.c