Re: [PATCH] scsi: use ATA-12 pass-thru for OPAL as fallback

[Milan Broz]

On 10/16/23 15:27, Christoph Hellwig wrote:
> On Mon, Oct 16, 2023 at 02:46:03PM +0200, Milan Broz wrote:
> > The problem is that we (for simplicity) decided to use kernel SED-ioctl interface that
> > internally wraps OPAL command to SCSI SECURITY command only. It means, that all devices
>
> No, it doesn't.  It uses the properly specified protocol for each
> layer.  That is NVMe uses NVMe Security Send/Receive, SCSI uses the
> SCSI protocol, and libata translats for ATA devices.
>
> > that can use ATA-12 just cannot work with this kernel interface (unlike userspace which
> > can decide which wrapper to use).
>
> It supports all devices that actually speak ATA perfectly fine, take
> a look at ata_scsi_security_inout_xlat.

Yes, I have several of them in my test machine. The comment was about (S)ATA connected
through USB bridge only.
 
> > And IMO it is not correct - if it was designed only for some servers with directly connected
> > devices, then it is really not generic OPAL support. It should work for any hw that supports it.
>
> Let's get off your crack pipe before we continue.  It is designed and
> implemented to support the security protocols exactly as spec'ed.
>
> You seem to have found devices that claim to be SCSI, but actually
> require ATA passthrough for security.  That's no secret cabal to lock
> out non-server hardware but just proper protocol design.

*grin* I just bought several NVMe to USB adapters that presents NVMe device as SCSI, this
is pretty common.

(And Thunderbolt adapters - that present NMVe as real NVMe is another story too.
But once configured, it is doing it correctly.)

But yes, you are right - except the USB hw is here (in huge quantities) and I want to use it.
It is quite possible that there is not way to do it clearly - fine, that's why I sent the patch
for review.

> > For USB, it actually works quite nice with the patch (ignoring usual bugs in firmware).
>
> So move it into usb if you can convince the usb maintainers that they
> are fine with it.

Yep, fair enough. My initial motivation was just understand WTF is going there.
Put the support on a proper layer is step #2.

> > > Note that nowhere in your patch do you test if you are talking to an ATA device.
> >
> > Yes, I know. I expected the command to be rejected if not supported.
>
> Good luck.  Cheap storage hardware trips up on unknown commands all the
> time.

... And my tests for TCG OPAL commands shows that it can be even worse on this layer :-)
(To be fair, recent NVMe devices looks much better. Anyway, yes, I know what you mean.)

> > IMO it is quite similar to discard/TRIM support...
>
> Where we also don't support weird ATA commands directly from sd
> for good reason.

ok, I am actually quite happy that I get some response to this patch.
Supporting it is a mess, but I still believe we can do it (if fw is not completely bogus).

Thanks,
Milan