On Mon, Oct 16, 2023 at 09:02:11AM +0200, Milan Broz wrote: > All common USB/SATA or USB/NVMe adapters I tested need this patch. > > In short, these steps are run for OPAL support check: > 1) Storage driver enables security driver flag (security_supported). > USB-attached storage drivers will enable it in a separate patchset. > SCSI and NNVMe drivers do it already. If the flag is not enabled, > no following steps are run, and OPAL remains disabled. > 2) SCSI device enumerates SECURITY IN/OUT command support. If detected, > SECURITY ON/OUT wrapper is used (as in the current code). > If not, new ATA-12 pass-thru wrapper is used instead. > 3) SED OPAL code tries OPAL discovery command for the device. > If it receives a correct reply, OPAL is enabled for the device. > If SCSI SECURITY or ATA-12 command with discovery command is rejected, > OPAL remains disabled. > > Note, USB attached storage needs an additional patchset sent separately > as requested by USB driver maintainers (it contains required changes > related to USB quirk processing). This just feels wrong. These adapters are broken if they can't translated, and we should not put ATA command submission into sd.c. > + cdb[0] = ATA_12; > + cdb[1] = (send ? 5 /* ATA_PROTOCOL_PIO_DATA_IN */ : 4 /* ATA_PROTOCOL_PIO_DATA_OUT */) << 1; > + cdb[2] = 2 /* t_length */ | (1 << 2) /* byt_blok */ | ((send ? 0 : 1) << 3) /* t_dir */; > + cdb[3] = secp; > + put_unaligned_le16(len / 512, &cdb[4]); > + put_unaligned_le16(spsp, &cdb[6]); > + cdb[9] = send ? 0x5e /* ATA_CMD_TRUSTED_SND */: 0x5c /* ATA_CMD_TRUSTED_RCV */; Also avoid all these crazy long lines, and please use the actual constants. Using a good old if/else is actually a very good way to structure the code in a somewhat readable way. > + if (sdkp->security) > + sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit); > + else > + sdkp->opal_dev = init_opal_dev(sdkp, &sd_ata12_submit); Messed up indentation here. besides the fact that the statement is fundamentally wrong and you'll start sending ATA command to random devices.
