Re: [PATCH v2 2/4] block: Add Sed-opal library

[Christoph Hellwig <hch@infradead.org>]

On Wed, Nov 30, 2016 at 07:50:07PM -0500, Keith Busch wrote:
> I think we should get rid of the "majmin" stuff

Absolutely agreed.

>
> and directly use
> block_device. Then if we add the security send/receive operations to the
> block_device_operations, that will simplify chaining the security request
> to the driver without needing to thread the driver's requested callback
> and data the way you have to here since all the necessary information
> is encapsulated in the block_device.

Maybe.  I need to look at the TCG spec again (oh my good, what a fucking
mess), but if I remember the context if it is the whole nvme controller
and not just a namespace, so a block_device might be the wrong context.
Then again we can always go from the block_device to the controller
fairly easily.  So instead of adding the security operation to the
block_device_operations which we don't really need for now maybe we
should add a security_conext to the block device so that we can avoid
all the lookup code?

> We shouldn't need to be allocating an 'opal_dev' for every range. The
> range-specific parts should be in a different structure that the opal_dev
> can have a list of. That will simpify the unlock from suspend a bit.

Agreed.

> I can appreciate how compact this is, but this is a little harder to
> read IMO, and it works only because you were so careful in setting up
> the array. I think expanding the ioctl into a switch will be easier to
> follow, and has a more tolerent coding convention for future additions.

Agreed.