linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] SED OPAL Library
@ 2016-12-19 19:35 Scott Bauer
  2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
                   ` (4 more replies)
  0 siblings, 5 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi


Changes from v2->v3:

1) Removed the necessity of passing around block devices into the opal
   code. We now pass around a sed_context structure which contains a
   previously allocated opal_dev structure, sec_ops fn pointers, and
   opaque data for the send/recv functions to use.
2) Removed the allocation of the opal_dev structure from the opal_code
   to the driver wishing to use opal. The driver will use a sed_context,
   structure and allocate an opal_dev structure for each device. In the
   case of NVMe we store the sed_context structure in the control struct.
   When someone wishes to issue opal commands down to the controller they
   open the char dev. In the NVMe open implementation we assign our
   sed_context into the file structure.

   Pushing the burden of allocating and storing the opal_dev into the
   driver alleviates a bunch of look up code we had in v1/v2. Now by the
   time we get into the sed-opal code the driver has assigned us a
   sed_context and we operate directly on that.

   This should help with in-kernel use cases as well.
3) Since Opal will operate on the entire device, not per-namespace,
   we moved from block/ to fs/ and will operate on the nvme character
   driver. Because of that  the sed "guts" have moved from block/ back
   to lib/.
4) Numerous code clean ups in sed-opal.c to shorten the file. ~700
   lines reduced
5) Removed the variadic test_and_add_va for a hopefully easier to understand
   and maintain ADD_TOKEN macro that assigns values into the flat buffer
   and does error checking.

--------------------------------------------------------------------

This Patch series implements a large portion of the Opal protocol for
self encrypting devices. The driver has the capability of storing a
locking range's password. The password can then be replayed
during a resume from previous suspend-to-RAM.

The driver also supports logic to bring the device out of a factory
default-inactive state into a functional Opal state.

The following logic is supported in order to bring the tper into a
working state:

1) Taking Ownership of the drive (Setting the Admin CPIN).
2) Activating the Locking SP (In Single User Mode or Normal Mode).
3) Setting up Locking Ranges (Single User or Normal Mode).
4) Adding users to Locking Ranges (Normal Mode Only).
5) Locking or Unlocking Locking Ranges (Single User Mode or Normal Mode).
6) Reverting the TPer (Restore to factory default).
7) Setting LR/User passwords (Single User Mode or Normal Mode).
8) Enabling/disabling Shadow MBR.
9) Enabling Users in the LockingSP (Normal Mode Only).
10) Saving Password for resume from suspend.
11) Erase and Secure erasing locking ranges.

All commands are exported through the Fs ioctl.

Scott Bauer (5):
  include: Add definitions for sed
  lib: Add Sed-opal library
  fs: Wire up SED/Opal to ioctl
  nvme: Implement resume_from_suspend and SED Allocation code.
  Maintainers: Add Information for SED Opal library

 MAINTAINERS                   |   10 +
 drivers/nvme/host/core.c      |   67 ++
 drivers/nvme/host/nvme.h      |    8 +-
 drivers/nvme/host/pci.c       |   10 +-
 fs/ioctl.c                    |    3 +
 include/linux/fs.h            |    2 +
 include/linux/sed-opal.h      |   38 +
 include/linux/sed.h           |   76 ++
 include/uapi/linux/sed-opal.h |   94 ++
 include/uapi/linux/sed.h      |   64 ++
 lib/Makefile                  |    2 +-
 lib/sed-opal.c                | 2376 +++++++++++++++++++++++++++++++++++++++++
 lib/sed-opal_internal.h       |  601 +++++++++++
 lib/sed.c                     |  197 ++++
 14 files changed, 3545 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/sed-opal.h
 create mode 100644 include/linux/sed.h
 create mode 100644 include/uapi/linux/sed-opal.h
 create mode 100644 include/uapi/linux/sed.h
 create mode 100644 lib/sed-opal.c
 create mode 100644 lib/sed-opal_internal.h
 create mode 100644 lib/sed.c

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3 1/5] include: Add definitions for sed
  2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
@ 2016-12-19 19:35 ` Scott Bauer
  2016-12-20  6:46   ` Christoph Hellwig
  2016-12-25 14:15   ` Jethro Beekman
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi, Scott Bauer

This patch adds the definitions and structures for the SED
Opal code.

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
---
 include/linux/sed-opal.h      | 38 +++++++++++++++++
 include/linux/sed.h           | 76 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/sed-opal.h | 94 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/sed.h      | 64 +++++++++++++++++++++++++++++
 4 files changed, 272 insertions(+)
 create mode 100644 include/linux/sed-opal.h
 create mode 100644 include/linux/sed.h
 create mode 100644 include/uapi/linux/sed-opal.h
 create mode 100644 include/uapi/linux/sed.h

diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
new file mode 100644
index 0000000..668401c
--- /dev/null
+++ b/include/linux/sed-opal.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef LINUX_OPAL_H
+#define LINUX_OPAL_H
+
+#include <linux/sed.h>
+#include <linux/kernel.h>
+
+int opal_save(struct sed_context *sedc, struct sed_key *key);
+int opal_lock_unlock(struct sed_context *sedc, struct sed_key *key);
+int opal_take_ownership(struct sed_context *sedc, struct sed_key *key);
+int opal_activate_lsp(struct sed_context *sedc, struct sed_key *key);
+int opal_set_new_pw(struct sed_context *sedc, struct sed_key *key);
+int opal_activate_user(struct sed_context *sedc, struct sed_key *key);
+int opal_reverttper(struct sed_context *sedc, struct sed_key *key);
+int opal_setup_locking_range(struct sed_context *sedc, struct sed_key *key);
+int opal_add_user_to_lr(struct sed_context *sedc, struct sed_key *key);
+int opal_enable_disable_shadow_mbr(struct sed_context *sedc, struct sed_key *key);
+int opal_erase_locking_range(struct sed_context *sedc, struct sed_key *key);
+int opal_secure_erase_locking_range(struct sed_context *sedc, struct sed_key *key);
+int opal_unlock_from_suspend(struct sed_context *sedc);
+struct opal_dev *alloc_opal_dev(struct request_queue *q);
+#endif /* LINUX_OPAL_H */
diff --git a/include/linux/sed.h b/include/linux/sed.h
new file mode 100644
index 0000000..bc848b2
--- /dev/null
+++ b/include/linux/sed.h
@@ -0,0 +1,76 @@
+/*
+ * Self-Encrypting Drive interface - sed.h
+ *
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This code is the generic layer to interface with self-encrypting
+ * drives. Specific command sets should advertise support to sed uapi
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifndef LINUX_SED_H
+#define LINUX_SED_H
+
+#include <linux/blkdev.h>
+#include <uapi/linux/sed.h>
+
+/*
+ * These constant values come from:
+ * TCG Storage Architecture Core Spec v2.01 r1
+ * Section: 3.3 Interface Communications
+ */
+enum {
+	TCG_SECP_00 = 0,
+	TCG_SECP_01,
+};
+
+/**
+ * struct sed_context - SED Security context for a device
+ * @ops:The Trusted Send/Recv functions.
+ * @sec_data:Opaque pointer that will be passed to the send/recv fn.
+ *Drivers can use this to pass necessary data required for
+ *Their implementation of send/recv.
+ * @dev:Currently an Opal Dev structure. In the future can be other types
+ *Of security structures.
+ */
+struct sed_context {
+	const struct sec_ops *ops;
+	void *sec_data;
+	void *dev;
+};
+
+/*
+ * sec_ops - transport specific Trusted Send/Receive functions
+* See SPC-4 for specific definitions
+ *
+ * @sec_send: sends the payload to the trusted peripheral
+ *     spsp: Security Protocol Specific
+ *     secp: Security Protocol
+ *     buf: Payload
+ *     len: Payload length
+ * @recv: Receives a payload from the trusted peripheral
+ *     spsp: Security Protocol Specific
+ *     secp: Security Protocol
+ *     buf: Payload
+ *     len: Payload length
+ */
+struct sec_ops {
+	int (*sec_send)(void *ctrl_data, u16 spsp, u8 secp, void *buf, size_t len);
+	int (*sec_recv)(void *ctrl_data, u16 spsp, u8 secp, void *buf, size_t len);
+};
+int fdev_sed_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+
+#endif /* LINUX_SED_H */
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
new file mode 100644
index 0000000..f168dac
--- /dev/null
+++ b/include/uapi/linux/sed-opal.h
@@ -0,0 +1,94 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef _UAPI_OPAL_H
+#define _UAPI_OPAL_H
+
+#include <linux/types.h>
+
+#define OPAL_KEY_MAX 256
+
+enum opal_mbr {
+	OPAL_MBR_ENABLE,
+	OPAL_MBR_DISABLE,
+};
+
+enum opal_user {
+	OPAL_ADMIN1,
+	OPAL_USER1,
+	OPAL_USER2,
+	OPAL_USER3,
+	OPAL_USER4,
+	OPAL_USER5,
+	OPAL_USER6,
+	OPAL_USER7,
+	OPAL_USER8,
+	OPAL_USER9,
+};
+
+enum opal_lock_state {
+	OPAL_RO = 0x01, /* 0001 */
+	OPAL_RW = 0x02, /* 0010 */
+	OPAL_LK = 0x04, /* 0100 */
+};
+
+struct opal_key {
+	__u8	lr;
+	__u8	key_len;
+	__u8	key[OPAL_KEY_MAX];
+};
+
+struct opal_session_info {
+	bool SUM;
+	struct opal_key opal_key;
+	enum opal_user who;
+};
+
+struct opal_user_lr_setup {
+	struct opal_session_info session;
+	size_t range_start;
+	size_t range_length;
+	int    RLE; /* Read Lock enabled */
+	int    WLE; /* Write Lock Enabled */
+};
+
+struct opal_lock_unlock {
+	struct opal_session_info session;
+	enum opal_lock_state l_state;
+};
+
+struct opal_new_pw {
+	struct opal_session_info session;
+
+	/* When we're not operating in SUM, and we first set
+	 * passwords we need to set them via ADMIN authority.
+	 * After passwords are changed, we can set them via,
+	 * User authorities.
+	 * Because of this restriction we need to know about
+	 * Two different users. One in 'who' which we will use
+	 * to start the session and user_for_pw as the user we're
+	 * chaning the pw for.
+	 */
+	struct opal_session_info new_user_pw;
+};
+
+struct opal_mbr_data {
+	u8 enable_disable;
+	struct opal_key key;
+};
+
+#endif /* _UAPI_SED_H */
diff --git a/include/uapi/linux/sed.h b/include/uapi/linux/sed.h
new file mode 100644
index 0000000..1d2b45c
--- /dev/null
+++ b/include/uapi/linux/sed.h
@@ -0,0 +1,64 @@
+/*
+ * Definitions for the self-encrypting drive interface
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef _UAPI_SED_H
+#define _UAPI_SED_H
+
+#include <linux/types.h>
+#include "sed-opal.h"
+
+enum sed_key_type {
+	OPAL,
+	OPAL_PW,
+	OPAL_ACT_USR,
+	OPAL_LR_SETUP,
+	OPAL_LOCK_UNLOCK,
+	OPAL_MBR_DATA,
+};
+
+struct sed_key {
+	__u32 sed_type;
+	union {
+		struct opal_key            opal;
+		struct opal_new_pw         opal_pw;
+		struct opal_session_info   opal_session;
+		struct opal_user_lr_setup  opal_lrs;
+		struct opal_lock_unlock    opal_lk_unlk;
+		struct opal_mbr_data       opal_mbr;
+		/* additional command set key types */
+	};
+};
+
+#define IOC_SED_SAVE		   _IOW('p', 220, struct sed_key)
+#define IOC_SED_LOCK_UNLOCK	   _IOW('p', 221, struct sed_key)
+#define IOC_SED_TAKE_OWNERSHIP	   _IOW('p', 222, struct sed_key)
+#define IOC_SED_ACTIVATE_LSP       _IOW('p', 223, struct sed_key)
+#define IOC_SED_SET_PW             _IOW('p', 224, struct sed_key)
+#define IOC_SED_ACTIVATE_USR       _IOW('p', 225, struct sed_key)
+#define IOC_SED_REVERT_TPR         _IOW('p', 226, struct sed_key)
+#define IOC_SED_LR_SETUP           _IOW('p', 227, struct sed_key)
+#define IOC_SED_ADD_USR_TO_LR      _IOW('p', 228, struct sed_key)
+#define IOC_SED_ENABLE_DISABLE_MBR _IOW('p', 229, struct sed_key)
+#define IOC_SED_ERASE_LR           _IOW('p', 230, struct sed_key)
+#define IOC_SED_SECURE_ERASE_LR    _IOW('p', 231, struct sed_key)
+
+static inline int is_sed_ioctl(unsigned int cmd)
+{
+	return (cmd >= IOC_SED_SAVE && cmd <= IOC_SED_SECURE_ERASE_LR);
+}
+#endif /* _UAPI_SED_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
  2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
@ 2016-12-19 19:35 ` Scott Bauer
  2016-12-19 21:34   ` Keith Busch
                     ` (4 more replies)
  2016-12-19 19:35 ` [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl Scott Bauer
                   ` (2 subsequent siblings)
  4 siblings, 5 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi, Scott Bauer

This patch implements the necessary logic to bring an Opal
enabled drive out of a factory-enabled into a working
Opal state.

This patch set also enables logic to save a password to
be replayed during a resume from suspend.

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
---
 lib/Makefile            |    2 +-
 lib/sed-opal.c          | 2376 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/sed-opal_internal.h |  601 ++++++++++++
 lib/sed.c               |  197 ++++
 4 files changed, 3175 insertions(+), 1 deletion(-)
 create mode 100644 lib/sed-opal.c
 create mode 100644 lib/sed-opal_internal.h
 create mode 100644 lib/sed.c

diff --git a/lib/Makefile b/lib/Makefile
index 50144a3..acb5d82 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -36,7 +36,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
 	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \
 	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
 	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \
-	 once.o
+	 once.o sed.o sed-opal.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
 obj-y += hexdump.o
diff --git a/lib/sed-opal.c b/lib/sed-opal.c
new file mode 100644
index 0000000..65f7263
--- /dev/null
+++ b/lib/sed-opal.c
@@ -0,0 +1,2376 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":OPAL: " fmt
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/genhd.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <uapi/linux/sed-opal.h>
+#include <linux/sed.h>
+#include <linux/sed-opal.h>
+#include <linux/string.h>
+#include <linux/kdev_t.h>
+#include <linux/key.h>
+
+#include "sed-opal_internal.h"
+
+#define IO_BUFFER_LENGTH 2048
+#define MAX_TOKS 64
+
+struct opal_dev;
+typedef int (cont_fn)(struct opal_dev *dev);
+
+struct opal_cmd {
+	cont_fn *cb;
+	void *cb_data;
+
+	size_t pos;
+	u8 cmd_buf[IO_BUFFER_LENGTH * 2];
+	u8 resp_buf[IO_BUFFER_LENGTH * 2];
+	u8 *cmd;
+	u8 *resp;
+};
+
+/*
+ * On the parsed response, we don't store again the toks that are already
+ * stored in the response buffer. Instead, for each token, we just store a
+ * pointer to the position in the buffer where the token starts, and the size
+ * of the token in bytes.
+ */
+struct opal_resp_tok {
+	const u8 *pos;
+	size_t len;
+	enum OPAL_RESPONSE_TOKEN type;
+	enum OPAL_ATOM_WIDTH width;
+	union {
+		u64 u;
+		s64 s;
+	} stored;
+};
+
+/*
+ * From the response header it's not possible to know how many tokens there are
+ * on the payload. So we hardcode that the maximum will be MAX_TOKS, and later
+ * if we start dealing with messages that have more than that, we can increase
+ * this number. This is done to avoid having to make two passes through the
+ * response, the first one counting how many tokens we have and the second one
+ * actually storing the positions.
+ */
+struct parsed_resp {
+	int num;
+	struct opal_resp_tok toks[MAX_TOKS];
+};
+
+struct opal_dev;
+
+typedef int (*opal_step)(struct opal_dev *dev);
+
+struct opal_suspend_data {
+	struct opal_lock_unlock unlk;
+	u8 lr;
+	size_t key_name_len;
+	char key_name[36];
+	struct list_head node;
+};
+
+/**
+ * struct opal_dev - The structure representing a OPAL enabled SED.
+ * @sed_ctx:The SED context, contains fn pointers to sec_send/recv.
+ * @opal_step:A series of opal methods that are necessary to complete a comannd.
+ * @func_data:An array of parameters for the opal methods above.
+ * @state:Describes the current opal_step we're working on.
+ * @dev_lock:Locks the entire opal_dev structure.
+ * @parsed:Parsed response from controller.
+ * @prev_data:Data returned from a method to the controller
+ * @error_cb:Error function that handles closing sessions after a failed method.
+ * @unlk_lst:A list of Locking ranges to unlock on this device during a resume.
+ */
+struct opal_dev {
+	struct sed_context *sed_ctx;
+	const opal_step *funcs;
+	void **func_data;
+	int state;
+	struct mutex dev_lock;
+	u16 comID;
+	u32 HSN;
+	u32 TSN;
+	u64 align;
+	u64 lowest_lba;
+	struct opal_cmd cmd;
+	struct parsed_resp parsed;
+	size_t prev_d_len;
+	void *prev_data;
+	opal_step error_cb;
+	void *error_cb_data;
+
+	struct list_head unlk_lst;
+};
+
+DEFINE_SPINLOCK(list_spinlock);
+
+static void print_buffer(const u8 *ptr, u32 length)
+{
+#ifdef DEBUG
+	print_hex_dump_bytes("OPAL: ", DUMP_PREFIX_OFFSET, ptr, length);
+	pr_debug("\n");
+#endif
+}
+
+#define TPER_SYNC_SUPPORTED BIT(0)
+
+static bool check_tper(const void *data)
+{
+	const struct d0_tper_features *tper = data;
+	u8 flags = tper->supported_features;
+
+	if (!(flags & TPER_SYNC_SUPPORTED)) {
+		pr_err("TPer sync not supported. flags = %d\n",
+		       tper->supported_features);
+		return false;
+	}
+
+	return true;
+}
+
+static bool check_SUM(const void *data)
+{
+	const struct d0_single_user_mode *sum = data;
+	u32 nlo = be32_to_cpu(sum->num_locking_objects);
+
+	if (nlo == 0) {
+		pr_err("Need at least one locking object.\n");
+		return false;
+	}
+
+	pr_debug("Number of locking objects: %d\n", nlo);
+
+	return true;
+}
+
+static u16 get_comID_v100(const void *data)
+{
+	const struct d0_opal_v100 *v100 = data;
+
+	return be16_to_cpu(v100->baseComID);
+}
+
+static u16 get_comID_v200(const void *data)
+{
+	const struct d0_opal_v200 *v200 = data;
+
+	return be16_to_cpu(v200->baseComID);
+}
+
+static int opal_send_cmd(struct opal_dev *dev)
+{
+	return dev->sed_ctx->ops->sec_send(dev->sed_ctx->sec_data,
+					   dev->comID, TCG_SECP_01,
+					   dev->cmd.cmd, IO_BUFFER_LENGTH);
+}
+
+static int opal_recv_cmd(struct opal_dev *dev)
+{
+	return dev->sed_ctx->ops->sec_recv(dev->sed_ctx->sec_data,
+					   dev->comID, TCG_SECP_01,
+					   dev->cmd.resp, IO_BUFFER_LENGTH);
+}
+
+static int opal_recv_check(struct opal_dev *dev)
+{
+	size_t buflen = IO_BUFFER_LENGTH;
+	void *buffer = dev->cmd.resp;
+	struct opal_header *hdr = buffer;
+	int ret;
+
+	do {
+		pr_debug("Sent OPAL command: outstanding=%d, minTransfer=%d\n",
+			 hdr->cp.outstandingData,
+			 hdr->cp.minTransfer);
+
+		if (hdr->cp.outstandingData == 0 ||
+		    hdr->cp.minTransfer != 0)
+			return 0;
+
+		memset(buffer, 0, buflen);
+		ret = opal_recv_cmd(dev);
+	} while (!ret);
+
+	return ret;
+}
+
+static int opal_send_recv(struct opal_dev *dev, cont_fn *cont)
+{
+	int ret;
+
+	ret = opal_send_cmd(dev);
+	if (ret)
+		return ret;
+	ret = opal_recv_cmd(dev);
+	if (ret)
+		return ret;
+	ret = opal_recv_check(dev);
+	if (ret)
+		return ret;
+	return cont(dev);
+}
+
+static void check_geometry(struct opal_dev *dev, const void *data)
+{
+	const struct d0_geometry_features *geo = data;
+
+	dev->align = geo->alignment_granularity;
+	dev->lowest_lba = geo->lowest_aligned_lba;
+}
+
+static int next(struct opal_dev *dev)
+{
+	opal_step func;
+	int error = 0;
+
+	do {
+		func = dev->funcs[dev->state];
+		if (!func)
+			break;
+
+		dev->state++;
+		error = func(dev);
+
+		if (error) {
+			pr_err("Error on step function: %d with error %d: %s\n",
+			       dev->state, error,
+			       opal_error_to_human(error));
+
+			if (dev->error_cb && dev->state > 2)
+				dev->error_cb(dev->error_cb_data);
+		}
+	} while (!error);
+
+	return error;
+}
+
+static int opal_discovery0_end(struct opal_dev *dev)
+{
+	bool foundComID = false, supported = true, single_user = false;
+	const struct d0_header *hdr;
+	const u8 *epos, *cpos;
+	u16 comID = 0;
+	int error = 0;
+
+	epos = dev->cmd.resp;
+	cpos = dev->cmd.resp;
+	hdr = (struct d0_header *)dev->cmd.resp;
+
+	print_buffer(dev->cmd.resp, be32_to_cpu(hdr->length));
+
+	epos += be32_to_cpu(hdr->length); /* end of buffer */
+	cpos += sizeof(*hdr); /* current position on buffer */
+
+	while (cpos < epos && supported) {
+		const struct d0_features *body =
+			(const struct d0_features *)cpos;
+
+		switch (be16_to_cpu(body->code)) {
+		case FC_TPER:
+			supported = check_tper(body->features);
+			break;
+		case FC_SINGLEUSER:
+			single_user = check_SUM(body->features);
+			break;
+		case FC_GEOMETRY:
+			check_geometry(dev, body);
+			break;
+		case FC_LOCKING:
+		case FC_ENTERPRISE:
+		case FC_DATASTORE:
+			/* some ignored properties */
+			pr_debug("Found OPAL feature description: %d\n",
+				 be16_to_cpu(body->code));
+			break;
+		case FC_OPALV100:
+			comID = get_comID_v100(body->features);
+			foundComID = true;
+			break;
+		case FC_OPALV200:
+			comID = get_comID_v200(body->features);
+			foundComID = true;
+			break;
+		case 0xbfff ... 0xffff:
+			/* vendor specific, just ignore */
+			break;
+		default:
+			pr_warn("OPAL Unknown feature: %d\n",
+				be16_to_cpu(body->code));
+
+		}
+		cpos += body->length + 4;
+	}
+
+	if (!supported) {
+		pr_err("This device is not Opal enabled. Not Supported!\n");
+		return 1;
+	}
+
+	if (!single_user)
+		pr_warn("Device doesn't support single user mode\n");
+
+
+	if (!foundComID) {
+		pr_warn("Could not find OPAL comID for device. Returning early\n");
+		return 1;
+	}
+
+	dev->comID = comID;
+
+	return 0;
+}
+
+static int opal_discovery0(struct opal_dev *dev)
+{
+	int ret;
+
+	memset(dev->cmd.resp, 0, IO_BUFFER_LENGTH);
+	dev->comID = 0x0001;
+	ret = opal_recv_cmd(dev);
+	if (ret)
+		return ret;
+	return opal_discovery0_end(dev);
+}
+
+static void add_token_u8(struct opal_cmd *cmd, u8 tok)
+{
+	cmd->cmd[cmd->pos++] = tok;
+}
+
+static ssize_t test_and_add_token_u8(struct opal_cmd *cmd, u8 tok)
+{
+	BUILD_BUG_ON(IO_BUFFER_LENGTH >= SIZE_MAX);
+
+	if (cmd->pos >= IO_BUFFER_LENGTH - 1) {
+		pr_err("Error adding u8: end of buffer.\n");
+		return -ERANGE;
+	}
+
+	add_token_u8(cmd, tok);
+
+	return 0;
+}
+
+#define TINY_ATOM_DATA_MASK GENMASK(5, 0)
+#define TINY_ATOM_SIGNED BIT(6)
+
+#define SHORT_ATOM_ID BIT(7)
+#define SHORT_ATOM_BYTESTRING BIT(5)
+#define SHORT_ATOM_SIGNED BIT(4)
+#define SHORT_ATOM_LEN_MASK GENMASK(3, 0)
+
+static void add_short_atom_header(struct opal_cmd *cmd, bool bytestring,
+				  bool has_sign, int len)
+{
+	u8 atom;
+
+	atom = SHORT_ATOM_ID;
+	atom |= bytestring ? SHORT_ATOM_BYTESTRING : 0;
+	atom |= has_sign ? SHORT_ATOM_SIGNED : 0;
+	atom |= len & SHORT_ATOM_LEN_MASK;
+
+	add_token_u8(cmd, atom);
+}
+
+#define MEDIUM_ATOM_ID (BIT(7) | BIT(6))
+#define MEDIUM_ATOM_BYTESTRING BIT(4)
+#define MEDIUM_ATOM_SIGNED BIT(3)
+#define MEDIUM_ATOM_LEN_MASK GENMASK(2, 0)
+
+static void add_medium_atom_header(struct opal_cmd *cmd, bool bytestring,
+				   bool has_sign, int len)
+{
+	u8 header0;
+
+	header0 = MEDIUM_ATOM_ID;
+	header0 |= bytestring ? MEDIUM_ATOM_BYTESTRING : 0;
+	header0 |= has_sign ? MEDIUM_ATOM_SIGNED : 0;
+	header0 |= (len >> 8) & MEDIUM_ATOM_LEN_MASK;
+	cmd->cmd[cmd->pos++] = header0;
+	cmd->cmd[cmd->pos++] = len;
+}
+
+static void add_token_u64(struct opal_cmd *cmd, u64 number, size_t len)
+{
+	add_short_atom_header(cmd, false, false, len);
+
+	while (len--) {
+		u8 n = number >> (len * 8);
+
+		add_token_u8(cmd, n);
+	}
+}
+
+static ssize_t test_and_add_token_u64(struct opal_cmd *cmd, u64 number)
+{
+	int len;
+	int msb;
+
+	if (!(number & ~TINY_ATOM_DATA_MASK))
+		return test_and_add_token_u8(cmd, number);
+
+	msb = fls(number);
+	len = DIV_ROUND_UP(msb, 4);
+
+	if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
+		pr_err("Error adding u64: end of buffer.\n");
+		return -ERANGE;
+	}
+
+	add_token_u64(cmd, number, len);
+
+	return 0;
+}
+
+static ssize_t add_token_bytestring(struct opal_cmd *cmd,
+				    const u8 *bytestring, size_t len)
+{
+	size_t header_len = 1;
+	bool is_short_atom = true;
+
+	if (len & ~SHORT_ATOM_LEN_MASK) {
+		header_len = 2;
+		is_short_atom = false;
+	}
+
+	if (len >= IO_BUFFER_LENGTH - cmd->pos - header_len) {
+		pr_err("Error adding bytestring: end of buffer.\n");
+		return -ERANGE;
+	}
+
+	if (is_short_atom)
+		add_short_atom_header(cmd, true, false, len);
+	else
+		add_medium_atom_header(cmd, true, false, len);
+
+	memcpy(&cmd->cmd[cmd->pos], bytestring, len);
+	cmd->pos += len;
+
+	return 0;
+}
+
+static ssize_t test_and_add_string(struct opal_cmd *cmd,
+				   const u8 *string,
+				   size_t len)
+{
+	return add_token_bytestring(cmd, string, len);
+}
+
+static ssize_t test_and_add_token_bytestr(struct opal_cmd *cmd,
+					     const u8 *bytestring)
+{
+	return add_token_bytestring(cmd, bytestring, OPAL_UID_LENGTH);
+}
+
+static ssize_t test_and_add_token_half(struct opal_cmd *cmd,
+				       const u8 *bytestring)
+{
+	return add_token_bytestring(cmd, bytestring, OPAL_UID_LENGTH/2);
+}
+
+#define LOCKING_RANGE_NON_GLOBAL 0x03
+
+static int build_locking_range(u8 *buffer, size_t length, u8 lr)
+{
+	if (length < OPAL_UID_LENGTH) {
+		pr_err("Can't build locking range. Length OOB\n");
+		return -ERANGE;
+	}
+
+	memcpy(buffer, OPALUID[OPAL_LOCKINGRANGE_GLOBAL], OPAL_UID_LENGTH);
+
+	if (lr == 0)
+		return 0;
+	buffer[5] = LOCKING_RANGE_NON_GLOBAL;
+	buffer[7] = lr;
+
+	return 0;
+}
+
+static int build_locking_user(u8 *buffer, size_t length, u8 lr)
+{
+	if (length < OPAL_UID_LENGTH) {
+		pr_err("Can't build locking range user, Length OOB\n");
+		return -ERANGE;
+	}
+
+	memcpy(buffer, OPALUID[OPAL_USER1_UID], OPAL_UID_LENGTH);
+
+	buffer[7] = lr + 1;
+
+	return 0;
+}
+
+#define ADD_TOKEN_STRING(cmd, key, keylen)		        \
+	if (!err)					        \
+		err = test_and_add_string(cmd, key, keylen);
+
+#define ADD_TOKEN(type, cmd, tok)				\
+	if (!err)						\
+		err = test_and_add_token_##type(cmd, tok);
+
+static void set_comID(struct opal_cmd *cmd, u16 comID)
+{
+	struct opal_header *hdr = (struct opal_header *)cmd->cmd;
+
+	hdr->cp.extendedComID[0] = comID >> 8;
+	hdr->cp.extendedComID[1] = comID;
+	hdr->cp.extendedComID[2] = 0;
+	hdr->cp.extendedComID[3] = 0;
+}
+
+static int cmd_finalize(struct opal_cmd *cmd, u32 hsn, u32 tsn)
+{
+	struct opal_header *hdr;
+	int err = 0;
+
+
+	ADD_TOKEN(u8, cmd, OPAL_ENDOFDATA);
+	ADD_TOKEN(u8, cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8, cmd, 0);
+	ADD_TOKEN(u8, cmd, 0);
+	ADD_TOKEN(u8, cmd, 0);
+	ADD_TOKEN(u8, cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error finalizing command.\n");
+		return -EFAULT;
+	}
+
+	hdr = (struct opal_header *) cmd->cmd;
+
+	hdr->pkt.TSN = cpu_to_be32(tsn);
+	hdr->pkt.HSN = cpu_to_be32(hsn);
+
+	hdr->subpkt.length = cpu_to_be32(cmd->pos - sizeof(*hdr));
+	while (cmd->pos % 4) {
+		if (cmd->pos >= IO_BUFFER_LENGTH) {
+			pr_err("Error: Buffer overrun\n");
+			return -ERANGE;
+		}
+		cmd->cmd[cmd->pos++] = 0;
+	}
+	hdr->pkt.length = cpu_to_be32(cmd->pos - sizeof(hdr->cp) -
+				      sizeof(hdr->pkt));
+	hdr->cp.length = cpu_to_be32(cmd->pos - sizeof(hdr->cp));
+
+	return 0;
+}
+
+static enum OPAL_RESPONSE_TOKEN token_type(const struct parsed_resp *resp,
+					   int n)
+{
+	const struct opal_resp_tok *tok;
+
+	if (n >= resp->num) {
+		pr_err("Token number doesn't exist: %d, resp: %d\n",
+		       n, resp->num);
+		return OPAL_DTA_TOKENID_INVALID;
+	}
+
+	tok = &resp->toks[n];
+	if (tok->len == 0) {
+		pr_err("Token length must be non-zero\n");
+		return OPAL_DTA_TOKENID_INVALID;
+	}
+
+	return tok->type;
+}
+
+/*
+ * This function returns 0 in case of invalid token. One should call
+ * token_type() first to find out if the token is valid or not.
+ */
+static enum OPAL_TOKEN response_get_token(const struct parsed_resp *resp,
+					  int n)
+{
+	const struct opal_resp_tok *tok;
+
+	if (n >= resp->num) {
+		pr_err("Token number doesn't exist: %d, resp: %d\n",
+		       n, resp->num);
+		return 0;
+	}
+
+	tok = &resp->toks[n];
+	if (tok->len == 0) {
+		pr_err("Token length must be non-zero\n");
+		return 0;
+	}
+
+	return tok->pos[0];
+}
+
+static size_t response_parse_tiny(struct opal_resp_tok *tok,
+				  const u8 *pos)
+{
+	tok->pos = pos;
+	tok->len = 1;
+	tok->width = OPAL_WIDTH_TINY;
+
+	if (pos[0] & TINY_ATOM_SIGNED) {
+		tok->type = OPAL_DTA_TOKENID_SINT;
+	} else {
+		tok->type = OPAL_DTA_TOKENID_UINT;
+		tok->stored.u = pos[0] & 0x3f;
+	}
+
+	return tok->len;
+}
+
+static size_t response_parse_short(struct opal_resp_tok *tok,
+				   const u8 *pos)
+{
+	tok->pos = pos;
+	tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
+	tok->width = OPAL_WIDTH_SHORT;
+
+	if (pos[0] & SHORT_ATOM_BYTESTRING) {
+		tok->type = OPAL_DTA_TOKENID_BYTESTRING;
+	} else if (pos[0] & SHORT_ATOM_SIGNED) {
+		tok->type = OPAL_DTA_TOKENID_SINT;
+	} else {
+		u64 u_integer = 0;
+		int i, b = 0;
+
+		tok->type = OPAL_DTA_TOKENID_UINT;
+		if (tok->len > 9) {
+			pr_warn("uint64 with more than 8 bytes\n");
+			return -EINVAL;
+		}
+		for (i = tok->len - 1; i > 0; i--) {
+			u_integer |= ((u64)pos[i] << (8 * b));
+			b++;
+		}
+		tok->stored.u = u_integer;
+	}
+
+	return tok->len;
+}
+
+static size_t response_parse_medium(struct opal_resp_tok *tok,
+				    const u8 *pos)
+{
+	tok->pos = pos;
+	tok->len = (((pos[0] & MEDIUM_ATOM_LEN_MASK) << 8) | pos[1]) + 2;
+	tok->width = OPAL_WIDTH_MEDIUM;
+
+	if (pos[0] & MEDIUM_ATOM_BYTESTRING)
+		tok->type = OPAL_DTA_TOKENID_BYTESTRING;
+	else if (pos[0] & MEDIUM_ATOM_SIGNED)
+		tok->type = OPAL_DTA_TOKENID_SINT;
+	else
+		tok->type = OPAL_DTA_TOKENID_UINT;
+
+	return tok->len;
+}
+
+#define LONG_ATOM_ID (BIT(7) | BIT(6) | BIT(5))
+#define LONG_ATOM_BYTESTRING BIT(1)
+#define LONG_ATOM_SIGNED BIT(0)
+static size_t response_parse_long(struct opal_resp_tok *tok,
+				  const u8 *pos)
+{
+	tok->pos = pos;
+	tok->len = ((pos[1] << 16) | (pos[2] << 8) | pos[3]) + 4;
+	tok->width = OPAL_WIDTH_LONG;
+
+	if (pos[0] & LONG_ATOM_BYTESTRING)
+		tok->type = OPAL_DTA_TOKENID_BYTESTRING;
+	else if (pos[0] & LONG_ATOM_SIGNED)
+		tok->type = OPAL_DTA_TOKENID_SINT;
+	else
+		tok->type = OPAL_DTA_TOKENID_UINT;
+
+	return tok->len;
+}
+
+static size_t response_parse_token(struct opal_resp_tok *tok,
+				   const u8 *pos)
+{
+	tok->pos = pos;
+	tok->len = 1;
+	tok->type = OPAL_DTA_TOKENID_TOKEN;
+	tok->width = OPAL_WIDTH_TOKEN;
+
+	return tok->len;
+}
+
+static int response_parse(const u8 *buf, size_t length,
+			  struct parsed_resp *resp)
+{
+	const struct opal_header *hdr;
+	struct opal_resp_tok *iter;
+	int ret, num_entries = 0;
+	u32 cpos = 0, total;
+	size_t token_length;
+	const u8 *pos;
+
+	if (!buf)
+		return -EFAULT;
+
+	if (!resp)
+		return -EFAULT;
+
+	hdr = (struct opal_header *)buf;
+	pos = buf;
+	pos += sizeof(*hdr);
+
+	pr_debug("Response size: cp: %d, pkt: %d, subpkt: %d\n",
+		 be32_to_cpu(hdr->cp.length),
+		 be32_to_cpu(hdr->pkt.length),
+		 be32_to_cpu(hdr->subpkt.length));
+
+	if ((hdr->cp.length == 0)
+	    || (hdr->pkt.length == 0)
+	    || (hdr->subpkt.length == 0)) {
+		pr_err("Bad header length. cp: %d, pkt: %d, subpkt: %d\n",
+		       be32_to_cpu(hdr->cp.length),
+		       be32_to_cpu(hdr->pkt.length),
+		       be32_to_cpu(hdr->subpkt.length));
+		print_buffer(pos, sizeof(*hdr));
+		return -EINVAL;
+	}
+
+	if (pos > buf + length)
+		return -EFAULT;
+
+	iter = resp->toks;
+	total = be32_to_cpu(hdr->subpkt.length);
+	print_buffer(pos, total);
+	while (cpos < total) {
+		if (!(pos[0] & 0x80)) /* tiny atom */
+			token_length = response_parse_tiny(iter, pos);
+		else if (!(pos[0] & 0x40)) /* short atom */
+			token_length = response_parse_short(iter, pos);
+		else if (!(pos[0] & 0x20)) /* medium atom */
+			token_length = response_parse_medium(iter, pos);
+		else if (!(pos[0] & 0x10)) /* long atom */
+			token_length = response_parse_long(iter, pos);
+		else /* TOKEN */
+			token_length = response_parse_token(iter, pos);
+
+		if (token_length == -EINVAL)
+			return -EINVAL;
+
+		pos += token_length;
+		cpos += token_length;
+		iter++;
+		num_entries++;
+	}
+
+	if (num_entries == 0) {
+		pr_err("Couldn't parse response.\n");
+		return -EINVAL;
+	resp->num = num_entries;
+
+	return 0;
+}
+
+static size_t response_get_string(const struct parsed_resp *resp, int n,
+				  const char **store)
+{
+	*store = NULL;
+	if (!resp) {
+		pr_err("Response is NULL\n");
+		return 0;
+	}
+
+	if (n > resp->num) {
+		pr_err("Response has %d tokens. Can't access %d\n",
+		       resp->num, n);
+		return 0;
+	}
+
+	if (resp->toks[n].type != OPAL_DTA_TOKENID_BYTESTRING) {
+		pr_err("Token is not a byte string!\n");
+		return 0;
+	}
+
+	*store = resp->toks[n].pos + 1;
+	return resp->toks[n].len - 1;
+}
+
+static u64 response_get_u64(const struct parsed_resp *resp, int n)
+{
+	if (!resp) {
+		pr_err("Response is NULL\n");
+		return 0;
+	}
+
+	if (n > resp->num) {
+		pr_err("Response has %d tokens. Can't access %d\n",
+		       resp->num, n);
+		return 0;
+	}
+
+	if (resp->toks[n].type != OPAL_DTA_TOKENID_UINT) {
+		pr_err("Token is not unsigned it: %d\n",
+		       resp->toks[n].type);
+		return 0;
+	}
+
+	if (!((resp->toks[n].width == OPAL_WIDTH_TINY) ||
+	      (resp->toks[n].width == OPAL_WIDTH_SHORT))) {
+		pr_err("Atom is not short or tiny: %d\n",
+		       resp->toks[n].width);
+		return 0;
+	}
+
+	return resp->toks[n].stored.u;
+}
+
+static u8 response_status(const struct parsed_resp *resp)
+{
+	if ((token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN)
+	    && (response_get_token(resp, 0) == OPAL_ENDOFSESSION)) {
+		return 0;
+	}
+
+	if (resp->num < 5)
+		return DTAERROR_NO_METHOD_STATUS;
+
+	if ((token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN) ||
+	    (token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN) ||
+	    (response_get_token(resp, resp->num - 1) != OPAL_ENDLIST) ||
+	    (response_get_token(resp, resp->num - 5) != OPAL_STARTLIST))
+		return DTAERROR_NO_METHOD_STATUS;
+
+	return response_get_u64(resp, resp->num - 4);
+}
+
+/* Parses and checks for errors */
+static int parse_and_check_status(struct opal_dev *dev)
+{
+	struct opal_cmd *cmd;
+	int error;
+
+	cmd = &dev->cmd;
+	print_buffer(cmd->cmd, cmd->pos);
+
+	error = response_parse(cmd->resp, IO_BUFFER_LENGTH, &dev->parsed);
+	if (error) {
+		pr_err("Couldn't parse response.\n");
+		return error;
+	}
+
+	return response_status(&dev->parsed);
+}
+
+static void clear_opal_cmd(struct opal_cmd *cmd)
+{
+	cmd->pos = sizeof(struct opal_header);
+	memset(cmd->cmd, 0, IO_BUFFER_LENGTH);
+	cmd->cb = NULL;
+	cmd->cb_data = NULL;
+}
+
+static int start_opal_session_cont(struct opal_dev *dev)
+{
+	u32 HSN, TSN;
+	int error = 0;
+
+	error = parse_and_check_status(dev);
+	if (error)
+		return error;
+
+	HSN = response_get_u64(&dev->parsed, 4);
+	TSN = response_get_u64(&dev->parsed, 5);
+
+	if (HSN == 0 && TSN == 0) {
+		pr_err("Couldn't authenticate session\n");
+		return -EPERM;
+	}
+
+	dev->HSN = HSN;
+	dev->TSN = TSN;
+	return 0;
+}
+
+static inline void opal_dev_get(struct opal_dev *dev)
+{
+	mutex_lock(&dev->dev_lock);
+}
+
+static inline void opal_dev_put(struct opal_dev *dev)
+{
+	mutex_unlock(&dev->dev_lock);
+}
+
+static int add_suspend_info(struct opal_dev *dev, struct opal_suspend_data *sus)
+{
+	struct opal_suspend_data *iter;
+	bool found = false;
+
+	if (list_empty(&dev->unlk_lst))
+		goto add_out;
+
+	list_for_each_entry(iter, &dev->unlk_lst, node) {
+		if (iter->lr == sus->lr) {
+			found = true;
+			break;
+		}
+	}
+
+	if (found) {
+		/* Replace the old with the new */
+		list_del(&iter->node);
+		kfree(iter);
+	}
+
+ add_out:
+	list_add_tail(&sus->node, &dev->unlk_lst);
+	return 0;
+}
+
+static int end_session_cont(struct opal_dev *dev)
+{
+	dev->HSN = 0;
+	dev->TSN = 0;
+	return parse_and_check_status(dev);
+}
+
+static int finalize_and_send(struct opal_dev *dev, struct opal_cmd *cmd,
+			     cont_fn cont)
+{
+	int ret;
+
+	ret = cmd_finalize(cmd, dev->HSN, dev->TSN);
+	if (ret) {
+		pr_err("Error finalizing command buffer: %d\n", ret);
+		return ret;
+	}
+
+	print_buffer(cmd->cmd, cmd->pos);
+
+	return opal_send_recv(dev, cont);
+}
+
+static int gen_key(struct opal_dev *dev)
+{
+	const u8 *method;
+	u8 uid[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	memcpy(uid, dev->prev_data, min(sizeof(uid), dev->prev_d_len));
+	method = OPALMETHOD[OPAL_GENKEY];
+	kfree(dev->prev_data);
+	dev->prev_data = NULL;
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_GENKEY]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building gen key command\n");
+		return err;
+
+	}
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int get_active_key_cont(struct opal_dev *dev)
+{
+	const char *activekey;
+	size_t keylen;
+	int error = 0;
+
+	error = parse_and_check_status(dev);
+	if (error)
+		return error;
+	keylen = response_get_string(&dev->parsed, 4, &activekey);
+	if (!activekey) {
+		pr_err("%s: Couldn't extract the Activekey from the response\n",
+		       __func__);
+		return 0x0A;
+	}
+	dev->prev_data = kmemdup(activekey, keylen, GFP_KERNEL);
+
+	if (!dev->prev_data)
+		return -ENOMEM;
+
+	dev->prev_d_len = keylen;
+
+	return 0;
+}
+
+static int get_active_key(struct opal_dev *dev)
+{
+	u8 uid[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	int err = 0;
+	u8 *lr;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+	lr = dev->func_data[dev->state - 1];
+
+	err = build_locking_range(uid, sizeof(uid), *lr);
+	if (err)
+		return err;
+
+	err = 0;
+	ADD_TOKEN(u8, cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_GET]);
+	ADD_TOKEN(u8, cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8, cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8, cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8, cmd, OPAL_TINY_UINT_03); /* startCloumn */
+	ADD_TOKEN(u8, cmd, OPAL_TINY_UINT_10); /* ActiveKey */
+	ADD_TOKEN(u8, cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8, cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8, cmd, OPAL_TINY_UINT_04); /* endColumn */
+	ADD_TOKEN(u8, cmd, OPAL_TINY_UINT_10); /* ActiveKey */
+	ADD_TOKEN(u8, cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8, cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8, cmd, OPAL_ENDLIST);
+	if (err) {
+		pr_err("Error building get active key command\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, get_active_key_cont);
+}
+
+static int generic_lr_enable_disable(struct opal_cmd *cmd,
+				     u8 *uid, bool rle, bool wle,
+				     bool rl, bool wl)
+{
+	int err = 0;
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_05); /* ReadLockEnabled */
+	ADD_TOKEN(u8,      cmd, rle);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_06); /* WriteLockEnabled */
+	ADD_TOKEN(u8,      cmd, wle);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_READLOCKED);
+	ADD_TOKEN(u8,      cmd, rl);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_WRITELOCKED);
+	ADD_TOKEN(u8,      cmd, wl);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	return err;
+}
+
+static inline int enable_global_lr(struct opal_cmd *cmd, u8 *uid,
+				   struct opal_user_lr_setup *setup)
+{
+	int err;
+	err = generic_lr_enable_disable(cmd, uid, !!setup->RLE, !!setup->WLE,
+					0, 0);
+	if (err)
+		pr_err("Failed to create enable global lr command\n");
+	return err;
+}
+
+static int setup_locking_range(struct opal_dev *dev)
+{
+	u8 uid[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	struct opal_user_lr_setup *setup;
+	u8 lr;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	setup = dev->func_data[dev->state - 1];
+	lr = setup->session.opal_key.lr;
+	err = build_locking_range(uid, sizeof(uid), lr);
+	if (err)
+		return err;
+
+	if (lr == 0)
+		err = enable_global_lr(cmd, uid, setup);
+	else {
+		ADD_TOKEN(u8,      cmd, OPAL_CALL);
+		ADD_TOKEN(bytestr, cmd, uid);
+		ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+		ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* Ranges Start */
+		ADD_TOKEN(u64,     cmd, setup->range_start);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_04); /* Ranges length */
+		ADD_TOKEN(u64,     cmd, setup->range_length);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_05); /* ReadLockEnabled */
+		ADD_TOKEN(u64,     cmd, !!setup->RLE);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_06); /* WriteLockEnabled */
+		ADD_TOKEN(u64,     cmd, !!setup->WLE);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+
+	}
+	if (err) {
+		pr_err("Error building Setup Locking range command.\n");
+		return err;
+
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int start_generic_opal_session(struct opal_dev *dev,
+				      enum OPAL_UID auth,
+				      enum OPAL_UID sp_type,
+				      const char *key,
+				      u8 key_len)
+{
+	struct opal_cmd *cmd;
+	u32 HSN;
+	int err = 0;
+
+	if (key == NULL && auth != OPAL_ANYBODY_UID) {
+		pr_err("%s: Attempted to open ADMIN_SP Session without a Host" \
+		       "Challenge, and not as the Anybody UID\n", __func__);
+		return 1;
+	}
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+
+	set_comID(cmd, dev->comID);
+	HSN = GENERIC_HOST_SESSION_NUM;
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_SMUID_UID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_STARTSESSION]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u64,     cmd, HSN);
+	ADD_TOKEN(bytestr, cmd, OPALUID[sp_type]);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_01);
+
+	switch (auth) {
+	case OPAL_ANYBODY_UID:
+		ADD_TOKEN(u8, cmd, OPAL_ENDLIST);
+		break;
+	case OPAL_ADMIN1_UID:
+	case OPAL_SID_UID:
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_00); /* HostChallenge */
+		ADD_TOKEN_STRING(cmd, key, key_len);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* HostSignAuth */
+		ADD_TOKEN(bytestr, cmd, OPALUID[auth]);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+		break;
+	default:
+		pr_err("Cannot start Admin SP session with auth %d\n", auth);
+		return 1;
+	}
+
+	if (err) {
+		pr_err("Error building start adminsp session command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, start_opal_session_cont);
+}
+
+static int start_anybodyASP_opal_session(struct opal_dev *dev)
+{
+	return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
+					  OPAL_ADMINSP_UID, NULL, 0);
+}
+
+static int start_SIDASP_opal_session(struct opal_dev *dev)
+{
+	int ret;
+	const u8 *key = dev->prev_data;
+	struct opal_key *okey;
+
+	if (!key) {
+		okey = dev->func_data[dev->state - 1];
+		ret = start_generic_opal_session(dev, OPAL_SID_UID,
+						 OPAL_ADMINSP_UID,
+						 okey->key,
+						 okey->key_len);
+	}
+	else {
+		ret = start_generic_opal_session(dev, OPAL_SID_UID,
+						 OPAL_ADMINSP_UID,
+						 key, dev->prev_d_len);
+		kfree(key);
+		dev->prev_data = NULL;
+	}
+	return ret;
+}
+
+static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+{
+	struct opal_key *key = dev->func_data[dev->state - 1];
+	return start_generic_opal_session(dev, OPAL_ADMIN1_UID,
+					  OPAL_LOCKINGSP_UID,
+					  key->key, key->key_len);
+}
+
+static int start_auth_opal_session(struct opal_dev *dev)
+{
+	u8 lk_ul_user[OPAL_UID_LENGTH];
+	int err = 0;
+
+	struct opal_session_info *session = dev->func_data[dev->state - 1];
+	struct opal_cmd *cmd = &dev->cmd;
+	size_t keylen = session->opal_key.key_len;
+	u8 *key = session->opal_key.key;
+	u32 HSN = GENERIC_HOST_SESSION_NUM;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	if (session->SUM) {
+		err = build_locking_user(lk_ul_user, sizeof(lk_ul_user),
+					 session->opal_key.lr);
+		if (err)
+			return err;
+
+	} else if (session->who != OPAL_ADMIN1 && !session->SUM) {
+		err = build_locking_user(lk_ul_user, sizeof(lk_ul_user),
+					 session->who - 1);
+		if (err)
+			return err;
+	} else
+		memcpy(lk_ul_user, OPALUID[OPAL_ADMIN1_UID], OPAL_UID_LENGTH);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_SMUID_UID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_STARTSESSION]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u64,     cmd, HSN);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_LOCKINGSP_UID]);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_01);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_00);
+	ADD_TOKEN_STRING(cmd, key, keylen);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03);
+	ADD_TOKEN(bytestr, cmd, lk_ul_user);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building STARTSESSION command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, start_opal_session_cont);
+}
+
+static int revert_tper(struct opal_dev *dev)
+{
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_ADMINSP_UID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_REVERT]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	if (err) {
+		pr_err("Error building REVERT TPER command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int internal_activate_user(struct opal_dev *dev)
+{
+	struct opal_session_info *session = dev->func_data[dev->state - 1];
+	u8 uid[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	memcpy(uid, OPALUID[OPAL_USER1_UID], OPAL_UID_LENGTH);
+	uid[7] = session->who;
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_05); /* Enabled */
+	ADD_TOKEN(u8,      cmd, OPAL_TRUE);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building Activate UserN command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int erase_locking_range(struct opal_dev *dev)
+{
+	struct opal_session_info *session;
+	u8 uid[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+	session = dev->func_data[dev->state - 1];
+
+	if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
+		return -ERANGE;
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_ERASE]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building Erase Locking Range Cmmand.\n");
+		return err;
+	}
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int set_mbr_done(struct opal_dev *dev)
+{
+	u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state - 1];
+	struct opal_cmd *cmd = &dev->cmd;
+	int err = 0;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_MBRCONTROL]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_02); /* Done */
+	ADD_TOKEN(u8,      cmd, mbr_done_tf); /* Done T or F */
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+  	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error Building set MBR Done command\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int set_mbr_enable_disable(struct opal_dev *dev)
+{
+	u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state - 1];
+	struct opal_cmd *cmd = &dev->cmd;
+	int err = 0;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_MBRCONTROL]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_01);
+	ADD_TOKEN(u8,      cmd, mbr_en_dis);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+  	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error Building set MBR done command\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
+			  struct opal_dev *dev)
+{
+	struct opal_cmd *cmd = &dev->cmd;
+	int err = 0;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, cpin_uid);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* PIN */
+	ADD_TOKEN_STRING(cmd, key, key_len);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	return err;
+}
+
+static int set_new_pw(struct opal_dev *dev)
+{
+	u8 cpin_uid[OPAL_UID_LENGTH];
+	struct opal_session_info *usr = dev->func_data[dev->state - 1];
+	struct opal_cmd *cmd = &dev->cmd;
+
+	memcpy(cpin_uid, OPALUID[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
+
+	if (usr->who != OPAL_ADMIN1) {
+		cpin_uid[5] = 0x03;
+		if (usr->SUM)
+			cpin_uid[7] = usr->opal_key.lr + 1;
+		else
+			cpin_uid[7] = usr->who;
+	}
+
+
+	if (generic_pw_cmd(usr->opal_key.key, usr->opal_key.key_len,
+			   cpin_uid, dev)) {
+		pr_err("Error building set password command.\n");
+		return -ERANGE;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int set_sid_cpin_pin(struct opal_dev *dev)
+{
+	u8 cpin_uid[OPAL_UID_LENGTH];
+	struct opal_key *key = dev->func_data[dev->state - 1];
+	struct opal_cmd *cmd = &dev->cmd;
+
+	memcpy(cpin_uid, OPALUID[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
+	
+	if (generic_pw_cmd(key->key, key->key_len, cpin_uid, dev)) {
+		pr_err("Error building Set SID cpin\n");
+		return -ERANGE;
+	}
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int add_user_to_lr(struct opal_dev *dev)
+{
+	u8 lr_buffer[OPAL_UID_LENGTH];
+	u8 user_uid[OPAL_UID_LENGTH];
+	struct opal_lock_unlock *lkul;
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	lkul = dev->func_data[dev->state - 1];
+
+	memcpy(lr_buffer, OPALUID[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
+	       OPAL_UID_LENGTH);
+
+	if (lkul->l_state == OPAL_RW)
+		memcpy(lr_buffer, OPALUID[OPAL_LOCKINGRANGE_ACE_WRLOCKED],
+		       OPAL_UID_LENGTH);
+
+	lr_buffer[7] = lkul->session.opal_key.lr;
+
+	memcpy(user_uid, OPALUID[OPAL_USER1_UID], OPAL_UID_LENGTH);
+
+	user_uid[7] = lkul->session.who;
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, lr_buffer);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(half,    cmd, OPALUID[OPAL_HALF_UID_AUTHORITY_OBJ_REF]);
+	ADD_TOKEN(bytestr, cmd, user_uid);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(half,    cmd, OPALUID[OPAL_HALF_UID_AUTHORITY_OBJ_REF]);
+	ADD_TOKEN(bytestr, cmd, user_uid);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(half,    cmd, OPALUID[OPAL_HALF_UID_BOOLEAN_ACE]);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_01);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building add user to locking range command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int lock_unlock_locking_range(struct opal_dev *dev)
+{
+	u8 lr_buffer[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	const u8 *method;
+	struct opal_lock_unlock *lkul;
+	u8 read_locked = 1, write_locked = 1;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	method = OPALMETHOD[OPAL_SET];
+	lkul = dev->func_data[dev->state - 1];
+	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
+				lkul->session.opal_key.lr) < 0)
+		return -ERANGE;
+
+	switch (lkul->l_state) {
+	case OPAL_RO:
+		read_locked = 0;
+		write_locked = 1;
+		break;
+	case OPAL_RW:
+		read_locked = 0;
+		write_locked = 0;
+		break;
+	case OPAL_LK:
+		/* vars are initalized to locked */
+		break;
+	default:
+		pr_err("Tried to set an invalid locking state... returning to uland\n");
+		return 1;
+	}
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, lr_buffer);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_SET]);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_VALUES);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_READLOCKED);
+	ADD_TOKEN(u8,      cmd, read_locked);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_WRITELOCKED);
+	ADD_TOKEN(u8,      cmd, write_locked);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building SET command.\n");
+		return err;
+	}
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+
+static int lock_unlock_locking_range_SUM(struct opal_dev *dev)
+{
+	u8 lr_buffer[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	const u8 *method;
+	struct opal_lock_unlock *lkul;
+	int ret;
+	u8 read_locked = 1, write_locked = 1;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	method = OPALMETHOD[OPAL_SET];
+	lkul = dev->func_data[dev->state - 1];
+	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
+				lkul->session.opal_key.lr) < 0)
+		return -ERANGE;
+
+	switch (lkul->l_state) {
+	case OPAL_RO:
+		read_locked = 0;
+		write_locked = 1;
+		break;
+	case OPAL_RW:
+		read_locked = 0;
+		write_locked = 0;
+		break;
+	case OPAL_LK:
+		/* vars are initalized to locked */
+		break;
+	default:
+		pr_err("Tried to set an invalid locking state.\n");
+		return 1;
+	}
+	ret = generic_lr_enable_disable(cmd, lr_buffer, 1, 1,
+					read_locked, write_locked);
+
+	if (ret < 0) {
+		pr_err("Error building SET command.\n");
+		return ret;
+	}
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+int activate_lsp(struct opal_dev *dev)
+{
+	u8 user_lr[OPAL_UID_LENGTH];
+	struct opal_cmd *cmd;
+	u8 uint_3 = 0x83;
+	int err = 0;
+	u8 *lr;
+
+	cmd = &dev->cmd;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	lr = dev->func_data[dev->state - 1];
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_LOCKINGSP_UID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_ACTIVATE]);
+	/* Activating as SUM */
+	if (*lr > 0) {
+		err = build_locking_range(user_lr, sizeof(user_lr), *lr);
+		if (err)
+			return err;
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+		ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+		ADD_TOKEN(u8,      cmd, uint_3);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_06);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_00);
+		ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_00);
+
+		ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+		ADD_TOKEN(bytestr, cmd, user_lr);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	} else {
+		ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+		ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	}
+
+	if (err) {
+		pr_err("Error building Activate LockingSP command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, parse_and_check_status);
+}
+
+static int get_lsp_lifecycle_cont(struct opal_dev *dev)
+{
+	u8 lc_status;
+	int error = 0;
+
+	error = parse_and_check_status(dev);
+	if (error)
+		return error;
+
+	lc_status = response_get_u64(&dev->parsed, 4);
+	/* 0x08 is Manufacured Inactive */
+	/* 0x09 is Manufactured */
+	if (lc_status != 0x08) {
+		pr_err("Couldn't determine the status of the Lifcycle state\n");
+		return -ENODEV;
+	}
+
+err_return:
+	return 0;
+}
+
+/* Determine if we're in the Manufactured Inactive or Active state */
+int get_lsp_lifecycle(struct opal_dev *dev)
+{
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_LOCKINGSP_UID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_GET]);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* Start Column */
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_06); /* Lifecycle Column */
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_04); /* End Column */
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_06); /* Lifecycle Column */
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error Building GET Lifecycle Status command\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, get_lsp_lifecycle_cont);
+}
+
+static int get_msid_cpin_pin_cont(struct opal_dev *dev)
+{
+	const char *msid_pin;
+	size_t strlen;
+	int error = 0;
+
+	error = parse_and_check_status(dev);
+	if (error)
+		return error;
+
+	strlen = response_get_string(&dev->parsed, 4, &msid_pin);
+	if (!msid_pin) {
+		pr_err("%s: Couldn't extract PIN from response\n", __func__);
+		return 11;
+	}
+
+	dev->prev_data = kmemdup(msid_pin, strlen, GFP_KERNEL);
+	if (!dev->prev_data)
+		return -ENOMEM;
+
+	dev->prev_d_len = strlen;
+
+ err_return:
+	return 0;
+}
+
+static int get_msid_cpin_pin(struct opal_dev *dev)
+{
+	struct opal_cmd *cmd;
+	int err = 0;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+	set_comID(cmd, dev->comID);
+
+
+	ADD_TOKEN(u8,      cmd, OPAL_CALL);
+	ADD_TOKEN(bytestr, cmd, OPALUID[OPAL_C_PIN_MSID]);
+	ADD_TOKEN(bytestr, cmd, OPALMETHOD[OPAL_GET]);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_STARTLIST);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* Start Column */
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* PIN */
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_STARTNAME);
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_04); /* End Column */
+	ADD_TOKEN(u8,      cmd, OPAL_TINY_UINT_03); /* Lifecycle Column */
+	ADD_TOKEN(u8,      cmd, OPAL_ENDNAME);
+
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+	ADD_TOKEN(u8,      cmd, OPAL_ENDLIST);
+
+	if (err) {
+		pr_err("Error building Get MSID CPIN PIN command.\n");
+		return err;
+	}
+
+	return finalize_and_send(dev, cmd, get_msid_cpin_pin_cont);
+}
+
+static int build_end_opal_session(struct opal_dev *dev)
+{
+	struct opal_cmd *cmd;
+
+	cmd = &dev->cmd;
+	clear_opal_cmd(cmd);
+
+	set_comID(cmd, dev->comID);
+	return test_and_add_token_u8(cmd, OPAL_ENDOFSESSION);
+}
+
+static int end_opal_session(struct opal_dev *dev)
+{
+	int ret = build_end_opal_session(dev);
+
+	if (ret < 0)
+		return ret;
+	return finalize_and_send(dev, &dev->cmd, end_session_cont);
+}
+
+const opal_step error_end_session[] = {
+	end_opal_session,
+	NULL,
+};
+
+static int end_opal_session_error(struct opal_dev *dev)
+{
+	dev->funcs = error_end_session;
+	dev->state = 0;
+	dev->error_cb = NULL;
+	return next(dev);
+}
+
+struct opal_dev *alloc_opal_dev(struct request_queue *q)
+{
+	struct opal_dev *opal_dev;
+	unsigned long dma_align;
+	struct opal_cmd *cmd;
+
+	opal_dev = kzalloc(sizeof(*opal_dev), GFP_KERNEL);
+	if (!opal_dev)
+		return opal_dev;
+
+	cmd = &opal_dev->cmd;
+	cmd->cmd = cmd->cmd_buf;
+	cmd->resp = cmd->resp_buf;
+
+	dma_align = (queue_dma_alignment(q) | q->dma_pad_mask) + 1;
+	cmd->cmd = (u8 *)round_up((uintptr_t)cmd->cmd, dma_align);
+	cmd->resp = (u8 *)round_up((uintptr_t)cmd->resp, dma_align);
+
+	INIT_LIST_HEAD(&opal_dev->unlk_lst);
+
+	opal_dev->state = 0;
+
+	mutex_init(&opal_dev->dev_lock);
+
+	return opal_dev;
+
+}
+EXPORT_SYMBOL(alloc_opal_dev);
+
+static int do_cmds(struct opal_dev *dev)
+{
+	int ret;
+	ret = next(dev);
+	opal_dev_put(dev);
+	return ret;
+}
+
+static struct opal_dev *get_opal_dev(struct sed_context *sedc,
+				     const opal_step *funcs)
+{
+	struct opal_dev *dev = sedc->dev;
+	if (dev) {
+		dev->state = 0;
+		dev->funcs = funcs;
+		dev->TSN = 0;
+		dev->HSN = 0;
+		dev->error_cb = end_opal_session_error;
+		dev->error_cb_data = dev;
+		dev->func_data = NULL;
+		dev->sed_ctx = sedc;
+		opal_dev_get(dev);
+	}
+	return dev;
+}
+
+int opal_secure_erase_locking_range(struct sed_context *sedc, struct sed_key *key)
+{
+	struct opal_dev *dev;
+	void *data[3] = { NULL };
+	const opal_step erase_funcs[] = {
+		opal_discovery0,
+		start_auth_opal_session,
+		get_active_key,
+		gen_key,
+		end_opal_session,
+		NULL,
+	};
+
+	dev = get_opal_dev(sedc, erase_funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = &key->opal_session;
+	dev->func_data[2] = &key->opal_session.opal_key.lr;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_secure_erase_locking_range);
+
+int opal_erase_locking_range(struct sed_context *sedc, struct sed_key *key)
+{
+	struct opal_dev *dev;
+	void *data[3] = { NULL };
+	const opal_step erase_funcs[] = {
+		opal_discovery0,
+		start_auth_opal_session,
+		erase_locking_range,
+		end_opal_session,
+		NULL,
+	};
+
+	dev = get_opal_dev(sedc, erase_funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = &key->opal_session;
+	dev->func_data[2] = &key->opal_session;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_erase_locking_range);
+
+int opal_enable_disable_shadow_mbr(struct sed_context *sedc,
+				   struct sed_key *key)
+{
+	void *func_data[6] = { NULL };
+	struct opal_dev *dev;
+	const opal_step mbr_funcs[] = {
+		opal_discovery0,
+		start_admin1LSP_opal_session,
+		set_mbr_done,
+		end_opal_session,
+		start_admin1LSP_opal_session,
+		set_mbr_enable_disable,
+		end_opal_session,
+		NULL,
+	};
+
+	if (key->opal_mbr.enable_disable != OPAL_MBR_ENABLE &&
+	    key->opal_mbr.enable_disable != OPAL_MBR_DISABLE)
+		return -EINVAL;
+
+	dev = get_opal_dev(sedc, mbr_funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = func_data;
+	dev->func_data[1] = &key->opal_mbr.key;
+	dev->func_data[2] = &key->opal_mbr.enable_disable;
+	dev->func_data[4] = &key->opal_mbr.key;
+	dev->func_data[5] = &key->opal_mbr.enable_disable;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_enable_disable_shadow_mbr);
+
+int opal_save(struct sed_context *sedc, struct sed_key *key)
+{
+	struct opal_suspend_data *suspend;
+	struct opal_dev *dev;
+	int ret;
+
+	dev = get_opal_dev(sedc, NULL);
+	if (!dev)
+		return -ENODEV;
+	suspend = kzalloc(sizeof(*suspend), GFP_KERNEL);
+	if(!suspend)
+		return -ENOMEM;
+
+	suspend->unlk = key->opal_lk_unlk;
+	suspend->lr = key->opal_lk_unlk.session.opal_key.lr;
+	ret = add_suspend_info(dev, suspend);
+	opal_dev_put(dev);
+	return ret;
+}
+EXPORT_SYMBOL(opal_save);
+
+int opal_add_user_to_lr(struct sed_context *sedc, struct sed_key *key)
+{
+	void *func_data[3] = { NULL };
+	struct opal_dev *dev;
+		const opal_step funcs[] = {
+		opal_discovery0,
+		start_admin1LSP_opal_session,
+		add_user_to_lr,
+		end_opal_session,
+		NULL
+	};
+
+	if (key->opal_lk_unlk.l_state != OPAL_RO &&
+	    key->opal_lk_unlk.l_state != OPAL_RW) {
+		pr_err("Locking state was not RO or RW\n");
+		return -EINVAL;
+	}
+	if (key->opal_lk_unlk.session.who < OPAL_USER1 &&
+	    key->opal_lk_unlk.session.who > OPAL_USER9) {
+		pr_err("Authority was not within the range of users: %d\n",
+		       key->opal_lk_unlk.session.who);
+		return -EINVAL;
+	}
+	if (key->opal_lk_unlk.session.SUM) {
+		pr_err("%s not supported in SUM. Use setup locking range\n",
+		       __func__);
+		return -EINVAL;
+	}
+
+	dev = get_opal_dev(sedc, funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = func_data;
+	dev->func_data[1] = &key->opal_lk_unlk.session.opal_key;
+	dev->func_data[2] = &key->opal_lk_unlk;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_add_user_to_lr);
+
+int opal_reverttper(struct sed_context *sedc, struct sed_key *key)
+{
+	void *data[2] = { NULL };
+	const opal_step revert_funcs[] = {
+		opal_discovery0,
+		start_SIDASP_opal_session,
+		revert_tper, /* controller will terminate session */
+		NULL,
+	};
+	struct opal_dev *dev = get_opal_dev(sedc, revert_funcs);
+
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = &key->opal;
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_reverttper);
+
+/* These are global'd because both lock_unlock_internal
+ * and opal_unlock_from_suspend need them.
+ */
+const opal_step ulk_funcs_SUM[] = {
+	opal_discovery0,
+	start_auth_opal_session,
+	lock_unlock_locking_range_SUM,
+	end_opal_session,
+	NULL
+};
+const opal_step _unlock_funcs[] = {
+	opal_discovery0,
+	start_auth_opal_session,
+	lock_unlock_locking_range,
+	end_opal_session,
+	NULL
+};
+int opal_lock_unlock(struct sed_context *sedc, struct sed_key *key)
+{
+	void *func_data[3] = { NULL };
+	struct opal_dev *dev;
+
+	if (key->opal_lk_unlk.session.who < OPAL_ADMIN1 ||
+	    key->opal_lk_unlk.session.who > OPAL_USER9)
+		return -EINVAL;
+
+	dev = get_opal_dev(sedc, NULL);
+	if (!dev)
+		return -ENODEV;
+
+	if (key->opal_lk_unlk.session.SUM)
+		dev->funcs = _unlock_funcs;//ulk_funcs_SUM;
+	else
+		dev->funcs = _unlock_funcs;
+
+	dev->func_data = func_data;
+	dev->func_data[1] = &key->opal_lk_unlk.session;
+	dev->func_data[2] = &key->opal_lk_unlk;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_lock_unlock);
+
+int opal_take_ownership(struct sed_context *sedc, struct sed_key *key)
+{
+	const opal_step owner_funcs[] = {
+		opal_discovery0,
+		start_anybodyASP_opal_session,
+		get_msid_cpin_pin,
+		end_opal_session,
+		start_SIDASP_opal_session,
+		set_sid_cpin_pin,
+		end_opal_session,
+		NULL
+	};
+	void *data[6] = { NULL };
+	struct opal_dev *dev = get_opal_dev(sedc, owner_funcs);
+
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[4] = &key->opal;
+	dev->func_data[5] = &key->opal;
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_take_ownership);
+
+int opal_activate_lsp(struct sed_context *sedc, struct sed_key *key)
+{
+	void *data[4] = { NULL };
+	const opal_step active_funcs[] = {
+		opal_discovery0,
+		start_SIDASP_opal_session, /* Open session as SID auth */
+		get_lsp_lifecycle,
+		activate_lsp,
+		end_opal_session,
+		NULL
+	};
+	struct opal_dev *dev = get_opal_dev(sedc, active_funcs);
+
+	if (!dev)
+		return -ENODEV;
+	dev->func_data = data;
+	dev->func_data[1] = &key->opal;
+	dev->func_data[3] = &key->opal.lr;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_activate_lsp);
+
+int opal_setup_locking_range(struct sed_context *sedc, struct sed_key *pw)
+{
+	void *data[3] = { NULL };
+	const opal_step lr_funcs[] = {
+		opal_discovery0,
+		start_auth_opal_session,
+		setup_locking_range,
+		end_opal_session,
+		NULL,
+	};
+	struct opal_dev *dev = get_opal_dev(sedc, lr_funcs);
+
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = &pw->opal_lrs.session;
+	dev->func_data[2] = &pw->opal_lrs;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_setup_locking_range);
+
+int opal_set_new_pw(struct sed_context *sedc, struct sed_key *pw)
+{
+	const opal_step pw_funcs[] = {
+		opal_discovery0,
+		start_auth_opal_session,
+		set_new_pw,
+		end_opal_session,
+		NULL
+	};
+	struct opal_dev *dev;
+	void *data[3] = { NULL };
+
+	if (pw->sed_type != OPAL_PW)
+		return -EINVAL;
+
+	if (pw->opal_pw.session.who < OPAL_ADMIN1 ||
+	    pw->opal_pw.session.who > OPAL_USER9  ||
+	    pw->opal_pw.new_user_pw.who < OPAL_ADMIN1 ||
+	    pw->opal_pw.new_user_pw.who > OPAL_USER9)
+		return -EINVAL;
+
+	dev = get_opal_dev(sedc, pw_funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = (void *) &pw->opal_pw.session;
+	dev->func_data[2] = (void *) &pw->opal_pw.new_user_pw;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_set_new_pw);
+
+int opal_activate_user(struct sed_context *sedc, struct sed_key *pw)
+{
+	const opal_step act_funcs[] = {
+		opal_discovery0,
+		start_admin1LSP_opal_session,
+		internal_activate_user,
+		end_opal_session,
+		NULL
+	};
+	struct opal_dev *dev;
+	void *data[3] = { NULL };
+
+	if (pw->sed_type != OPAL_ACT_USR) {
+		pr_err("Sed type was not act user\n");
+		return -EINVAL;
+	}
+
+	/* We can't activate Admin1 it's active as manufactured */
+	if (pw->opal_session.who < OPAL_USER1 &&
+	    pw->opal_session.who > OPAL_USER9) {
+		pr_err("Who was not a valid user: %d \n", pw->opal_session.who);
+		return -EINVAL;
+	}
+
+	dev = get_opal_dev(sedc, act_funcs);
+	if (!dev)
+		return -ENODEV;
+
+	dev->func_data = data;
+	dev->func_data[1] = &pw->opal_session.opal_key;
+	dev->func_data[2] = &pw->opal_session;
+
+	return do_cmds(dev);
+}
+EXPORT_SYMBOL(opal_activate_user);
+
+int opal_unlock_from_suspend(struct sed_context *sedc)
+{
+	struct opal_suspend_data *suspend;
+	void *func_data[3] = { NULL };
+	bool was_failure = false;
+	struct opal_dev *dev = get_opal_dev(sedc, NULL);
+	int ret = 0;
+
+	if (!dev)
+		return 0;
+
+	dev->func_data = func_data;
+	dev->error_cb = end_opal_session_error;
+	dev->error_cb_data = dev;
+
+	if (!list_empty(&dev->unlk_lst)) {
+		list_for_each_entry(suspend, &dev->unlk_lst, node) {
+			dev->state = 0;
+			dev->func_data[1] = &suspend->unlk.session;
+			dev->func_data[2] = &suspend->unlk;
+			if (suspend->unlk.session.SUM)
+				dev->funcs = ulk_funcs_SUM;
+			else
+				dev->funcs = _unlock_funcs;
+			dev->TSN = 0;
+			dev->HSN = 0;
+			ret = next(dev);
+			if (ret)
+				was_failure = true;
+		}
+	}
+	opal_dev_put(dev);
+	return was_failure ? 1 : 0;
+}
+EXPORT_SYMBOL(opal_unlock_from_suspend);
diff --git a/lib/sed-opal_internal.h b/lib/sed-opal_internal.h
new file mode 100644
index 0000000..12369eb
--- /dev/null
+++ b/lib/sed-opal_internal.h
@@ -0,0 +1,601 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef _NVME_OPAL_INTERNAL_H
+#define _NVME_OPAL_INTERNAL_H
+
+#include <linux/key-type.h>
+#include <keys/user-type.h>
+
+#define DTAERROR_NO_METHOD_STATUS 0x89
+#define GENERIC_HOST_SESSION_NUM 0x41
+
+
+
+/*
+ * Derived from:
+ * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * Section: 5.1.5 Method Status Codes
+ */
+static const char *opal_errors[] = {
+	"Success",
+	"Not Authorized",
+	"Unknown Error",
+	"SP Busy",
+	"SP Failed",
+	"SP Disabled",
+	"SP Frozen",
+	"No Sessions Available",
+	"Uniqueness Conflict",
+	"Insufficient Space",
+	"Insufficient Rows",
+	"Invalid Function",
+	"Invalid Parameter",
+	"Invalid Reference",
+	"Unknown Error",
+	"TPER Malfunction",
+	"Transaction Failure",
+	"Response Overflow",
+	"Authority Locked Out",
+};
+
+static const char *opal_error_to_human(int error)
+{
+	if (error == 0x3f)
+		return "Failed";
+
+	if (error >= ARRAY_SIZE(opal_errors) || error < 0)
+		return "Unknown Error";
+
+	return opal_errors[error];
+}
+
+/*
+ * User IDs used in the TCG storage SSCs
+ * Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * Section: 6.3 Assigned UIDs
+ */
+static const u8 OPALUID[][8] = {
+	/* users */
+
+	/* session management  */
+	{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff},
+	/* special "thisSP" syntax */
+	{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 },
+	/* Administrative SP */
+	{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x01 },
+	/* Locking SP */
+	{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x02 },
+	/* ENTERPRISE Locking SP  */
+	{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x01, 0x00, 0x01 },
+	/* anybody */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x01 },
+	/* SID */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x06 },
+	/* ADMIN1 */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0x00, 0x01 },
+	/* USER1 */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x01 },
+	/* USER2 */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x02 },
+	/* PSID user */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0xff, 0x01 },
+	/* BandMaster 0 */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x80, 0x01 },
+	 /* EraseMaster */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x84, 0x01 },
+
+	/* tables */
+
+	/* Locking_GlobalRange */
+	{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
+	/* ACE_Locking_Range_Set_RdLocked UID */
+	{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE0, 0x01 },
+	/* ACE_Locking_Range_Set_WrLocked UID */
+	{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE8, 0x01 },
+	/* MBR Control */
+	{ 0x00, 0x00, 0x08, 0x03, 0x00, 0x00, 0x00, 0x01 },
+	/* Shadow MBR */
+	{ 0x00, 0x00, 0x08, 0x04, 0x00, 0x00, 0x00, 0x00 },
+	/* AUTHORITY_TABLE */
+	{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00},
+	/* C_PIN_TABLE */
+	{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x00},
+	/* OPAL Locking Info */
+	{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x01 },
+	/* Enterprise Locking Info */
+	{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x00 },
+
+	/* C_PIN_TABLE object ID's */
+
+	/* C_PIN_MSID */
+	{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x84, 0x02},
+	/* C_PIN_SID */
+	{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x01},
+	 /* C_PIN_ADMIN1 */
+	{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x01, 0x00, 0x01},
+
+	/* half UID's (only first 4 bytes used) */
+
+	/* Half-UID – Authority_object_ref */
+	{ 0x00, 0x00, 0x0C, 0x05, 0xff, 0xff, 0xff, 0xff },
+	/* Half-UID – Boolean ACE */
+	{ 0x00, 0x00, 0x04, 0x0E, 0xff, 0xff, 0xff, 0xff },
+
+	/* special value for omitted optional parameter */
+
+	/* HEXFF for omitted */
+	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
+};
+static const size_t OPAL_UID_LENGTH = 8;
+static const size_t OPAL_MSID_KEYLEN = 15;
+static const size_t OPAL_UID_LENGTH_HALF = 4;
+
+
+/* Enum to index OPALUID array */
+enum OPAL_UID {
+	/* users */
+	OPAL_SMUID_UID,
+	OPAL_THISSP_UID,
+	OPAL_ADMINSP_UID,
+	OPAL_LOCKINGSP_UID,
+	OPAL_ENTERPRISE_LOCKINGSP_UID,
+	OPAL_ANYBODY_UID,
+	OPAL_SID_UID,
+	OPAL_ADMIN1_UID,
+	OPAL_USER1_UID,
+	OPAL_USER2_UID,
+	OPAL_PSID_UID,
+	OPAL_ENTERPRISE_BANDMASTER0_UID,
+	OPAL_ENTERPRISE_ERASEMASTER_UID,
+	/* tables */
+	OPAL_LOCKINGRANGE_GLOBAL,
+	OPAL_LOCKINGRANGE_ACE_RDLOCKED,
+	OPAL_LOCKINGRANGE_ACE_WRLOCKED,
+	OPAL_MBRCONTROL,
+	OPAL_MBR,
+	OPAL_AUTHORITY_TABLE,
+	OPAL_C_PIN_TABLE,
+	OPAL_LOCKING_INFO_TABLE,
+	OPAL_ENTERPRISE_LOCKING_INFO_TABLE,
+	/* C_PIN_TABLE object ID's */
+	OPAL_C_PIN_MSID,
+	OPAL_C_PIN_SID,
+	OPAL_C_PIN_ADMIN1,
+	/* half UID's (only first 4 bytes used) */
+	OPAL_HALF_UID_AUTHORITY_OBJ_REF,
+	OPAL_HALF_UID_BOOLEAN_ACE,
+	/* omitted optional parameter */
+	OPAL_UID_HEXFF,
+};
+
+/*
+ * TCG Storage SSC Methods.
+ * Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * Section: 6.3 Assigned UIDs
+ */
+static const u8 OPALMETHOD[][8] = {
+	/* Properties */
+	{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x01 },
+	/* STARTSESSION */
+	{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x02 },
+	/* Revert */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x02, 0x02 },
+	/* Activate */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x02, 0x03 },
+	/* Enterprise Get */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x06 },
+	/* Enterprise Set */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x07 },
+	/* NEXT */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x08 },
+	/* Enterprise Authenticate */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x0c },
+	/* GetACL */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x0d },
+	/* GenKey */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x10 },
+	/* revertSP */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x11 },
+	/* Get */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x16 },
+	/* Set */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x17 },
+	/* Authenticate */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x1c },
+	/* Random */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x06, 0x01 },
+	/* Erase */
+	{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
+};
+static const size_t OPAL_METHOD_LENGTH = 8;
+
+/* Enum for indexing the OPALMETHOD array */
+enum OPAL_METHOD {
+	OPAL_PROPERTIES,
+	OPAL_STARTSESSION,
+	OPAL_REVERT,
+	OPAL_ACTIVATE,
+	OPAL_EGET,
+	OPAL_ESET,
+	OPAL_NEXT,
+	OPAL_EAUTHENTICATE,
+	OPAL_GETACL,
+	OPAL_GENKEY,
+	OPAL_REVERTSP,
+	OPAL_GET,
+	OPAL_SET,
+	OPAL_AUTHENTICATE,
+	OPAL_RANDOM,
+	OPAL_ERASE,
+};
+
+/*
+ * Token defs derived from:
+ * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * 3.2.2 Data Stream Encoding
+ */
+enum OPAL_RESPONSE_TOKEN {
+	OPAL_DTA_TOKENID_BYTESTRING = 0xe0,
+	OPAL_DTA_TOKENID_SINT = 0xe1,
+	OPAL_DTA_TOKENID_UINT = 0xe2,
+	OPAL_DTA_TOKENID_TOKEN = 0xe3, /* actual token is returned */
+	OPAL_DTA_TOKENID_INVALID = 0X0
+};
+
+enum OPAL_TOKEN {
+	/* Boolean */
+	OPAL_TRUE = 0x01,
+	OPAL_FALSE = 0x00,
+	OPAL_BOOLEAN_EXPR = 0x03,
+	/* cellblocks */
+	OPAL_TABLE = 0x00,
+	OPAL_STARTROW = 0x01,
+	OPAL_ENDROW = 0x02,
+	OPAL_STARTCOLUMN = 0x03,
+	OPAL_ENDCOLUMN = 0x04,
+	OPAL_VALUES = 0x01,
+	/* authority table */
+	OPAL_PIN = 0x03,
+	/* locking tokens */
+	OPAL_RANGESTART = 0x03,
+	OPAL_RANGELENGTH = 0x04,
+	OPAL_READLOCKENABLED = 0x05,
+	OPAL_WRITELOCKENABLED = 0x06,
+	OPAL_READLOCKED = 0x07,
+	OPAL_WRITELOCKED = 0x08,
+	OPAL_ACTIVEKEY = 0x0A,
+	/* locking info table */
+	OPAL_MAXRANGES = 0x04,
+	 /* mbr control */
+	OPAL_MBRENABLE = 0x01,
+	OPAL_MBRDONE = 0x02,
+	/* properties */
+	OPAL_HOSTPROPERTIES = 0x00,
+	/* atoms */
+	OPAL_STARTLIST = 0xf0,
+	OPAL_ENDLIST = 0xf1,
+	OPAL_STARTNAME = 0xf2,
+	OPAL_ENDNAME = 0xf3,
+	OPAL_CALL = 0xf8,
+	OPAL_ENDOFDATA = 0xf9,
+	OPAL_ENDOFSESSION = 0xfa,
+	OPAL_STARTTRANSACTON = 0xfb,
+	OPAL_ENDTRANSACTON = 0xfC,
+	OPAL_EMPTYATOM = 0xff,
+	OPAL_WHERE = 0x00,
+};
+
+/* Useful tiny atoms.
+ * Useful for table columns etc
+ */
+enum OPAL_TINY_ATOM {
+	OPAL_TINY_UINT_00 = 0x00,
+	OPAL_TINY_UINT_01 = 0x01,
+	OPAL_TINY_UINT_02 = 0x02,
+	OPAL_TINY_UINT_03 = 0x03,
+	OPAL_TINY_UINT_04 = 0x04,
+	OPAL_TINY_UINT_05 = 0x05,
+	OPAL_TINY_UINT_06 = 0x06,
+	OPAL_TINY_UINT_07 = 0x07,
+	OPAL_TINY_UINT_08 = 0x08,
+	OPAL_TINY_UINT_09 = 0x09,
+	OPAL_TINY_UINT_10 = 0x0a,
+	OPAL_TINY_UINT_11 = 0x0b,
+	OPAL_TINY_UINT_12 = 0x0c,
+	OPAL_TINY_UINT_13 = 0x0d,
+	OPAL_TINY_UINT_14 = 0x0e,
+	OPAL_TINY_UINT_15 = 0x0f,
+};
+
+enum OPAL_ATOM_WIDTH {
+	OPAL_WIDTH_TINY,
+	OPAL_WIDTH_SHORT,
+	OPAL_WIDTH_MEDIUM,
+	OPAL_WIDTH_LONG,
+	OPAL_WIDTH_TOKEN
+};
+
+/* Locking state for a locking range */
+enum OPAL_LOCKINGSTATE {
+	OPAL_LOCKING_READWRITE = 0x01,
+	OPAL_LOCKING_READONLY = 0x02,
+	OPAL_LOCKING_LOCKED = 0x03,
+};
+
+/*
+ * Structures to build and decode the Opal SSC messages
+ * fields that are NOT really numeric are defined as u8[] to
+ * help reduce the endianness issues
+ */
+
+/* Packets derived from:
+ * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * Secion: 3.2.3 ComPackets, Packets & Subpackets
+ */
+
+/* Comm Packet (header) for transmissions. */
+struct opal_compacket {
+	u32 reserved0;
+	u8 extendedComID[4];
+	u32 outstandingData;
+	u32 minTransfer;
+	u32 length;
+};
+
+/* Packet structure. */
+struct opal_packet {
+	u32 TSN;
+	u32 HSN;
+	u32 seq_number;
+	u16 reserved0;
+	u16 ack_type;
+	u32 acknowledgment;
+	u32 length;
+};
+
+/* Data sub packet header */
+struct opal_data_subpacket {
+	u8 reserved0[6];
+	u16 kind;
+	u32 length;
+};
+
+/* header of a response */
+struct opal_header {
+	struct opal_compacket cp;
+	struct opal_packet pkt;
+	struct opal_data_subpacket subpkt;
+};
+
+#define FC_TPER       0x0001
+#define FC_LOCKING    0x0002
+#define FC_GEOMETRY   0x0003
+#define FC_ENTERPRISE 0x0100
+#define FC_DATASTORE  0x0202
+#define FC_SINGLEUSER 0x0201
+#define FC_OPALV100   0x0200
+#define FC_OPALV200   0x0203
+
+/*
+ * The Discovery 0 Header. As defined in
+ * Opal SSC Documentation
+ * Section: 3.3.5 Capability Discovery
+ */
+struct d0_header {
+	u32 length; /* the length of the header 48 in 2.00.100 */
+	u32 revision; /**< revision of the header 1 in 2.00.100 */
+	u32 reserved01;
+	u32 reserved02;
+	/*
+	 * the remainder of the structure is vendor specific and will not be
+	 * addressed now
+	 */
+	u8 ignored[32];
+};
+
+/*
+ * TPer Feature Descriptor. Contains flags indicating support for the
+ * TPer features described in the OPAL specification. The names match the
+ * OPAL terminology
+ *
+ * code == 0x001 in 2.00.100
+ */
+struct d0_tper_features {
+	/*
+	 * supported_features bits:
+	 * bit 7: reserved
+	 * bit 6: com ID management
+	 * bit 5: reserved
+	 * bit 4: streaming support
+	 * bit 3: buffer management
+	 * bit 2: ACK/NACK
+	 * bit 1: async
+	 * bit 0: sync
+	 */
+	u8 supported_features;
+	/*
+	 * bytes 5 through 15 are reserved, but we represent the first 3 as
+	 * u8 to keep the other two 32bits integers aligned.
+	 */
+	u8 reserved01[3];
+	u32 reserved02;
+	u32 reserved03;
+};
+
+/*
+ * Locking Feature Descriptor. Contains flags indicating support for the
+ * locking features described in the OPAL specification. The names match the
+ * OPAL terminology
+ *
+ * code == 0x0002 in 2.00.100
+ */
+struct d0_locking_features {
+	/*
+	 * supported_features bits:
+	 * bits 6-7: reserved
+	 * bit 5: MBR done
+	 * bit 4: MBR enabled
+	 * bit 3: media encryption
+	 * bit 2: locked
+	 * bit 1: locking enabled
+	 * bit 0: locking supported
+	 */
+	u8 supported_features;
+	/*
+	 * bytes 5 through 15 are reserved, but we represent the first 3 as
+	 * u8 to keep the other two 32bits integers aligned.
+	 */
+	u8 reserved01[3];
+	u32 reserved02;
+	u32 reserved03;
+};
+
+/*
+ * Geometry Feature Descriptor. Contains flags indicating support for the
+ * geometry features described in the OPAL specification. The names match the
+ * OPAL terminology
+ *
+ * code == 0x0003 in 2.00.100
+ */
+struct d0_geometry_features {
+	/*
+	 * skip 32 bits from header, needed to align the struct to 64 bits.
+	 */
+	u8 header[4];
+	/*
+	 * reserved01:
+	 * bits 1-6: reserved
+	 * bit 0: align
+	 */
+	u8 reserved01;
+	u8 reserved02[7];
+	u32 logical_block_size;
+	u64 alignment_granularity;
+	u64 lowest_aligned_lba;
+};
+
+/*
+ * Enterprise SSC Feature
+ *
+ * code == 0x0100
+ */
+struct d0_enterprise_ssc {
+	u16 baseComID;
+	u16 numComIDs;
+	/* range_crossing:
+	 * bits 1-6: reserved
+	 * bit 0: range crossing
+	 */
+	u8 range_crossing;
+	u8 reserved01;
+	u16 reserved02;
+	u32 reserved03;
+	u32 reserved04;
+};
+
+/*
+ * Opal V1 feature
+ *
+ * code == 0x0200
+ */
+struct d0_opal_v100 {
+	u16 baseComID;
+	u16 numComIDs;
+};
+
+/*
+ * Single User Mode feature
+ *
+ * code == 0x0201
+ */
+struct d0_single_user_mode {
+	u32 num_locking_objects;
+	/* reserved01:
+	 * bit 0: any
+	 * bit 1: all
+	 * bit 2: policy
+	 * bits 3-7: reserved
+	 */
+	u8 reserved01;
+	u8 reserved02;
+	u16 reserved03;
+	u32 reserved04;
+};
+
+/*
+ * Additonal Datastores feature
+ *
+ * code == 0x0202
+ */
+struct d0_datastore_table {
+	u16 reserved01;
+	u16 max_tables;
+	u32 max_size_tables;
+	u32 table_size_alignment;
+};
+
+/*
+ * OPAL 2.0 feature
+ *
+ * code == 0x0203
+ */
+struct d0_opal_v200 {
+	u16 baseComID;
+	u16 numComIDs;
+	/* range_crossing:
+	 * bits 1-6: reserved
+	 * bit 0: range crossing
+	 */
+	u8 range_crossing;
+	/* num_locking_admin_auth:
+	 * not aligned to 16 bits, so use two u8.
+	 * stored in big endian:
+	 * 0: MSB
+	 * 1: LSB
+	 */
+	u8 num_locking_admin_auth[2];
+	/* num_locking_user_auth:
+	 * not aligned to 16 bits, so use two u8.
+	 * stored in big endian:
+	 * 0: MSB
+	 * 1: LSB
+	 */
+	u8 num_locking_user_auth[2];
+	u8 initialPIN;
+	u8 revertedPIN;
+	u8 reserved01;
+	u32 reserved02;
+};
+
+/* Union of features used to parse the discovery 0 response */
+struct d0_features {
+	u16 code;
+	/*
+	 * r_version bits:
+	 * bits 4-7: version
+	 * bits 0-3: reserved
+	 */
+	u8 r_version;
+	u8 length;
+	u8 features[];
+};
+
+struct key *request_user_key(const char *master_desc, const u8 **master_key,
+			     size_t *master_keylen);
+
+#endif /* _NVME_OPAL_INTERNAL_H */
diff --git a/lib/sed.c b/lib/sed.c
new file mode 100644
index 0000000..3b43a52
--- /dev/null
+++ b/lib/sed.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Authors:
+ *    Rafael Antognolli <rafael.antognolli@intel.com>
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/blkdev.h>
+#include <linux/sed.h>
+#include <linux/sed-opal.h>
+#include <asm/uaccess.h>
+
+int sed_save(struct sed_context *sed_ctx, struct sed_key *key)
+{
+	switch (key->sed_type) {
+	case OPAL_LOCK_UNLOCK:
+		return opal_save(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_lock_unlock(struct sed_context *sed_ctx, struct sed_key *key)
+{
+	switch (key->sed_type) {
+	case OPAL_LOCK_UNLOCK:
+		return opal_lock_unlock(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_take_ownership(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL:
+		return opal_take_ownership(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_activate_lsp(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL:
+		return opal_activate_lsp(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_set_pw(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL_PW:
+		return opal_set_new_pw(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_activate_user(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL_ACT_USR:
+		return opal_activate_user(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_reverttper(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL:
+		return opal_reverttper(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_setup_locking_range(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL_LR_SETUP:
+		return opal_setup_locking_range(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_adduser_to_lr(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL_LOCK_UNLOCK:
+		return opal_add_user_to_lr(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_do_mbr(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL_MBR_DATA:
+		return opal_enable_disable_shadow_mbr(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_erase_lr(struct sed_context *sed_ctx, struct sed_key *key)
+{
+
+	switch (key->sed_type) {
+	case OPAL:
+		return opal_erase_locking_range(sed_ctx, key);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+int sed_secure_erase_lr(struct sed_context *sed_ctx, struct sed_key *key)
+{
+	switch (key->sed_type) {
+	case OPAL_ACT_USR:
+		return opal_secure_erase_locking_range(sed_ctx, key);
+
+	}
+	return -EOPNOTSUPP;
+}
+
+int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
+		   unsigned long arg)
+{
+	struct sed_key key;
+	struct sed_context *sed_ctx;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
+		return -ENODEV;
+
+	sed_ctx = filep->f_sedctx;
+
+	if (copy_from_user(&key, (void __user *)arg, sizeof(key)))
+		return -EFAULT;
+
+	switch (cmd) {
+	case IOC_SED_SAVE:
+		return sed_save(sed_ctx, &key);
+	case IOC_SED_LOCK_UNLOCK:
+		return sed_lock_unlock(sed_ctx, &key);
+	case IOC_SED_TAKE_OWNERSHIP:
+		return sed_take_ownership(sed_ctx, &key);
+	case IOC_SED_ACTIVATE_LSP:
+		return sed_activate_lsp(sed_ctx, &key);
+	case IOC_SED_SET_PW:
+		return sed_set_pw(sed_ctx, &key);
+	case IOC_SED_ACTIVATE_USR:
+		return sed_activate_user(sed_ctx, &key);
+	case IOC_SED_REVERT_TPR:
+		return sed_reverttper(sed_ctx, &key);
+	case IOC_SED_LR_SETUP:
+		return sed_setup_locking_range(sed_ctx, &key);
+	case IOC_SED_ADD_USR_TO_LR:
+		return sed_adduser_to_lr(sed_ctx, &key);
+	case IOC_SED_ENABLE_DISABLE_MBR:
+		return sed_do_mbr(sed_ctx, &key);
+	case IOC_SED_ERASE_LR:
+		return sed_erase_lr(sed_ctx, &key);
+	case IOC_SED_SECURE_ERASE_LR:
+		return sed_secure_erase_lr(sed_ctx, &key);
+	}
+	return -ENOTTY;
+}
+EXPORT_SYMBOL_GPL(fdev_sed_ioctl);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl
  2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
  2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
@ 2016-12-19 19:35 ` Scott Bauer
  2016-12-20  6:21   ` Christoph Hellwig
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
  2016-12-19 19:35 ` [PATCH v3 5/5] Maintainers: Add Information for SED Opal library Scott Bauer
  4 siblings, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi, Scott Bauer

Adds a new sed_context pointer to file struct, for char devs who wish
to suppor SED.
Adds ioctl handling code.

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
---
 fs/ioctl.c         | 3 +++
 include/linux/fs.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index cb9b029..77def42 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -15,6 +15,7 @@
 #include <linux/writeback.h>
 #include <linux/buffer_head.h>
 #include <linux/falloc.h>
+#include <linux/sed.h>
 #include "internal.h"
 
 #include <asm/ioctls.h>
@@ -679,6 +680,8 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 	default:
 		if (S_ISREG(inode->i_mode))
 			error = file_ioctl(filp, cmd, arg);
+		else if (is_sed_ioctl(cmd))
+			error = fdev_sed_ioctl(filp, cmd, arg);
 		else
 			error = vfs_ioctl(filp, cmd, arg);
 		break;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2ba0743..75a99e5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -55,6 +55,7 @@ struct workqueue_struct;
 struct iov_iter;
 struct fscrypt_info;
 struct fscrypt_operations;
+struct sed_context;
 
 extern void __init inode_init(void);
 extern void __init inode_init_early(void);
@@ -853,6 +854,7 @@ struct file {
 #ifdef CONFIG_SECURITY
 	void			*f_security;
 #endif
+	struct sed_context      *f_sedctx;
 	/* needed for tty driver, and maybe others */
 	void			*private_data;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
                   ` (2 preceding siblings ...)
  2016-12-19 19:35 ` [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl Scott Bauer
@ 2016-12-19 19:35 ` Scott Bauer
  2016-12-19 21:59   ` Keith Busch
                     ` (4 more replies)
  2016-12-19 19:35 ` [PATCH v3 5/5] Maintainers: Add Information for SED Opal library Scott Bauer
  4 siblings, 5 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi, Scott Bauer

This patch implements the necessary logic to unlock a SED
enabled device coming back from an S3.

The patch also implements the necessary logic to allocate the
appropriate opal_dev structures to support the OPAL protocol.

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
---
 drivers/nvme/host/core.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h |  8 +++++-
 drivers/nvme/host/pci.c  | 10 +++++++-
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b40cfb0..f9731ce 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -28,6 +28,8 @@
 #include <linux/t10-pi.h>
 #include <scsi/sg.h>
 #include <asm/unaligned.h>
+#include <linux/sed.h>
+#include <linux/sed-opal.h>
 
 #include "nvme.h"
 #include "fabrics.h"
@@ -762,6 +764,48 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	return status;
 }
 
+static int nvme_sec_submit(struct nvme_ctrl *ctrl, u16 spsp, u8 secp,
+			   void *buffer, size_t len, u8 opcode)
+{
+	struct nvme_command cmd = { 0 };
+	struct nvme_ns *ns = NULL;
+
+	mutex_lock(&ctrl->namespaces_mutex);
+	if (!list_empty(&ctrl->namespaces))
+		ns = list_first_entry(&ctrl->namespaces, struct nvme_ns, list);
+
+	mutex_unlock(&ctrl->namespaces_mutex);
+	if (!ns)
+		return -ENODEV;
+
+	cmd.common.opcode = opcode;
+	cmd.common.nsid = ns->ns_id;
+	cmd.common.cdw10[0] = cpu_to_le32(((u32)secp) << 24 | ((u32)spsp) << 8);
+	cmd.common.cdw10[1] = cpu_to_le32(len);
+
+	return __nvme_submit_sync_cmd(ctrl->admin_q, &cmd, NULL, buffer, len,
+				      ADMIN_TIMEOUT, NVME_QID_ANY, 1, 0);
+}
+
+static int nvme_sec_send(void *ctrl_data, u16 spsp, u8 secp,
+			 void *buf, size_t len)
+{
+	return nvme_sec_submit(ctrl_data, spsp, secp, buf, len,
+			       nvme_admin_security_send);
+}
+
+static int nvme_sec_recv(void *ctrl_data, u16 spsp, u8 secp,
+			 void *buf, size_t len)
+{
+	return nvme_sec_submit(ctrl_data, spsp, secp, buf, len,
+			       nvme_admin_security_recv);
+}
+
+static const struct sec_ops nvme_sec_ops = {
+	.sec_send = nvme_sec_send,
+	.sec_recv = nvme_sec_recv,
+};
+
 static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 		unsigned int cmd, unsigned long arg)
 {
@@ -1051,6 +1095,28 @@ static const struct pr_ops nvme_pr_ops = {
 	.pr_clear	= nvme_pr_clear,
 };
 
+int nvme_opal_initialize(struct nvme_ctrl *ctrl)
+{
+	/* Opal dev has already been allocated for this controller */
+	if (ctrl->sed_ctx.dev)
+		return 0;
+
+	ctrl->sed_ctx.dev = alloc_opal_dev(ctrl->admin_q);
+	if (!ctrl->sed_ctx.dev)
+		return -ENOMEM;
+	ctrl->sed_ctx.ops = &nvme_sec_ops;
+	ctrl->sed_ctx.sec_data = ctrl;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nvme_opal_initialize);
+
+void nvme_unlock_from_suspend(struct nvme_ctrl *ctrl)
+{
+	if (opal_unlock_from_suspend(&ctrl->sed_ctx))
+		pr_warn("Failed to unlock one or more locking ranges!\n");
+}
+EXPORT_SYMBOL_GPL(nvme_unlock_from_suspend);
+
 static const struct block_device_operations nvme_fops = {
 	.owner		= THIS_MODULE,
 	.ioctl		= nvme_ioctl,
@@ -1312,6 +1378,7 @@ static int nvme_dev_open(struct inode *inode, struct file *file)
 		if (!kref_get_unless_zero(&ctrl->kref))
 			break;
 		file->private_data = ctrl;
+		file->f_sedctx = &ctrl->sed_ctx;
 		ret = 0;
 		break;
 	}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bd53214..851830b 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -19,6 +19,7 @@
 #include <linux/kref.h>
 #include <linux/blk-mq.h>
 #include <linux/lightnvm.h>
+#include <linux/sed.h>
 
 enum {
 	/*
@@ -151,6 +152,8 @@ struct nvme_ctrl {
 	struct work_struct async_event_work;
 	struct delayed_work ka_work;
 
+	struct sed_context sed_ctx;
+
 	/* Fabrics only */
 	u16 sqsize;
 	u32 ioccsz;
@@ -256,7 +259,8 @@ static inline int nvme_error_status(u16 status)
 
 static inline bool nvme_req_needs_retry(struct request *req, u16 status)
 {
-	return !(status & NVME_SC_DNR || blk_noretry_request(req)) &&
+	return !(status & NVME_SC_DNR || status & NVME_SC_ACCESS_DENIED ||
+		 blk_noretry_request(req)) &&
 		(jiffies - req->start_time) < req->timeout &&
 		req->retries < nvme_max_retries;
 }
@@ -275,6 +279,8 @@ int nvme_init_identify(struct nvme_ctrl *ctrl);
 
 void nvme_queue_scan(struct nvme_ctrl *ctrl);
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl);
+void nvme_unlock_from_suspend(struct nvme_ctrl *ctrl);
+int nvme_opal_initialize(struct nvme_ctrl *ctrl);
 
 #define NVME_NR_AERS	1
 void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 2fd7dc2..d298c15 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -43,6 +43,7 @@
 #include <linux/types.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <asm/unaligned.h>
+#include <linux/sed-opal.h>
 
 #include "nvme.h"
 
@@ -1765,7 +1766,7 @@ static void nvme_reset_work(struct work_struct *work)
 {
 	struct nvme_dev *dev = container_of(work, struct nvme_dev, reset_work);
 	int result = -ENODEV;
-
+	bool was_suspend = !!(dev->ctrl.ctrl_config & NVME_CC_SHN_NORMAL);
 	if (WARN_ON(dev->ctrl.state == NVME_CTRL_RESETTING))
 		goto out;
 
@@ -1796,6 +1797,13 @@ static void nvme_reset_work(struct work_struct *work)
 	if (result)
 		goto out;
 
+	result = nvme_opal_initialize(&dev->ctrl);
+	if (result)
+		goto out;
+
+	if (was_suspend)
+		nvme_unlock_from_suspend(&dev->ctrl);
+
 	result = nvme_setup_io_queues(dev);
 	if (result)
 		goto out;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 5/5] Maintainers: Add Information for SED Opal library
  2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
                   ` (3 preceding siblings ...)
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
@ 2016-12-19 19:35 ` Scott Bauer
  4 siblings, 0 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 19:35 UTC (permalink / raw)
  To: linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi, Scott Bauer

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f6eb97b..76d542c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11051,6 +11051,16 @@ L:	linux-mmc@vger.kernel.org
 S:	Maintained
 F:	drivers/mmc/host/sdhci-spear.c
 
+SECURE ENCRYPTING DEVICE (SED) OPAL DRIVER
+M:	Scott Bauer <scott.bauer@intel.com>
+M:	Jonathan Derrick <jonathan.derrick@intel.com>
+M:	Rafael Antognolli <rafael.antognolli@intel.com>
+L:	linux-nvme@lists.infradead.org
+S:	Supported
+F:	lib/sed*
+F:	include/linux/sed*
+F:	include/uapi/linux/sed*
+
 SECURITY SUBSYSTEM
 M:	James Morris <james.l.morris@oracle.com>
 M:	"Serge E. Hallyn" <serge@hallyn.com>
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
@ 2016-12-19 21:34   ` Keith Busch
  2016-12-20  6:07     ` Christoph Hellwig
  2016-12-20  3:21   ` kbuild test robot
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 35+ messages in thread
From: Keith Busch @ 2016-12-19 21:34 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, Rafael.Antognolli, axboe, jonathan.derrick, viro,
	hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 12:35:46PM -0700, Scott Bauer wrote:
> This patch implements the necessary logic to bring an Opal
> enabled drive out of a factory-enabled into a working
> Opal state.
> 
> This patch set also enables logic to save a password to
> be replayed during a resume from suspend.
> 
> Signed-off-by: Scott Bauer <scott.bauer@intel.com>
> Signed-off-by: Rafael Antognolli <Rafael.Antognolli@intel.com>
> ---
>  lib/Makefile            |    2 +-
>  lib/sed-opal.c          | 2376 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/sed-opal_internal.h |  601 ++++++++++++
>  lib/sed.c               |  197 ++++
>  4 files changed, 3175 insertions(+), 1 deletion(-)
>  create mode 100644 lib/sed-opal.c
>  create mode 100644 lib/sed-opal_internal.h
>  create mode 100644 lib/sed.c
> 
> diff --git a/lib/Makefile b/lib/Makefile
> index 50144a3..acb5d82 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -36,7 +36,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
>  	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \
>  	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
>  	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \
> -	 once.o
> +	 once.o sed.o sed-opal.o

This seems like an optional library that some environments may wish to
opt-out of building into the kernel. Any reason not to add an entry into
the Kconfig to turn this on/off?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
@ 2016-12-19 21:59   ` Keith Busch
  2016-12-19 22:23     ` Scott Bauer
  2016-12-20  4:11   ` kbuild test robot
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 35+ messages in thread
From: Keith Busch @ 2016-12-19 21:59 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, Rafael.Antognolli, axboe, jonathan.derrick, viro,
	hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 12:35:48PM -0700, Scott Bauer wrote:
> +static int nvme_sec_submit(struct nvme_ctrl *ctrl, u16 spsp, u8 secp,
> +			   void *buffer, size_t len, u8 opcode)
> +{
> +	struct nvme_command cmd = { 0 };
> +	struct nvme_ns *ns = NULL;
> +
> +	mutex_lock(&ctrl->namespaces_mutex);
> +	if (!list_empty(&ctrl->namespaces))
> +		ns = list_first_entry(&ctrl->namespaces, struct nvme_ns, list);
> +
> +	mutex_unlock(&ctrl->namespaces_mutex);
> +	if (!ns)
> +		return -ENODEV;
> +
> +	cmd.common.opcode = opcode;
> +	cmd.common.nsid = ns->ns_id;

Should be:

	cmd.common.nsid = cpu_to_le32(ns->ns_id);

But now wondering how you can send a security command to different
namespaces. That's why I thought it'd make more sense to threa this
through block_device, but maybe Christoph had some idea on how to get
the same functionality without that?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 21:59   ` Keith Busch
@ 2016-12-19 22:23     ` Scott Bauer
  2016-12-20  6:17       ` Christoph Hellwig
  0 siblings, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-19 22:23 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-nvme, Rafael.Antognolli, axboe, jonathan.derrick, viro,
	hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 04:59:54PM -0500, Keith Busch wrote:
> On Mon, Dec 19, 2016 at 12:35:48PM -0700, Scott Bauer wrote:
> > +static int nvme_sec_submit(struct nvme_ctrl *ctrl, u16 spsp, u8 secp,
> > +			   void *buffer, size_t len, u8 opcode)
> > +{
> > +	struct nvme_command cmd = { 0 };
> > +	struct nvme_ns *ns = NULL;
> > +
> > +	mutex_lock(&ctrl->namespaces_mutex);
> > +	if (!list_empty(&ctrl->namespaces))
> > +		ns = list_first_entry(&ctrl->namespaces, struct nvme_ns, list);
> > +
> > +	mutex_unlock(&ctrl->namespaces_mutex);
> > +	if (!ns)
> > +		return -ENODEV;
> > +
> > +	cmd.common.opcode = opcode;
> > +	cmd.common.nsid = ns->ns_id;
> 
> Should be:
> 
> 	cmd.common.nsid = cpu_to_le32(ns->ns_id);
> 
> But now wondering how you can send a security command to different
> namespaces. That's why I thought it'd make more sense to threa this
> through block_device, but maybe Christoph had some idea on how to get
> the same functionality without that?


I went back and reviewed the spec 1.2.1:

http://www.nvmexpress.org/wp-content/uploads/NVM_Express_1_2_1_Gold_20160603.pdf
Section 5.18 (page 140->141)

Describes the security send command type and it doesn't have any reference of
a namespace ID. Anecdotally, I just removed the ns->ns_id line from the code and
everything still works as intended. Is there another portion of the spec or errata
that requires ns_id? (I can't access 1.2.1 errta the link doesn't work).

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
  2016-12-19 21:34   ` Keith Busch
@ 2016-12-20  3:21   ` kbuild test robot
  2016-12-20  3:48   ` kbuild test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 35+ messages in thread
From: kbuild test robot @ 2016-12-20  3:21 UTC (permalink / raw)
  To: Scott Bauer
  Cc: kbuild-all, linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi, Scott Bauer

[-- Attachment #1: Type: text/plain, Size: 9180 bytes --]

Hi Scott,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9 next-20161219]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Scott-Bauer/include-Add-definitions-for-sed/20161220-110214
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   In file included from lib/sed.c:20:0:
>> include/linux/sed-opal.h:37:40: warning: 'struct request_queue' declared inside parameter list will not be visible outside of this definition or declaration
    struct opal_dev *alloc_opal_dev(struct request_queue *q);
                                           ^~~~~~~~~~~~~
   lib/sed.c: In function 'fdev_sed_ioctl':
>> lib/sed.c:161:12: error: dereferencing pointer to incomplete type 'struct file'
     if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
               ^~
--
   In file included from lib/sed-opal.c:29:0:
>> include/linux/sed-opal.h:37:40: warning: 'struct request_queue' declared inside parameter list will not be visible outside of this definition or declaration
    struct opal_dev *alloc_opal_dev(struct request_queue *q);
                                           ^~~~~~~~~~~~~
   lib/sed-opal.c: In function 'opal_discovery0_end':
   lib/sed-opal.c:276:6: warning: unused variable 'error' [-Wunused-variable]
     int error = 0;
         ^~~~~
   lib/sed-opal.c: In function 'response_parse':
>> lib/sed-opal.c:793:15: error: invalid storage class for function 'response_get_string'
    static size_t response_get_string(const struct parsed_resp *resp, int n,
                  ^~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:793:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static size_t response_get_string(const struct parsed_resp *resp, int n,
    ^~~~~~
>> lib/sed-opal.c:817:12: error: invalid storage class for function 'response_get_u64'
    static u64 response_get_u64(const struct parsed_resp *resp, int n)
               ^~~~~~~~~~~~~~~~
>> lib/sed-opal.c:846:11: error: invalid storage class for function 'response_status'
    static u8 response_status(const struct parsed_resp *resp)
              ^~~~~~~~~~~~~~~
>> lib/sed-opal.c:866:12: error: invalid storage class for function 'parse_and_check_status'
    static int parse_and_check_status(struct opal_dev *dev)
               ^~~~~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:883:13: error: invalid storage class for function 'clear_opal_cmd'
    static void clear_opal_cmd(struct opal_cmd *cmd)
                ^~~~~~~~~~~~~~
>> lib/sed-opal.c:891:12: error: invalid storage class for function 'start_opal_session_cont'
    static int start_opal_session_cont(struct opal_dev *dev)
               ^~~~~~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:913:20: error: invalid storage class for function 'opal_dev_get'
    static inline void opal_dev_get(struct opal_dev *dev)
                       ^~~~~~~~~~~~
>> lib/sed-opal.c:918:20: error: invalid storage class for function 'opal_dev_put'
    static inline void opal_dev_put(struct opal_dev *dev)
                       ^~~~~~~~~~~~
>> lib/sed-opal.c:923:12: error: invalid storage class for function 'add_suspend_info'
    static int add_suspend_info(struct opal_dev *dev, struct opal_suspend_data *sus)
               ^~~~~~~~~~~~~~~~
>> lib/sed-opal.c:949:12: error: invalid storage class for function 'end_session_cont'
    static int end_session_cont(struct opal_dev *dev)
               ^~~~~~~~~~~~~~~~
>> lib/sed-opal.c:956:12: error: invalid storage class for function 'finalize_and_send'
    static int finalize_and_send(struct opal_dev *dev, struct opal_cmd *cmd,
               ^~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:972:12: error: invalid storage class for function 'gen_key'
    static int gen_key(struct opal_dev *dev)
               ^~~~~~~
>> lib/sed-opal.c:1002:12: error: invalid storage class for function 'get_active_key_cont'
    static int get_active_key_cont(struct opal_dev *dev)
               ^~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:1027:12: error: invalid storage class for function 'get_active_key'
    static int get_active_key(struct opal_dev *dev)
               ^~~~~~~~~~~~~~
>> lib/sed-opal.c:1067:12: error: invalid storage class for function 'generic_lr_enable_disable'
    static int generic_lr_enable_disable(struct opal_cmd *cmd,
               ^~~~~~~~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:1107:19: error: invalid storage class for function 'enable_global_lr'
    static inline int enable_global_lr(struct opal_cmd *cmd, u8 *uid,
                      ^~~~~~~~~~~~~~~~
>> lib/sed-opal.c:1118:12: error: invalid storage class for function 'setup_locking_range'
    static int setup_locking_range(struct opal_dev *dev)
               ^~~~~~~~~~~~~~~~~~~
>> lib/sed-opal.c:1183:12: error: invalid storage class for function 'start_generic_opal_session'
    static int start_generic_opal_session(struct opal_dev *dev,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +161 lib/sed.c

    14	 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
    15	 * more details.
    16	 */
    17	
    18	#include <linux/blkdev.h>
    19	#include <linux/sed.h>
  > 20	#include <linux/sed-opal.h>
    21	#include <asm/uaccess.h>
    22	
    23	int sed_save(struct sed_context *sed_ctx, struct sed_key *key)
    24	{
    25		switch (key->sed_type) {
    26		case OPAL_LOCK_UNLOCK:
    27			return opal_save(sed_ctx, key);
    28		}
    29	
    30		return -EOPNOTSUPP;
    31	}
    32	
    33	int sed_lock_unlock(struct sed_context *sed_ctx, struct sed_key *key)
    34	{
    35		switch (key->sed_type) {
    36		case OPAL_LOCK_UNLOCK:
    37			return opal_lock_unlock(sed_ctx, key);
    38		}
    39	
    40		return -EOPNOTSUPP;
    41	}
    42	
    43	int sed_take_ownership(struct sed_context *sed_ctx, struct sed_key *key)
    44	{
    45	
    46		switch (key->sed_type) {
    47		case OPAL:
    48			return opal_take_ownership(sed_ctx, key);
    49		}
    50	
    51		return -EOPNOTSUPP;
    52	}
    53	
    54	int sed_activate_lsp(struct sed_context *sed_ctx, struct sed_key *key)
    55	{
    56	
    57		switch (key->sed_type) {
    58		case OPAL:
    59			return opal_activate_lsp(sed_ctx, key);
    60		}
    61	
    62		return -EOPNOTSUPP;
    63	}
    64	
    65	int sed_set_pw(struct sed_context *sed_ctx, struct sed_key *key)
    66	{
    67	
    68		switch (key->sed_type) {
    69		case OPAL_PW:
    70			return opal_set_new_pw(sed_ctx, key);
    71		}
    72	
    73		return -EOPNOTSUPP;
    74	}
    75	
    76	int sed_activate_user(struct sed_context *sed_ctx, struct sed_key *key)
    77	{
    78	
    79		switch (key->sed_type) {
    80		case OPAL_ACT_USR:
    81			return opal_activate_user(sed_ctx, key);
    82		}
    83	
    84		return -EOPNOTSUPP;
    85	}
    86	
    87	int sed_reverttper(struct sed_context *sed_ctx, struct sed_key *key)
    88	{
    89	
    90		switch (key->sed_type) {
    91		case OPAL:
    92			return opal_reverttper(sed_ctx, key);
    93		}
    94	
    95		return -EOPNOTSUPP;
    96	}
    97	
    98	int sed_setup_locking_range(struct sed_context *sed_ctx, struct sed_key *key)
    99	{
   100	
   101		switch (key->sed_type) {
   102		case OPAL_LR_SETUP:
   103			return opal_setup_locking_range(sed_ctx, key);
   104		}
   105	
   106		return -EOPNOTSUPP;
   107	}
   108	
   109	int sed_adduser_to_lr(struct sed_context *sed_ctx, struct sed_key *key)
   110	{
   111	
   112		switch (key->sed_type) {
   113		case OPAL_LOCK_UNLOCK:
   114			return opal_add_user_to_lr(sed_ctx, key);
   115		}
   116	
   117		return -EOPNOTSUPP;
   118	}
   119	
   120	int sed_do_mbr(struct sed_context *sed_ctx, struct sed_key *key)
   121	{
   122	
   123		switch (key->sed_type) {
   124		case OPAL_MBR_DATA:
   125			return opal_enable_disable_shadow_mbr(sed_ctx, key);
   126		}
   127	
   128		return -EOPNOTSUPP;
   129	}
   130	
   131	int sed_erase_lr(struct sed_context *sed_ctx, struct sed_key *key)
   132	{
   133	
   134		switch (key->sed_type) {
   135		case OPAL:
   136			return opal_erase_locking_range(sed_ctx, key);
   137		}
   138	
   139		return -EOPNOTSUPP;
   140	}
   141	
   142	int sed_secure_erase_lr(struct sed_context *sed_ctx, struct sed_key *key)
   143	{
   144		switch (key->sed_type) {
   145		case OPAL_ACT_USR:
   146			return opal_secure_erase_locking_range(sed_ctx, key);
   147	
   148		}
   149		return -EOPNOTSUPP;
   150	}
   151	
   152	int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
   153			   unsigned long arg)
   154	{
   155		struct sed_key key;
   156		struct sed_context *sed_ctx;
   157	
   158		if (!capable(CAP_SYS_ADMIN))
   159			return -EACCES;
   160	
 > 161		if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
   162			return -ENODEV;
   163	
   164		sed_ctx = filep->f_sedctx;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6384 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
  2016-12-19 21:34   ` Keith Busch
  2016-12-20  3:21   ` kbuild test robot
@ 2016-12-20  3:48   ` kbuild test robot
  2016-12-20  6:50   ` Al Viro
  2016-12-20  7:28   ` Christoph Hellwig
  4 siblings, 0 replies; 35+ messages in thread
From: kbuild test robot @ 2016-12-20  3:48 UTC (permalink / raw)
  To: Scott Bauer
  Cc: kbuild-all, linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi, Scott Bauer

[-- Attachment #1: Type: text/plain, Size: 14849 bytes --]

Hi Scott,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.9 next-20161219]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Scott-Bauer/include-Add-definitions-for-sed/20161220-110214
config: i386-randconfig-r0-201651 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from lib/sed.c:20:0:
>> include/linux/sed-opal.h:37:40: warning: 'struct request_queue' declared inside parameter list
    struct opal_dev *alloc_opal_dev(struct request_queue *q);
                                           ^
>> include/linux/sed-opal.h:37:40: warning: its scope is only this definition or declaration, which is probably not what you want
   lib/sed.c: In function 'fdev_sed_ioctl':
   lib/sed.c:161:12: error: dereferencing pointer to incomplete type 'struct file'
     if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
               ^
--
   In file included from lib/sed-opal.c:29:0:
>> include/linux/sed-opal.h:37:40: warning: 'struct request_queue' declared inside parameter list
    struct opal_dev *alloc_opal_dev(struct request_queue *q);
                                           ^
>> include/linux/sed-opal.h:37:40: warning: its scope is only this definition or declaration, which is probably not what you want
   lib/sed-opal.c: In function 'opal_discovery0_end':
   lib/sed-opal.c:276:6: warning: unused variable 'error' [-Wunused-variable]
     int error = 0;
         ^
   lib/sed-opal.c: In function 'response_parse':
   lib/sed-opal.c:793:15: error: invalid storage class for function 'response_get_string'
    static size_t response_get_string(const struct parsed_resp *resp, int n,
                  ^
   lib/sed-opal.c:793:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static size_t response_get_string(const struct parsed_resp *resp, int n,
    ^
   lib/sed-opal.c:817:12: error: invalid storage class for function 'response_get_u64'
    static u64 response_get_u64(const struct parsed_resp *resp, int n)
               ^
   lib/sed-opal.c:846:11: error: invalid storage class for function 'response_status'
    static u8 response_status(const struct parsed_resp *resp)
              ^
   lib/sed-opal.c:866:12: error: invalid storage class for function 'parse_and_check_status'
    static int parse_and_check_status(struct opal_dev *dev)
               ^
   lib/sed-opal.c:883:13: error: invalid storage class for function 'clear_opal_cmd'
    static void clear_opal_cmd(struct opal_cmd *cmd)
                ^
   lib/sed-opal.c:891:12: error: invalid storage class for function 'start_opal_session_cont'
    static int start_opal_session_cont(struct opal_dev *dev)
               ^
   lib/sed-opal.c:913:20: error: invalid storage class for function 'opal_dev_get'
    static inline void opal_dev_get(struct opal_dev *dev)
                       ^
   lib/sed-opal.c:918:20: error: invalid storage class for function 'opal_dev_put'
    static inline void opal_dev_put(struct opal_dev *dev)
                       ^
   lib/sed-opal.c:923:12: error: invalid storage class for function 'add_suspend_info'
    static int add_suspend_info(struct opal_dev *dev, struct opal_suspend_data *sus)
               ^
   lib/sed-opal.c:949:12: error: invalid storage class for function 'end_session_cont'
    static int end_session_cont(struct opal_dev *dev)
               ^
   lib/sed-opal.c:956:12: error: invalid storage class for function 'finalize_and_send'
    static int finalize_and_send(struct opal_dev *dev, struct opal_cmd *cmd,
               ^
   lib/sed-opal.c:972:12: error: invalid storage class for function 'gen_key'
    static int gen_key(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1002:12: error: invalid storage class for function 'get_active_key_cont'
    static int get_active_key_cont(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1027:12: error: invalid storage class for function 'get_active_key'
    static int get_active_key(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1067:12: error: invalid storage class for function 'generic_lr_enable_disable'
    static int generic_lr_enable_disable(struct opal_cmd *cmd,
               ^
   lib/sed-opal.c:1107:19: error: invalid storage class for function 'enable_global_lr'
    static inline int enable_global_lr(struct opal_cmd *cmd, u8 *uid,
                      ^
   lib/sed-opal.c:1118:12: error: invalid storage class for function 'setup_locking_range'
    static int setup_locking_range(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1183:12: error: invalid storage class for function 'start_generic_opal_session'
    static int start_generic_opal_session(struct opal_dev *dev,
               ^
   lib/sed-opal.c:1242:12: error: invalid storage class for function 'start_anybodyASP_opal_session'
    static int start_anybodyASP_opal_session(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1248:12: error: invalid storage class for function 'start_SIDASP_opal_session'
    static int start_SIDASP_opal_session(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1271:19: error: invalid storage class for function 'start_admin1LSP_opal_session'
    static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
                      ^
   lib/sed-opal.c:1279:12: error: invalid storage class for function 'start_auth_opal_session'
    static int start_auth_opal_session(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1332:12: error: invalid storage class for function 'revert_tper'
    static int revert_tper(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1354:12: error: invalid storage class for function 'internal_activate_user'
    static int internal_activate_user(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1391:12: error: invalid storage class for function 'erase_locking_range'
    static int erase_locking_range(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1419:12: error: invalid storage class for function 'set_mbr_done'
    static int set_mbr_done(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1451:12: error: invalid storage class for function 'set_mbr_enable_disable'
    static int set_mbr_enable_disable(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1483:12: error: invalid storage class for function 'generic_pw_cmd'
    static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
               ^
   lib/sed-opal.c:1510:12: error: invalid storage class for function 'set_new_pw'
    static int set_new_pw(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1536:12: error: invalid storage class for function 'set_sid_cpin_pin'
    static int set_sid_cpin_pin(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1551:12: error: invalid storage class for function 'add_user_to_lr'
    static int add_user_to_lr(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1621:12: error: invalid storage class for function 'lock_unlock_locking_range'
    static int lock_unlock_locking_range(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1687:12: error: invalid storage class for function 'lock_unlock_locking_range_SUM'
    static int lock_unlock_locking_range_SUM(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1782:12: error: invalid storage class for function 'get_lsp_lifecycle_cont'
    static int get_lsp_lifecycle_cont(struct opal_dev *dev)
               ^
   lib/sed-opal.c: In function 'get_lsp_lifecycle_cont':
   lib/sed-opal.c:1799:1: warning: label 'err_return' defined but not used [-Wunused-label]
    err_return:
    ^
   lib/sed-opal.c: In function 'response_parse':
   lib/sed-opal.c:1842:12: error: invalid storage class for function 'get_msid_cpin_pin_cont'
    static int get_msid_cpin_pin_cont(struct opal_dev *dev)
               ^
   lib/sed-opal.c: In function 'get_msid_cpin_pin_cont':
   lib/sed-opal.c:1864:2: warning: label 'err_return' defined but not used [-Wunused-label]
     err_return:
     ^
   lib/sed-opal.c: In function 'response_parse':
   lib/sed-opal.c:1868:12: error: invalid storage class for function 'get_msid_cpin_pin'
    static int get_msid_cpin_pin(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1906:12: error: invalid storage class for function 'build_end_opal_session'
    static int build_end_opal_session(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1917:12: error: invalid storage class for function 'end_opal_session'
    static int end_opal_session(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1931:12: error: invalid storage class for function 'end_opal_session_error'
    static int end_opal_session_error(struct opal_dev *dev)
               ^
>> lib/sed-opal.c:1939:40: warning: 'struct request_queue' declared inside parameter list
    struct opal_dev *alloc_opal_dev(struct request_queue *q)
                                           ^
   lib/sed-opal.c: In function 'alloc_opal_dev':
   lib/sed-opal.c:1953:15: error: implicit declaration of function 'queue_dma_alignment' [-Werror=implicit-function-declaration]
     dma_align = (queue_dma_alignment(q) | q->dma_pad_mask) + 1;
                  ^
   lib/sed-opal.c:1953:41: error: dereferencing pointer to incomplete type 'struct request_queue'
     dma_align = (queue_dma_alignment(q) | q->dma_pad_mask) + 1;
                                            ^
   lib/sed-opal.c: In function 'response_parse':
   lib/sed-opal.c:1968:12: error: invalid storage class for function 'do_cmds'
    static int do_cmds(struct opal_dev *dev)
               ^
   lib/sed-opal.c:1968:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static int do_cmds(struct opal_dev *dev)
    ^
   lib/sed-opal.c:1976:25: error: invalid storage class for function 'get_opal_dev'
    static struct opal_dev *get_opal_dev(struct sed_context *sedc,
                            ^
   lib/sed-opal.c:2019:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_erase_locking_range(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2043:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_enable_disable_shadow_mbr(struct sed_context *sedc,
    ^
   lib/sed-opal.c:2077:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_save(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2098:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_add_user_to_lr(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2139:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_reverttper(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2162:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    const opal_step ulk_funcs_SUM[] = {
    ^
   lib/sed-opal.c:2202:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_take_ownership(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2227:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_activate_lsp(struct sed_context *sedc, struct sed_key *key)
    ^
   lib/sed-opal.c:2250:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_setup_locking_range(struct sed_context *sedc, struct sed_key *pw)
    ^
   lib/sed-opal.c:2273:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_set_new_pw(struct sed_context *sedc, struct sed_key *pw)
    ^
   lib/sed-opal.c:2306:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_activate_user(struct sed_context *sedc, struct sed_key *pw)
    ^
   lib/sed-opal.c:2342:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int opal_unlock_from_suspend(struct sed_context *sedc)
    ^
   lib/sed-opal.c:2376:1: error: expected declaration or statement at end of input
    EXPORT_SYMBOL(opal_unlock_from_suspend);
    ^
   lib/sed-opal.c:727:6: warning: unused variable 'ret' [-Wunused-variable]
     int ret, num_entries = 0;
         ^
   cc1: some warnings being treated as errors

vim +37 include/linux/sed-opal.h

0d963c73 Scott Bauer 2016-12-19  21  #include <linux/sed.h>
0d963c73 Scott Bauer 2016-12-19  22  #include <linux/kernel.h>
0d963c73 Scott Bauer 2016-12-19  23  
0d963c73 Scott Bauer 2016-12-19  24  int opal_save(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  25  int opal_lock_unlock(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  26  int opal_take_ownership(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  27  int opal_activate_lsp(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  28  int opal_set_new_pw(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  29  int opal_activate_user(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  30  int opal_reverttper(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  31  int opal_setup_locking_range(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  32  int opal_add_user_to_lr(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  33  int opal_enable_disable_shadow_mbr(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  34  int opal_erase_locking_range(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  35  int opal_secure_erase_locking_range(struct sed_context *sedc, struct sed_key *key);
0d963c73 Scott Bauer 2016-12-19  36  int opal_unlock_from_suspend(struct sed_context *sedc);
0d963c73 Scott Bauer 2016-12-19 @37  struct opal_dev *alloc_opal_dev(struct request_queue *q);
0d963c73 Scott Bauer 2016-12-19  38  #endif /* LINUX_OPAL_H */

:::::: The code at line 37 was first introduced by commit
:::::: 0d963c733cceca032531cf3f500125bc28d01178 include: Add definitions for sed

:::::: TO: Scott Bauer <scott.bauer@intel.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22850 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
  2016-12-19 21:59   ` Keith Busch
@ 2016-12-20  4:11   ` kbuild test robot
  2016-12-20  6:21   ` Christoph Hellwig
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 35+ messages in thread
From: kbuild test robot @ 2016-12-20  4:11 UTC (permalink / raw)
  To: Scott Bauer
  Cc: kbuild-all, linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi, Scott Bauer

[-- Attachment #1: Type: text/plain, Size: 8549 bytes --]

Hi Scott,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.9 next-20161219]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Scott-Bauer/include-Add-definitions-for-sed/20161220-110214
config: x86_64-randconfig-i0-201651 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/nvme/host/core.c: In function 'nvme_sec_submit':
>> drivers/nvme/host/core.c:770:9: warning: missing braces around initializer [-Wmissing-braces]
     struct nvme_command cmd = { 0 };
            ^
   drivers/nvme/host/core.c:770:9: warning: (near initialization for 'cmd.<anonymous>') [-Wmissing-braces]
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:__set_bit
   Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:test_and_set_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64
   Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le64_to_cpup
   Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le16_to_cpup
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64
   Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
   Cyclomatic Complexity 1 include/linux/list.h:__list_del
   Cyclomatic Complexity 1 include/linux/list.h:list_empty
   Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current
   Cyclomatic Complexity 2 arch/x86/include/asm/page_64.h:__phys_addr_nodebug
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_read
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_set
   Cyclomatic Complexity 2 arch/x86/include/asm/atomic.h:atomic_sub_and_test
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_add_return
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_cmpxchg
   Cyclomatic Complexity 5 arch/x86/include/asm/atomic.h:__atomic_add_unless
   Cyclomatic Complexity 1 include/linux/atomic.h:atomic_add_unless
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 1 include/linux/thread_info.h:check_object_size
   Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock_irq
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irq
   Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies
   Cyclomatic Complexity 5 include/linux/jiffies.h:msecs_to_jiffies
   Cyclomatic Complexity 1 include/linux/workqueue.h:to_delayed_work
   Cyclomatic Complexity 1 include/linux/signal.h:sigismember
   Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info
   Cyclomatic Complexity 1 include/linux/slab.h:__kmalloc_node
   Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kmalloc_node
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc_node
   Cyclomatic Complexity 1 include/linux/kref.h:kref_init
   Cyclomatic Complexity 1 include/linux/kref.h:kref_get_unless_zero
   Cyclomatic Complexity 1 include/linux/device.h:dev_to_node
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/fs.h:iminor
   Cyclomatic Complexity 1 include/linux/genhd.h:get_capacity
   Cyclomatic Complexity 1 include/linux/genhd.h:set_capacity
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write
   Cyclomatic Complexity 11 arch/x86/include/asm/uaccess.h:copy_from_user
   Cyclomatic Complexity 11 arch/x86/include/asm/uaccess.h:copy_to_user
   Cyclomatic Complexity 1 include/linux/blk_types.h:op_is_write
   Cyclomatic Complexity 1 include/linux/blkdev.h:queue_flag_set_unlocked
   Cyclomatic Complexity 1 include/linux/blkdev.h:blk_rq_pos
   Cyclomatic Complexity 1 include/linux/blkdev.h:blk_rq_bytes
   Cyclomatic Complexity 1 include/linux/blkdev.h:blk_integrity_rq
   Cyclomatic Complexity 1 include/linux/blkdev.h:blk_queue_max_integrity_segments
   Cyclomatic Complexity 1 include/linux/blk-mq.h:blk_mq_rq_to_pdu
   Cyclomatic Complexity 1 include/linux/module.h:try_module_get
   Cyclomatic Complexity 1 include/linux/module.h:module_put
   Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:put_unaligned_le64
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_req
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_block_nr
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_nvm_register
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_nvm_unregister
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_nvm_register_sysfs
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_nvm_unregister_sysfs
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_nvm_ns_supported
   Cyclomatic Complexity 1 drivers/nvme/host/nvme.h:nvme_get_ns_from_dev
   Cyclomatic Complexity 1 drivers/nvme/host/core.c:nvme_getgeo
   Cyclomatic Complexity 7 drivers/nvme/host/core.c:nvme_pr_type
   Cyclomatic Complexity 1 drivers/nvme/host/core.c:nvme_sysfs_show_address
   Cyclomatic Complexity 1 drivers/nvme/host/core.c:ns_cmp
   Cyclomatic Complexity 2 drivers/nvme/host/core.c:nvme_async_event_work
   Cyclomatic Complexity 2 include/linux/thread_info.h:test_ti_thread_flag
   Cyclomatic Complexity 1 include/linux/sched.h:test_tsk_thread_flag
   Cyclomatic Complexity 3 include/linux/blkdev.h:blk_get_integrity
   Cyclomatic Complexity 3 drivers/nvme/host/core.c:nvme_sysfs_reset
   Cyclomatic Complexity 21 drivers/nvme/host/core.c:nvme_dev_attrs_are_visible
   Cyclomatic Complexity 9 include/linux/blkdev.h:queue_logical_block_size
   Cyclomatic Complexity 7 drivers/nvme/host/core.c:nvme_dev_open
   Cyclomatic Complexity 3 drivers/nvme/host/nvme.h:nvme_reset_subsystem
   Cyclomatic Complexity 3 include/linux/nvme.h:nvme_is_write
   Cyclomatic Complexity 2 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 1 include/linux/sched.h:signal_pending
   Cyclomatic Complexity 1 include/linux/sched.h:__fatal_signal_pending
   Cyclomatic Complexity 3 include/linux/sched.h:fatal_signal_pending
   Cyclomatic Complexity 1 drivers/nvme/host/core.c:nvme_setup_flush
   Cyclomatic Complexity 3 drivers/nvme/host/core.c:nvme_setup_discard
   Cyclomatic Complexity 13 drivers/nvme/host/core.c:nvme_setup_rw
   Cyclomatic Complexity 4 arch/x86/include/asm/uaccess.h:copy_user_overflow
   Cyclomatic Complexity 1 include/linux/workqueue.h:queue_delayed_work
   Cyclomatic Complexity 1 include/linux/workqueue.h:schedule_delayed_work
   Cyclomatic Complexity 3 drivers/nvme/host/core.c:nvme_keep_alive_end_io
   Cyclomatic Complexity 12 drivers/nvme/host/core.c:nvme_wait_ready
   Cyclomatic Complexity 7 drivers/nvme/host/core.c:nvme_set_queue_limits

vim +770 drivers/nvme/host/core.c

   754			timeout = msecs_to_jiffies(cmd.timeout_ms);
   755	
   756		status = nvme_submit_user_cmd(ns ? ns->queue : ctrl->admin_q, &c,
   757				(void __user *)(uintptr_t)cmd.addr, cmd.data_len,
   758				&cmd.result, timeout);
   759		if (status >= 0) {
   760			if (put_user(cmd.result, &ucmd->result))
   761				return -EFAULT;
   762		}
   763	
   764		return status;
   765	}
   766	
   767	static int nvme_sec_submit(struct nvme_ctrl *ctrl, u16 spsp, u8 secp,
   768				   void *buffer, size_t len, u8 opcode)
   769	{
 > 770		struct nvme_command cmd = { 0 };
   771		struct nvme_ns *ns = NULL;
   772	
   773		mutex_lock(&ctrl->namespaces_mutex);
   774		if (!list_empty(&ctrl->namespaces))
   775			ns = list_first_entry(&ctrl->namespaces, struct nvme_ns, list);
   776	
   777		mutex_unlock(&ctrl->namespaces_mutex);
   778		if (!ns)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20893 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 21:34   ` Keith Busch
@ 2016-12-20  6:07     ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:07 UTC (permalink / raw)
  To: Keith Busch
  Cc: Scott Bauer, linux-nvme, Rafael.Antognolli, axboe,
	jonathan.derrick, viro, hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 04:34:15PM -0500, Keith Busch wrote:
> This seems like an optional library that some environments may wish to
> opt-out of building into the kernel. Any reason not to add an entry into
> the Kconfig to turn this on/off?

This needs to be a CONFIG_BLOCK_SED / CONFIG_BLOCK_SED_OPAL and
should move to block/.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 22:23     ` Scott Bauer
@ 2016-12-20  6:17       ` Christoph Hellwig
  2016-12-20 15:49         ` Keith Busch
  0 siblings, 1 reply; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:17 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Keith Busch, linux-nvme, Rafael.Antognolli, axboe,
	jonathan.derrick, viro, hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 03:23:12PM -0700, Scott Bauer wrote:
> I went back and reviewed the spec 1.2.1:
> 
> http://www.nvmexpress.org/wp-content/uploads/NVM_Express_1_2_1_Gold_20160603.pdf
> Section 5.18 (page 140->141)
> 
> Describes the security send command type and it doesn't have any reference of
> a namespace ID. Anecdotally, I just removed the ns->ns_id line from the code and
> everything still works as intended. Is there another portion of the spec or errata
> that requires ns_id? (I can't access 1.2.1 errta the link doesn't work).

As far as I can tell Security Send / Receive has always been intended to
apply to the whole controller, even if that's something I would not
personally think is a good idea.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
  2016-12-19 21:59   ` Keith Busch
  2016-12-20  4:11   ` kbuild test robot
@ 2016-12-20  6:21   ` Christoph Hellwig
  2016-12-20  6:49   ` Christoph Hellwig
  2016-12-25 14:15   ` Jethro Beekman
  4 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:21 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi

> +static int nvme_sec_send(void *ctrl_data, u16 spsp, u8 secp,
> +			 void *buf, size_t len)
> +{
> +	return nvme_sec_submit(ctrl_data, spsp, secp, buf, len,
> +			       nvme_admin_security_send);
> +}
> +
> +static int nvme_sec_recv(void *ctrl_data, u16 spsp, u8 secp,
> +			 void *buf, size_t len)
> +{
> +	return nvme_sec_submit(ctrl_data, spsp, secp, buf, len,
> +			       nvme_admin_security_recv);
> +}
> +
> +static const struct sec_ops nvme_sec_ops = {
> +	.sec_send = nvme_sec_send,
> +	.sec_recv = nvme_sec_recv,
> +};

Just make sec_submit the callback passed to the core and avoid
this boiler-plate code.

>  
> +int nvme_opal_initialize(struct nvme_ctrl *ctrl)
> +{
> +	/* Opal dev has already been allocated for this controller */
> +	if (ctrl->sed_ctx.dev)
> +		return 0;
> +
> +	ctrl->sed_ctx.dev = alloc_opal_dev(ctrl->admin_q);
> +	if (!ctrl->sed_ctx.dev)
> +		return -ENOMEM;
> +	ctrl->sed_ctx.ops = &nvme_sec_ops;
> +	ctrl->sed_ctx.sec_data = ctrl;

No need for sec_data callback, just pass the sed_ctx to the driver
and use container_of to get at the containing structure.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl
  2016-12-19 19:35 ` [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl Scott Bauer
@ 2016-12-20  6:21   ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:21 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi

> @@ -853,6 +854,7 @@ struct file {
>  #ifdef CONFIG_SECURITY
>  	void			*f_security;
>  #endif
> +	struct sed_context      *f_sedctx;

Adding a new field to the global struct file for a block driver
feature is not acceptable.  And I don't really see why it would
be nessecary anyway.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 1/5] include: Add definitions for sed
  2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
@ 2016-12-20  6:46   ` Christoph Hellwig
  2016-12-25 14:15   ` Jethro Beekman
  1 sibling, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:46 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, keith.busch, sagi, hch, Rafael.Antognolli,
	linux-kernel, axboe, viro, jonathan.derrick

On Mon, Dec 19, 2016 at 12:35:45PM -0700, Scott Bauer wrote:
> This patch adds the definitions and structures for the SED
> Opal code.

This seems to contain a few things:  userspace ABIs, on the wire
defintions, and prototypes for the code added in patch 2.

The userspace ABIs should be a header on it's own.  For new code
we also usually try to split the protocol definitions from the
in-kernel APIs, although that's not even reflected in the headers
here.  If you think it's reasonable to split those out those should
be a separate new patch, and the actual in-kernel data structures
and protoypes should just go with the actual code in your current patch
2.

> +/**
> + * struct sed_context - SED Security context for a device
> + * @ops:The Trusted Send/Recv functions.
> + * @sec_data:Opaque pointer that will be passed to the send/recv fn.
> + *Drivers can use this to pass necessary data required for
> + *Their implementation of send/recv.
> + * @dev:Currently an Opal Dev structure. In the future can be other types
> + *Of security structures.
> + */

This is missing a couple spaces for the common kerneldoc format.

> +struct sed_context {
> +	const struct sec_ops *ops;
> +	void *sec_data;
> +	void *dev;
> +};

What is the difference between sec_data and dev?  And do your really
need either one when using the container_of trick I pointed out in
response to the other patch.

> +struct sed_key {
> +	__u32 sed_type;
> +	union {
> +		struct opal_key            opal;
> +		struct opal_new_pw         opal_pw;
> +		struct opal_session_info   opal_session;
> +		struct opal_user_lr_setup  opal_lrs;
> +		struct opal_lock_unlock    opal_lk_unlk;
> +		struct opal_mbr_data       opal_mbr;
> +		/* additional command set key types */
> +	};
> +};
> +
> +#define IOC_SED_SAVE		   _IOW('p', 220, struct sed_key)
> +#define IOC_SED_LOCK_UNLOCK	   _IOW('p', 221, struct sed_key)
> +#define IOC_SED_TAKE_OWNERSHIP	   _IOW('p', 222, struct sed_key)
> +#define IOC_SED_ACTIVATE_LSP       _IOW('p', 223, struct sed_key)
> +#define IOC_SED_SET_PW             _IOW('p', 224, struct sed_key)
> +#define IOC_SED_ACTIVATE_USR       _IOW('p', 225, struct sed_key)
> +#define IOC_SED_REVERT_TPR         _IOW('p', 226, struct sed_key)
> +#define IOC_SED_LR_SETUP           _IOW('p', 227, struct sed_key)
> +#define IOC_SED_ADD_USR_TO_LR      _IOW('p', 228, struct sed_key)
> +#define IOC_SED_ENABLE_DISABLE_MBR _IOW('p', 229, struct sed_key)
> +#define IOC_SED_ERASE_LR           _IOW('p', 230, struct sed_key)
> +#define IOC_SED_SECURE_ERASE_LR    _IOW('p', 231, struct sed_key)

I'll need to look at the details again, but it seems like each ioctl
uses exactly one of the structures in the union above, so there is no
real need for that union.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
                     ` (2 preceding siblings ...)
  2016-12-20  6:21   ` Christoph Hellwig
@ 2016-12-20  6:49   ` Christoph Hellwig
  2016-12-25 14:15   ` Jethro Beekman
  4 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  6:49 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, keith.busch, sagi, hch, Rafael.Antognolli,
	linux-kernel, axboe, viro, jonathan.derrick

> +void nvme_unlock_from_suspend(struct nvme_ctrl *ctrl)
> +{
> +	if (opal_unlock_from_suspend(&ctrl->sed_ctx))
> +		pr_warn("Failed to unlock one or more locking ranges!\n");
> +}
> +EXPORT_SYMBOL_GPL(nvme_unlock_from_suspend);

I don't think we even need this wrapper.  Also for the warning please
use dev_warn so that the user knows which controller failed.

> @@ -1765,7 +1766,7 @@ static void nvme_reset_work(struct work_struct *work)
>  {
>  	struct nvme_dev *dev = container_of(work, struct nvme_dev, reset_work);
>  	int result = -ENODEV;
> -
> +	bool was_suspend = !!(dev->ctrl.ctrl_config & NVME_CC_SHN_NORMAL);
>  	if (WARN_ON(dev->ctrl.state == NVME_CTRL_RESETTING))

Please don't remove the empty line after the variable declarations.
Also I would place your new line before the result line, as that makes
it a tad easier to read.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
                     ` (2 preceding siblings ...)
  2016-12-20  3:48   ` kbuild test robot
@ 2016-12-20  6:50   ` Al Viro
  2016-12-20  7:28   ` Christoph Hellwig
  4 siblings, 0 replies; 35+ messages in thread
From: Al Viro @ 2016-12-20  6:50 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, hch, linux-kernel, sagi

On Mon, Dec 19, 2016 at 12:35:46PM -0700, Scott Bauer wrote:

> +int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
> +		   unsigned long arg)
> +{
> +	struct sed_key key;
> +	struct sed_context *sed_ctx;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
> +		return -ENODEV;
> +
> +	sed_ctx = filep->f_sedctx;

First of all, that's a bisect hazard.  What's more, looking through the
rest of patchset, WTF does it
	* need to be called that early in ioctl(2) handling, instead of
having ->ioctl() instance for that sucker calling it?
	* _not_ get your ->f_sedctx as an explicit argument, passed by
the caller in ->ioctl(), seeing that it's possible to calculate by
file->private_data?
	* store that thing in struct file itself, bloating it for everything
all for the sake of few drivers that might want to use that helper?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
                     ` (3 preceding siblings ...)
  2016-12-20  6:50   ` Al Viro
@ 2016-12-20  7:28   ` Christoph Hellwig
  2016-12-20 21:55     ` Scott Bauer
  2016-12-20 22:07     ` Jon Derrick
  4 siblings, 2 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20  7:28 UTC (permalink / raw)
  To: Scott Bauer
  Cc: linux-nvme, keith.busch, sagi, hch, Rafael.Antognolli,
	linux-kernel, axboe, viro, jonathan.derrick

> +	u8 lr;
> +	size_t key_name_len;
> +	char key_name[36];

Who is going to use the key_name?  I can't find another reference to
it anywhere else in the code.   The reason why this tripped me off was
the hardcoded length so I was going to check on how access to it is
bounds checked.

> +/**
> + * struct opal_dev - The structure representing a OPAL enabled SED.
> + * @sed_ctx:The SED context, contains fn pointers to sec_send/recv.
> + * @opal_step:A series of opal methods that are necessary to complete a comannd.
> + * @func_data:An array of parameters for the opal methods above.
> + * @state:Describes the current opal_step we're working on.
> + * @dev_lock:Locks the entire opal_dev structure.
> + * @parsed:Parsed response from controller.
> + * @prev_data:Data returned from a method to the controller
> + * @error_cb:Error function that handles closing sessions after a failed method.
> + * @unlk_lst:A list of Locking ranges to unlock on this device during a resume.
> + */

Needs spaces after the colons.

> +	u16 comID;
> +	u32 HSN;
> +	u32 TSN;

Please use lower case variable names in Linux code, and separate
words by underscores if needed.

> +DEFINE_SPINLOCK(list_spinlock);

Should be marked static.

> +#define TPER_SYNC_SUPPORTED BIT(0)

This sounds like a protocol constant and should go into the header
with the rest of the protocol.

> +static bool check_tper(const void *data)
> +{
> +	const struct d0_tper_features *tper = data;
> +	u8 flags = tper->supported_features;
> +
> +	if (!(flags & TPER_SYNC_SUPPORTED)) {
> +		pr_err("TPer sync not supported. flags = %d\n",
> +		       tper->supported_features);

It would be great to use dev_err / dev_warn / etc in the opal code,
although for that you'll need to pass a struct device from the driver
into the code.

> +static bool check_SUM(const void *data)

The comment on member naming above also applies to variables and function
names.

> +	bool foundComID = false, supported = true, single_user = false;
> +	const struct d0_header *hdr;
> +	const u8 *epos, *cpos;
> +	u16 comID = 0;
> +	int error = 0;
> +
> +	epos = dev->cmd.resp;
> +	cpos = dev->cmd.resp;
> +	hdr = (struct d0_header *)dev->cmd.resp;

You probably want to structure your command buffers to use void pointers
and avoid ths kind of casting.

> +#define TINY_ATOM_DATA_MASK GENMASK(5, 0)
> +#define TINY_ATOM_SIGNED BIT(6)
> +
> +#define SHORT_ATOM_ID BIT(7)
> +#define SHORT_ATOM_BYTESTRING BIT(5)
> +#define SHORT_ATOM_SIGNED BIT(4)
> +#define SHORT_ATOM_LEN_MASK GENMASK(3, 0)

Protocol constants for the header again?

> +#define ADD_TOKEN_STRING(cmd, key, keylen)		        \
> +	if (!err)					        \
> +		err = test_and_add_string(cmd, key, keylen);
> +
> +#define ADD_TOKEN(type, cmd, tok)				\
> +	if (!err)						\
> +		err = test_and_add_token_##type(cmd, tok);

Please remove these macros and just open code the calls.  If you want
to avoid writing the if err lines again and again just pass err
by reference to the functions and move the err check into the add_token*
helpers.

> +	if ((hdr->cp.length == 0)
> +	    || (hdr->pkt.length == 0)
> +	    || (hdr->subpkt.length == 0)) {

No need for the inner braces, also please place the operators at the
end of the previous line instead of the beginning of the new line.

> +	while (cpos < total) {
> +		if (!(pos[0] & 0x80)) /* tiny atom */
> +			token_length = response_parse_tiny(iter, pos);
> +		else if (!(pos[0] & 0x40)) /* short atom */
> +			token_length = response_parse_short(iter, pos);
> +		else if (!(pos[0] & 0x20)) /* medium atom */
> +			token_length = response_parse_medium(iter, pos);
> +		else if (!(pos[0] & 0x10)) /* long atom */
> +			token_length = response_parse_long(iter, pos);

Please add symbolic names for these constants to the protocol header.

> +	if (num_entries == 0) {
> +		pr_err("Couldn't parse response.\n");
> +		return -EINVAL;
> +	resp->num = num_entries;

Where is the closing brace for that if?

> +	if (!((resp->toks[n].width == OPAL_WIDTH_TINY) ||
> +	      (resp->toks[n].width == OPAL_WIDTH_SHORT))) {

No need for the inner braces.

> +		pr_err("Atom is not short or tiny: %d\n",
> +		       resp->toks[n].width);
> +		return 0;
> +	}
> +
> +	return resp->toks[n].stored.u;
> +}
> +
> +static u8 response_status(const struct parsed_resp *resp)
> +{
> +	if ((token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN)
> +	    && (response_get_token(resp, 0) == OPAL_ENDOFSESSION)) {

Same here, also please fix the operator placement.

> +	if ((token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN) ||
> +	    (token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN) ||
> +	    (response_get_token(resp, resp->num - 1) != OPAL_ENDLIST) ||
> +	    (response_get_token(resp, resp->num - 5) != OPAL_STARTLIST))

More brace removal here please (and probably a lot more later on, I'm
going to skip them from here)

> +static inline void opal_dev_get(struct opal_dev *dev)
> +{
> +	mutex_lock(&dev->dev_lock);
> +}
> +
> +static inline void opal_dev_put(struct opal_dev *dev)
> +{
> +	mutex_unlock(&dev->dev_lock);
> +}

No trivial wrappers around locking primitives, please.

> +static int add_suspend_info(struct opal_dev *dev, struct opal_suspend_data *sus)
> +{
> +	struct opal_suspend_data *iter;
> +	bool found = false;
> +
> +	if (list_empty(&dev->unlk_lst))
> +		goto add_out;
> +
> +	list_for_each_entry(iter, &dev->unlk_lst, node) {

list_for_each_entry will do the right thing for an empty
list, you can remove the above check,

> +		if (iter->lr == sus->lr) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	if (found) {
> +		/* Replace the old with the new */
> +		list_del(&iter->node);
> +		kfree(iter);
> +	}

Just move the list_del and kfree inside the if above and you
can remove the found variable.

> +
> +int activate_lsp(struct opal_dev *dev)

static?

> +static int get_msid_cpin_pin_cont(struct opal_dev *dev)
> +{
> +	const char *msid_pin;
> +	size_t strlen;
> +	int error = 0;
> +
> +	error = parse_and_check_status(dev);
> +	if (error)
> +		return error;
> +
> +	strlen = response_get_string(&dev->parsed, 4, &msid_pin);
> +	if (!msid_pin) {
> +		pr_err("%s: Couldn't extract PIN from response\n", __func__);
> +		return 11;

please add constant for your magic return values.

> +	}
> +
> +	dev->prev_data = kmemdup(msid_pin, strlen, GFP_KERNEL);
> +	if (!dev->prev_data)
> +		return -ENOMEM;
> +
> +	dev->prev_d_len = strlen;
> +
> + err_return:
> +	return 0;

I can't find anything actually jumping to this label.  And if it did
it could just return directly.

> +
> +static struct opal_dev *get_opal_dev(struct sed_context *sedc,
> +				     const opal_step *funcs)
> +{
> +	struct opal_dev *dev = sedc->dev;
> +	if (dev) {
> +		dev->state = 0;
> +		dev->funcs = funcs;
> +		dev->TSN = 0;
> +		dev->HSN = 0;
> +		dev->error_cb = end_opal_session_error;
> +		dev->error_cb_data = dev;
> +		dev->func_data = NULL;
> +		dev->sed_ctx = sedc;
> +		opal_dev_get(dev);
> +	}
> +	return dev;

Who serialized access to sedv->dev?

> +	struct opal_dev *dev;
> +	void *data[3] = { NULL };
> +	const opal_step erase_funcs[] = {
> +		opal_discovery0,
> +		start_auth_opal_session,
> +		get_active_key,
> +		gen_key,
> +		end_opal_session,
> +		NULL,
> +	};

static?

> +EXPORT_SYMBOL(opal_enable_disable_shadow_mbr);

As far as I can tell nothing but alloc_opal_dev and
opal_unlock_from_suspend is every called from another module,
so all these exports could be dropped.


> +	struct opal_dev *dev;
> +		const opal_step funcs[] = {

wrong indentation.  also all these arrays of functions should be
marked static.

> +	if (!list_empty(&dev->unlk_lst)) {
> +		list_for_each_entry(suspend, &dev->unlk_lst, node) {

No need for the list_empty check.

> --- /dev/null
> +++ b/lib/sed-opal_internal.h

This pretty much seem to contain the OPAL protocol defintions, so why
not opal_proto.h?

> +#ifndef _NVME_OPAL_INTERNAL_H
> +#define _NVME_OPAL_INTERNAL_H

And this doesn't seem to match the file name.

> +#include <linux/key-type.h>
> +#include <keys/user-type.h>

These don't seem to be needed.

> +/*
> + * Derived from:
> + * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
> + * Section: 5.1.5 Method Status Codes
> + */
> +static const char *opal_errors[] = {

static consta char * const opal_errors[] = {

also please move this into a .c file so that it's not duplicated
in every file that includes the header.

> +static const char *opal_error_to_human(int error)
> +{
> +	if (error == 0x3f)
> +		return "Failed";
> +
> +	if (error >= ARRAY_SIZE(opal_errors) || error < 0)
> +		return "Unknown Error";
> +
> +	return opal_errors[error];
> +}

Same for this one.

> +/*
> + * User IDs used in the TCG storage SSCs
> + * Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
> + * Section: 6.3 Assigned UIDs
> + */
> +static const u8 OPALUID[][8] = {

And all the other following variable delcarations.

Also please use lower case names with underscores as separators
for your variable and type names.  Constants can remain in all caps.

> +static const size_t OPAL_UID_LENGTH = 8;
> +static const size_t OPAL_MSID_KEYLEN = 15;
> +static const size_t OPAL_UID_LENGTH_HALF = 4;

Use a #define or enum fo these constants.


> +struct key *request_user_key(const char *master_desc, const u8 **master_key,
> +			     size_t *master_keylen);

This one doesn't actually seem to be used.

> +int sed_save(struct sed_context *sed_ctx, struct sed_key *key)
> +{
> +	switch (key->sed_type) {
> +	case OPAL_LOCK_UNLOCK:
> +		return opal_save(sed_ctx, key);
> +	}
> +
> +	return -EOPNOTSUPP;

It seems to me that we should skip this whole sed_type indirections
and just specify the ioctls directly for OPAL (which would include
opalite and pyrite as subsets).  The only other protocols of interest
for Linux would be the ATA "security" plain text passwords, which can
be handled differently, or enterprise SSC which we can hopefully avoid
to implement entirely.

> +
> +int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
> +		   unsigned long arg)
> +{
> +	struct sed_key key;
> +	struct sed_context *sed_ctx;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
> +		return -ENODEV;

In the previous version this was called from the block driver which
could pass in the context (and ops).  Why was this changed?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20 15:49         ` Keith Busch
@ 2016-12-20 15:46           ` Christoph Hellwig
  2016-12-20 16:05             ` Scott Bauer
  2016-12-20 17:52             ` Scott Bauer
  0 siblings, 2 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-20 15:46 UTC (permalink / raw)
  To: Keith Busch
  Cc: Christoph Hellwig, Scott Bauer, linux-nvme, Rafael.Antognolli,
	axboe, jonathan.derrick, viro, linux-kernel, sagi

On Tue, Dec 20, 2016 at 10:49:16AM -0500, Keith Busch wrote:
> On Mon, Dec 19, 2016 at 10:17:44PM -0800, Christoph Hellwig wrote:
> > As far as I can tell Security Send / Receive has always been intended to
> > apply to the whole controller, even if that's something I would not
> > personally think is a good idea.
> 
> NVMe security commands required the namespace ID since the very
> beginning. It's currently documented in figure 42 of section 5,
> "Namespace Identifier Used" column.

Oh, for some reason I read a no there when looking it up.
Good to know, although TCG spec still seem to ignore it.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20  6:17       ` Christoph Hellwig
@ 2016-12-20 15:49         ` Keith Busch
  2016-12-20 15:46           ` Christoph Hellwig
  0 siblings, 1 reply; 35+ messages in thread
From: Keith Busch @ 2016-12-20 15:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Scott Bauer, linux-nvme, Rafael.Antognolli, axboe,
	jonathan.derrick, viro, linux-kernel, sagi

On Mon, Dec 19, 2016 at 10:17:44PM -0800, Christoph Hellwig wrote:
> As far as I can tell Security Send / Receive has always been intended to
> apply to the whole controller, even if that's something I would not
> personally think is a good idea.

NVMe security commands required the namespace ID since the very
beginning. It's currently documented in figure 42 of section 5,
"Namespace Identifier Used" column.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20 15:46           ` Christoph Hellwig
@ 2016-12-20 16:05             ` Scott Bauer
  2016-12-21  9:01               ` Christoph Hellwig
  2016-12-20 17:52             ` Scott Bauer
  1 sibling, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-20 16:05 UTC (permalink / raw)
  To: Christoph Hellwig, Keith Busch
  Cc: axboe, sagi, Rafael.Antognolli, linux-kernel, linux-nvme,
	Scott Bauer, jonathan.derrick, viro



On 12/20/2016 08:46 AM, Christoph Hellwig wrote:
> On Tue, Dec 20, 2016 at 10:49:16AM -0500, Keith Busch wrote:
>> On Mon, Dec 19, 2016 at 10:17:44PM -0800, Christoph Hellwig wrote:
>>> As far as I can tell Security Send / Receive has always been intended to
>>> apply to the whole controller, even if that's something I would not
>>> personally think is a good idea.
>>
>> NVMe security commands required the namespace ID since the very
>> beginning. It's currently documented in figure 42 of section 5,
>> "Namespace Identifier Used" column.
> 
> Oh, for some reason I read a no there when looking it up.
> Good to know, although TCG spec still seem to ignore it.

Thanks Keith. Although TCG Spec currently ignores it in the future it may not.
In that case we should probably attempt to future proof it a bit. Since the
namespace ID is accessible via the block device structure I'll find a way to
include that in some opaque pointer that we can deliver through the core into
NVMe.

But this also brings up another question (and part of the reason I moved from
the block ioctl to fs ioctl): For drives with multiple namsepaces is it 
acceptable to allow a namespace, who has a segregated chunk of space, the ability
to perform actions outside of its range? Since the multiple namespaces portion
of the spec says there will be one Global LR a namespace that doesn't encompass
the entire LBA range can end up locking other LBAs via locking the global.

That's why I wanted to go to char dev because it seemed like a better fit for 
this scenario. Any thoughts on the above?



> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20 15:46           ` Christoph Hellwig
  2016-12-20 16:05             ` Scott Bauer
@ 2016-12-20 17:52             ` Scott Bauer
  2016-12-21  9:37               ` Christoph Hellwig
  1 sibling, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-20 17:52 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Keith Busch, linux-nvme, Rafael.Antognolli, axboe,
	jonathan.derrick, viro, linux-kernel, sagi

On Tue, Dec 20, 2016 at 07:46:39AM -0800, Christoph Hellwig wrote:
> On Tue, Dec 20, 2016 at 10:49:16AM -0500, Keith Busch wrote:
> > On Mon, Dec 19, 2016 at 10:17:44PM -0800, Christoph Hellwig wrote:
> > > As far as I can tell Security Send / Receive has always been intended to
> > > apply to the whole controller, even if that's something I would not
> > > personally think is a good idea.
> > 
> > NVMe security commands required the namespace ID since the very
> > beginning. It's currently documented in figure 42 of section 5,
> > "Namespace Identifier Used" column.
> 
> Oh, for some reason I read a no there when looking it up.
> Good to know, although TCG spec still seem to ignore it.

Before I submit another version I want to address a few design issues we seem
to be walking around a bit:

The other reviews you gave for the series are fine and will be implemented,
thank you for that.

The main development issue seems to be how the drivers/block layer interact
with the core sed.

1) We will move the core from lib/ back to block/ and add CONFIGS in kconfig.

2) Do we want to continue passing around a sed_context to the core? Instead of
   a block_device struct like we did in previous versions.

 2a) If we do wish to do wish to continue passng sed_contexts to the core I
 have to add a new variable to the block_device structure for our sed_context.
 Will this be acceptable? It wasn't acceptable for the file struct. The reason
 I need a new variable in the struct is:
 On the ioctl path, if we intercept the SED call in the block layer ioctl and
 have the call chain be:
 uland -> blk_ioctl -> sed_ioctl() -> sedcore -> sec_send/recv -> nvme
 then I need to be able to pass a sed_ctx struct in blk_ioctl to sed-ioctl and
 the only  way is to have it sitting in our block_device structure.

 The other way which was sorta nack'd last time is the following call chain:
 uland -> blk_ioctl -> nvme_ioctl -> sed_ioctl -> sedcore -> send/rcv -> nvme

 In this call chain in nvme_ioctl we have access to our block device struct
 and from there we can do blkdev->bd_disk->private_data to get our ns and then
 eventually our sed_ctx to pass to sed_ioctl. I could add the ns to the sec_data
 pointer in sed_context. This would give us access to ns without having to pass
 around a block device or store it anywhere.

 In the first scenario I can't work at all with opaque pointers like we can in
 the drivers itself (private_data). I don't know what they are, the drivers have
 the domain knowledge of what type they actually stored in private_data. That's
 why I need an explicit member in the block_device for the first scenario.

3) For NVMe we need access to our ns ID. It's in the block_device behind a few
pointers. What I can do is if we want to continue with the first ioctl path
described above is something like:

sed_ioctl(struct block_device *bdev, ...)
{
  sed_context *ctx = bdev->sed_ctx;
  ctx->sed_data = bdev->bd_disk->private_data;
  switch(cmd) {
  ...
  ...
  return some_opal_cmd(ctx);
  }
}

While this works for NVMe I don't know if this is acceptible for *all* users.
Since this is in a generic ioctl that is supposed to work with all drivers, who
knows what the hell they're putting in private_data and whether its useful for
their implementation of sec_send/recv.

I think that's all I have for now. If I think of anything throughout the day I'll
reply to to this email.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-20  7:28   ` Christoph Hellwig
@ 2016-12-20 21:55     ` Scott Bauer
  2016-12-21  9:42       ` Christoph Hellwig
  2016-12-20 22:07     ` Jon Derrick
  1 sibling, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-20 21:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-nvme, keith.busch, sagi, Rafael.Antognolli, linux-kernel,
	axboe, viro, jonathan.derrick

> > +
> > +int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
> > +		   unsigned long arg)
> > +{
> > +	struct sed_key key;
> > +	struct sed_context *sed_ctx;
> > +
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return -EACCES;
> > +
> > +	if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
> > +		return -ENODEV;
> 
> In the previous version this was called from the block driver which
> could pass in the context (and ops).  Why was this changed?
> 

Do you mean from nvme_ioctl? or from blk_ioctl? It was removed from  blk_ioctl
because I must have misinterpreted your comments here:
http://lists.infradead.org/pipermail/linux-nvme/2016-December/007364.html

Copied from the link: 
>
> > 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?

I took your hesitation about the block_device to mean try something new,
that combined with my concern about having namespaces have the ability to
lock the global range which can span ourside their LBA ranges lead me to
remove it from block and put it in the char dev world.

On the same note there is a public review of
TCG Storage Opal SSC Feature Set: Configurable Namespace Locking:
Which Jon Derrick found for us:
https://www.trustedcomputinggroup.org/wp-content/uploads/TCG_Storage_Feature_Set_Namespaces_phase_1b_v1_00_r1_19_public-review.pdf

Where they are thinking about doing a complete 180 from:
https://www.trustedcomputinggroup.org/wp-content/uploads/TCG_SWG_SIIS_Version_1_05_Revision_1_00.pdf

And now Namespaces can have their own global locking range as well as locking
objects within them. 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-20  7:28   ` Christoph Hellwig
  2016-12-20 21:55     ` Scott Bauer
@ 2016-12-20 22:07     ` Jon Derrick
  2016-12-21  9:47       ` Christoph Hellwig
  1 sibling, 1 reply; 35+ messages in thread
From: Jon Derrick @ 2016-12-20 22:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Scott Bauer, linux-nvme, keith.busch, sagi, Rafael.Antognolli,
	linux-kernel, axboe, viro

On Mon, Dec 19, 2016 at 11:28:47PM -0800, Christoph Hellwig wrote:
[snip]
> > +	while (cpos < total) {
> > +		if (!(pos[0] & 0x80)) /* tiny atom */
> > +			token_length = response_parse_tiny(iter, pos);
> > +		else if (!(pos[0] & 0x40)) /* short atom */
> > +			token_length = response_parse_short(iter, pos);
> > +		else if (!(pos[0] & 0x20)) /* medium atom */
> > +			token_length = response_parse_medium(iter, pos);
> > +		else if (!(pos[0] & 0x10)) /* long atom */
> > +			token_length = response_parse_long(iter, pos);
> 
> Please add symbolic names for these constants to the protocol header.
> 
I get tripped up by this logic every time I look at it. I'd almost
rather see something like the following, which more closely follows the
TCG core spec and the optimizing compiler should turn it into something
similar to above anyways:

	if (pos[0] <= 0x7F)
		token_length = response_parse_tiny..
	else if (pos[0] <= 0xBF)
		token_length = response_parse_short..
	else if (pos[0] <= 0xDF)
		token_length = response_parse_medium..
	else if (pos[0] <= 0xE3)
		token_length = response_parse_long..

Then just add those 0x7F, 0xBF, 0xDF, and 0xE3 constants to proto
header. We could even add in a TCG reserved error for 0xE4-0xEF

Also instead of tracking cpos, we could subtract from total until
negative (if you make it signed). It's similar to how we do length in
nvme_setup_prps.

[snip]
> > --- /dev/null
> > +++ b/lib/sed-opal_internal.h
> 
> This pretty much seem to contain the OPAL protocol defintions, so why
> not opal_proto.h?
Since there might eventually be a whole class of opal-like sed
protocols, why does it make more sense to have opal_proto.h instead of
sed-opal.h or some variation? This is similar to how leds-*.h look to
me. Although I agree that sed-ATA.h would be dishonest since ATA
security doesn't imply a self-encrypting-disk.

[snip]
> > +int sed_save(struct sed_context *sed_ctx, struct sed_key *key)
> > +{
> > +	switch (key->sed_type) {
> > +	case OPAL_LOCK_UNLOCK:
> > +		return opal_save(sed_ctx, key);
> > +	}
> > +
> > +	return -EOPNOTSUPP;
> 
> It seems to me that we should skip this whole sed_type indirections
> and just specify the ioctls directly for OPAL (which would include
> opalite and pyrite as subsets).  The only other protocols of interest
> for Linux would be the ATA "security" plain text passwords, which can
> be handled differently, or enterprise SSC which we can hopefully avoid
> to implement entirely.
I'm on board with this if you think we won't have enough different, but
similar, SED protocols to justify the indirection. In that case you can
ignore the above comment as well.

> 
> > +
> > +int fdev_sed_ioctl(struct file *filep, unsigned int cmd,
> > +		   unsigned long arg)
> > +{
> > +	struct sed_key key;
> > +	struct sed_context *sed_ctx;
> > +
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return -EACCES;
> > +
> > +	if (!filep->f_sedctx || !filep->f_sedctx->ops || !filep->f_sedctx->dev)
> > +		return -ENODEV;
> 
> In the previous version this was called from the block driver which
> could pass in the context (and ops).  Why was this changed?
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20 16:05             ` Scott Bauer
@ 2016-12-21  9:01               ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-21  9:01 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Christoph Hellwig, Keith Busch, axboe, sagi, Rafael.Antognolli,
	linux-kernel, linux-nvme, Scott Bauer, jonathan.derrick, viro

On Tue, Dec 20, 2016 at 09:05:32AM -0700, Scott Bauer wrote:
> Thanks Keith. Although TCG Spec currently ignores it in the future it may not.
> In that case we should probably attempt to future proof it a bit. Since the
> namespace ID is accessible via the block device structure I'll find a way to
> include that in some opaque pointer that we can deliver through the core into
> NVMe.

Honestly I think this nsid issue is a major, major mess.  Without a nsid
only the global range makes sense, and in that case it does not make
sense to set a nsid (so it should be 0xFFFFFFFF).  This one more of the
major major issues because the NVMe group decided to not care about
security and outsourced it to that giant heap of crap called TCG.

I think we'll need to start a little conversation on the nvme list on
how to handle this, including potential errata.

> But this also brings up another question (and part of the reason I moved from
> the block ioctl to fs ioctl): For drives with multiple namsepaces is it 
> acceptable to allow a namespace, who has a segregated chunk of space, the ability
> to perform actions outside of its range? Since the multiple namespaces portion
> of the spec says there will be one Global LR a namespace that doesn't encompass
> the entire LBA range can end up locking other LBAs via locking the global.
> 
> That's why I wanted to go to char dev because it seemed like a better fit for 
> this scenario. Any thoughts on the above?

In SCSI we allow all kinds of ioctls affecting target-wide behavior on
the device nodes.  These needs to be protected using CAP_SYS_ADMIN,
otherwise we're going to get security issues, though (and in SCSI land
we had such issues before).

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-20 17:52             ` Scott Bauer
@ 2016-12-21  9:37               ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-21  9:37 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Christoph Hellwig, Keith Busch, linux-nvme, Rafael.Antognolli,
	axboe, jonathan.derrick, viro, linux-kernel, sagi

On Tue, Dec 20, 2016 at 10:52:41AM -0700, Scott Bauer wrote:
> 2) Do we want to continue passing around a sed_context to the core?
> Instead of a block_device struct like we did in previous versions.

My personal preference would be the block_device, but in this case my
preference collides with the (sad) reality, and that reality is that TCG
did a major, major fuckup in specifyiong the NVMe interaction in the
SIIG and does not treat TPer as per-Namespaces, as it does in SCSI for
LUNs.  So in NVMe all the interactions are on a per-controller level,
not per namespaces.  And the block_device maps to the namespacespace in
NVMe.  So I fear we'll have to keep the sed_context.

>  2a) If we do wish to do wish to continue passng sed_contexts to the core I
>  have to add a new variable to the block_device structure for our sed_context.
>  Will this be acceptable?

We have a lot less block_device structures, and they are limited to
block devices, so I think it should be acceptable.

>  It wasn't acceptable for the file struct. The reason
>  I need a new variable in the struct is:
>  On the ioctl path, if we intercept the SED call in the block layer ioctl and
>  have the call chain be:
>  uland -> blk_ioctl -> sed_ioctl() -> sedcore -> sec_send/recv -> nvme

But I really don't think we strictly need it for this reason.  The above
callchain should be:

	userland -> blk_ioctl -> nvme_ioctl -> sed_opal_iocl

This allows passing the sec context including the submission method
to sed_opal_ioctl from the driver and should not require anything
in the block device.

>  then I need to be able to pass a sed_ctx struct in blk_ioctl to sed-ioctl and
>  the only  way is to have it sitting in our block_device structure.
> 
>  The other way which was sorta nack'd last time is the following call chain:
>  uland -> blk_ioctl -> nvme_ioctl -> sed_ioctl -> sedcore -> send/rcv -> nvme

Why was this nacked?  This is still my preference, except that it could
still be simplified a bit per the other comments, e.g. I don't think
we really need a sec_core.

>  In this call chain in nvme_ioctl we have access to our block device struct
>  and from there we can do blkdev->bd_disk->private_data to get our ns and then
>  eventually our sed_ctx to pass to sed_ioctl. I could add the ns to the sec_data
>  pointer in sed_context. This would give us access to ns without having to pass
>  around a block device or store it anywhere.

> 3) For NVMe we need access to our ns ID. It's in the block_device behind a few
> pointers. What I can do is if we want to continue with the first ioctl path
> described above is something like:

The think is the ns does not matter for the OPAL device.  I suspect
the right answer is to pass 0xffffffff as the nsid.  I need to verify
this with some devices I should have access to, and you should verity
it with Intel.  I've also kicked a mail to some people involved with TCG
to see if we can get some agreement on this behavior.  If we can't get
agreement on that we'll just need to have a variable in the nvme_ctrl
that stores some valid nsid just for security send/receive.

> While this works for NVMe I don't know if this is acceptible for *all* users.

The other users are SCSI, ATA and eMMC.

For SCSI the scsi_device is per-lun, and the SIIS specifies that all
TPers are per-lun, so we'll simply have a context per scsi_device,
otherwise it should work the same as NVMe.  ATA doesn't support LUNs
(except for ATAPI, which is SCSI over ATA), so it's even simpler.
Additionally Linux hides ATA behind the SCSI layer, so the only thing
we'll need to implement is the translation between the two.  I don't
really know enough about eMMC to comment on it.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-20 21:55     ` Scott Bauer
@ 2016-12-21  9:42       ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-21  9:42 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Christoph Hellwig, linux-nvme, keith.busch, sagi,
	Rafael.Antognolli, linux-kernel, axboe, viro, jonathan.derrick

On Tue, Dec 20, 2016 at 02:55:10PM -0700, Scott Bauer wrote:
> Do you mean from nvme_ioctl? or from blk_ioctl? It was removed from  blk_ioctl
> because I must have misinterpreted your comments here:
> http://lists.infradead.org/pipermail/linux-nvme/2016-December/007364.html

nvme_ioctl, aka the ioctl method of the block driver.  In the future
I expect another caller in scsi_ioctl or sd_ioctl.

> I took your hesitation about the block_device to mean try something new,
> that combined with my concern about having namespaces have the ability to
> lock the global range which can span ourside their LBA ranges lead me to
> remove it from block and put it in the char dev world.

In the link I literally meant the struct block_device, not nessecarily
the block device node.

> 
> On the same note there is a public review of
> TCG Storage Opal SSC Feature Set: Configurable Namespace Locking:
> Which Jon Derrick found for us:
> https://www.trustedcomputinggroup.org/wp-content/uploads/TCG_Storage_Feature_Set_Namespaces_phase_1b_v1_00_r1_19_public-review.pdf
> 
> Where they are thinking about doing a complete 180 from:
> https://www.trustedcomputinggroup.org/wp-content/uploads/TCG_SWG_SIIS_Version_1_05_Revision_1_00.pdf
> 
> And now Namespaces can have their own global locking range as well as locking
> objects within them. 

Interesting.  From a quick look this is how it should have been done
from the start.  Now the big question is when devices are going to
implement it, and how we can support both from the same driver, as this
means we'll have to deal with two different ways to allocate the
security context based on the device.  And do the horrible mess that the
TCG SIISs are there is absolute no way to discover this at the NVMe
level, but we actually need to start doing TCG method calls to even
figure out what's going on.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 2/5] lib: Add Sed-opal library
  2016-12-20 22:07     ` Jon Derrick
@ 2016-12-21  9:47       ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-21  9:47 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Christoph Hellwig, Scott Bauer, linux-nvme, keith.busch, sagi,
	Rafael.Antognolli, linux-kernel, axboe, viro

On Tue, Dec 20, 2016 at 03:07:46PM -0700, Jon Derrick wrote:
> > This pretty much seem to contain the OPAL protocol defintions, so why
> > not opal_proto.h?
> Since there might eventually be a whole class of opal-like sed
> protocols, why does it make more sense to have opal_proto.h instead of
> sed-opal.h or some variation? This is similar to how leds-*.h look to
> me. Although I agree that sed-ATA.h would be dishonest since ATA
> security doesn't imply a self-encrypting-disk.

As far as I can tell the NVMe / SCSI / ATA security landscape looks
like:

 - ATA security - specified in ATA, kinda implicitly referenced by SPC
   and then even more implitly in NVMe.  Actually implemented in various
   NVMe consumer devices because OEMs insist on it.  Basically just
   a trivial plain text password lock/unlock.

 - TCG OPAL including the subsets OPALite and Pyrite, whereas the latter
   is just the above lock/unlock in a TCG way.

 - TCG Enterprise SSC

I think it's pretty obvious that ATA security should be something on
it's own.  OPALite and Pyrite are strict subsets of OPAL, so having them
in something named opal shouldn't be surprising.  The big question
is what to do about Enterprise SSC.  Which has some overlaps with OPAL
but also major differences, so keeping it separate if we ever have to
implement it (I hope we don't) would be best.

> I'm on board with this if you think we won't have enough different, but
> similar, SED protocols to justify the indirection. In that case you can
> ignore the above comment as well.

This goes back to the above.  The only thing that is not a strict subset
of OPAL but kinda sorta similar is TCG Enterprise SSC, but my preference
would be to ignore it, and the second best preference would be to keep
it separate.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
                     ` (3 preceding siblings ...)
  2016-12-20  6:49   ` Christoph Hellwig
@ 2016-12-25 14:15   ` Jethro Beekman
  2016-12-27 22:12     ` Scott Bauer
  4 siblings, 1 reply; 35+ messages in thread
From: Jethro Beekman @ 2016-12-25 14:15 UTC (permalink / raw)
  To: Scott Bauer, linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi

On 19-12-16 20:35, Scott Bauer wrote:
> @@ -1796,6 +1797,13 @@ static void nvme_reset_work(struct work_struct *work)
>  	if (result)
>  		goto out;
>  
> +	result = nvme_opal_initialize(&dev->ctrl);
> +	if (result)
> +		goto out;

It seems you always try to intialize OPAL even if the drive doesn't support it.
I think you should check if the device supports security commands and then see
if it supports OPAL before calling this. See e.g.
https://lkml.org/lkml/2016/6/19/139 . Ideally, this code would check all
supported protocols and initialize the appropriate security device based on that.

Jethro

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 1/5] include: Add definitions for sed
  2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
  2016-12-20  6:46   ` Christoph Hellwig
@ 2016-12-25 14:15   ` Jethro Beekman
  2016-12-27 22:14     ` Scott Bauer
  1 sibling, 1 reply; 35+ messages in thread
From: Jethro Beekman @ 2016-12-25 14:15 UTC (permalink / raw)
  To: Scott Bauer, linux-nvme
  Cc: Rafael.Antognolli, axboe, keith.busch, jonathan.derrick, viro,
	hch, linux-kernel, sagi

On 19-12-16 20:35, Scott Bauer wrote:
> diff --git a/include/linux/sed.h b/include/linux/sed.h
>
> +/*
> + * These constant values come from:
> + * TCG Storage Architecture Core Spec v2.01 r1
> + * Section: 3.3 Interface Communications
> + */
> +enum {
> +	TCG_SECP_00 = 0,

Protocol 0 is not defined by TCG. Perhaps you should reference SPC-4 section
6.30 SECURITY PROTOCOL IN command / table 265.

> diff --git a/include/uapi/linux/sed.h b/include/uapi/linux/sed.h
>
> +enum sed_key_type {
> +	OPAL,
> +	OPAL_PW,
> +	OPAL_ACT_USR,
> +	OPAL_LR_SETUP,
> +	OPAL_LOCK_UNLOCK,
> +	OPAL_MBR_DATA,
> +};
> +
> +struct sed_key {
> +	__u32 sed_type;
> +	union {
> +		struct opal_key            opal;
> +		struct opal_new_pw         opal_pw;
> +		struct opal_session_info   opal_session;
> +		struct opal_user_lr_setup  opal_lrs;
> +		struct opal_lock_unlock    opal_lk_unlk;
> +		struct opal_mbr_data       opal_mbr;
> +		/* additional command set key types */
> +	};
> +};
> +
> +#define IOC_SED_SAVE		   _IOW('p', 220, struct sed_key)
> +#define IOC_SED_LOCK_UNLOCK	   _IOW('p', 221, struct sed_key)
> +#define IOC_SED_TAKE_OWNERSHIP	   _IOW('p', 222, struct sed_key)
> +#define IOC_SED_ACTIVATE_LSP       _IOW('p', 223, struct sed_key)
> +#define IOC_SED_SET_PW             _IOW('p', 224, struct sed_key)
> +#define IOC_SED_ACTIVATE_USR       _IOW('p', 225, struct sed_key)
> +#define IOC_SED_REVERT_TPR         _IOW('p', 226, struct sed_key)
> +#define IOC_SED_LR_SETUP           _IOW('p', 227, struct sed_key)
> +#define IOC_SED_ADD_USR_TO_LR      _IOW('p', 228, struct sed_key)
> +#define IOC_SED_ENABLE_DISABLE_MBR _IOW('p', 229, struct sed_key)
> +#define IOC_SED_ERASE_LR           _IOW('p', 230, struct sed_key)
> +#define IOC_SED_SECURE_ERASE_LR    _IOW('p', 231, struct sed_key)

I'm slightly confused by the split between SED-generic and OPAL-specific files
here. Maybe I'm misunderstanding the intent of these ioctls. I think SED means
"possible any drive supporting the security command set". Therefore these
definitions (quoted) should have OPAL names and live in an OPAL header.

Jethro

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-25 14:15   ` Jethro Beekman
@ 2016-12-27 22:12     ` Scott Bauer
  2016-12-28  8:39       ` Christoph Hellwig
  0 siblings, 1 reply; 35+ messages in thread
From: Scott Bauer @ 2016-12-27 22:12 UTC (permalink / raw)
  To: Jethro Beekman
  Cc: linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi

On Sun, Dec 25, 2016 at 03:15:52PM +0100, Jethro Beekman wrote:
> On 19-12-16 20:35, Scott Bauer wrote:
> > @@ -1796,6 +1797,13 @@ static void nvme_reset_work(struct work_struct *work)
> >  	if (result)
> >  		goto out;
> >  
> > +	result = nvme_opal_initialize(&dev->ctrl);
> > +	if (result)
> > +		goto out;
> 
> It seems you always try to intialize OPAL even if the drive doesn't support it.
> I think you should check if the device supports security commands and then see
> if it supports OPAL before calling this. See e.g.
> https://lkml.org/lkml/2016/6/19/139 . Ideally, this code would check all
> supported protocols and initialize the appropriate security device based on that.

The nvme_opal_initalize should probably be changed to nvme_opal_allocate or something.
It's not really initalizing anything other than allocting the necessary structures
for OPAL. In order to determine if the controller supports opal we need to allocate
the previously mentioned structures anyway. I want to stay away from making payloads
(specifically discovery0 ) payload in the nvme driver and allow the opal core to do a
all the grunt work. In the future we'll probably have to refactor the core a bit to do
just packet generation. It looks like at least for NVMe we're going to have to do a discovery
to figure out whether we've got mutiple locking ranges per NS or just one global lr during
initialization.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 1/5] include: Add definitions for sed
  2016-12-25 14:15   ` Jethro Beekman
@ 2016-12-27 22:14     ` Scott Bauer
  0 siblings, 0 replies; 35+ messages in thread
From: Scott Bauer @ 2016-12-27 22:14 UTC (permalink / raw)
  To: Jethro Beekman
  Cc: linux-nvme, Rafael.Antognolli, axboe, keith.busch,
	jonathan.derrick, viro, hch, linux-kernel, sagi

On Sun, Dec 25, 2016 at 03:15:53PM +0100, Jethro Beekman wrote:
> On 19-12-16 20:35, Scott Bauer wrote:
> > diff --git a/include/linux/sed.h b/include/linux/sed.h
> >
> > +/*
> > + * These constant values come from:
> > + * TCG Storage Architecture Core Spec v2.01 r1
> > + * Section: 3.3 Interface Communications
> > + */
> > +enum {
> > +	TCG_SECP_00 = 0,
> 
> Protocol 0 is not defined by TCG. Perhaps you should reference SPC-4 section
> 6.30 SECURITY PROTOCOL IN command / table 265.

Thanks, I've added this.

> 
> > diff --git a/include/uapi/linux/sed.h b/include/uapi/linux/sed.h
> >
> > +enum sed_key_type {
> > +	OPAL,
> > +	OPAL_PW,
> > +	OPAL_ACT_USR,
> > +	OPAL_LR_SETUP,
> > +	OPAL_LOCK_UNLOCK,
> > +	OPAL_MBR_DATA,
> > +};
> > +
> > +struct sed_key {
> > +	__u32 sed_type;
> > +	union {
> > +		struct opal_key            opal;
> > +		struct opal_new_pw         opal_pw;
> > +		struct opal_session_info   opal_session;
> > +		struct opal_user_lr_setup  opal_lrs;
> > +		struct opal_lock_unlock    opal_lk_unlk;
> > +		struct opal_mbr_data       opal_mbr;
> > +		/* additional command set key types */
> > +	};
> > +};
> > +
> > +#define IOC_SED_SAVE		   _IOW('p', 220, struct sed_key)
> > +#define IOC_SED_LOCK_UNLOCK	   _IOW('p', 221, struct sed_key)
> > +#define IOC_SED_TAKE_OWNERSHIP	   _IOW('p', 222, struct sed_key)
> > +#define IOC_SED_ACTIVATE_LSP       _IOW('p', 223, struct sed_key)
> > +#define IOC_SED_SET_PW             _IOW('p', 224, struct sed_key)
> > +#define IOC_SED_ACTIVATE_USR       _IOW('p', 225, struct sed_key)
> > +#define IOC_SED_REVERT_TPR         _IOW('p', 226, struct sed_key)
> > +#define IOC_SED_LR_SETUP           _IOW('p', 227, struct sed_key)
> > +#define IOC_SED_ADD_USR_TO_LR      _IOW('p', 228, struct sed_key)
> > +#define IOC_SED_ENABLE_DISABLE_MBR _IOW('p', 229, struct sed_key)
> > +#define IOC_SED_ERASE_LR           _IOW('p', 230, struct sed_key)
> > +#define IOC_SED_SECURE_ERASE_LR    _IOW('p', 231, struct sed_key)
> 
> I'm slightly confused by the split between SED-generic and OPAL-specific files
> here. Maybe I'm misunderstanding the intent of these ioctls. I think SED means
> "possible any drive supporting the security command set". Therefore these
> definitions (quoted) should have OPAL names and live in an OPAL header.

You're right. These are opal specific methods. I've ripped them from sed.h and put
them in the opal headers, also changed from IOC_SED to IOC_OPAL. Thanks

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code.
  2016-12-27 22:12     ` Scott Bauer
@ 2016-12-28  8:39       ` Christoph Hellwig
  0 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2016-12-28  8:39 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Jethro Beekman, keith.busch, sagi, hch, Rafael.Antognolli,
	linux-kernel, linux-nvme, axboe, viro, jonathan.derrick

> for OPAL. In order to determine if the controller supports opal we need to allocate
> the previously mentioned structures anyway. I want to stay away from making payloads
> (specifically discovery0 ) payload in the nvme driver and allow the opal core to do a
> all the grunt work. In the future we'll probably have to refactor the core a bit to do
> just packet generation. It looks like at least for NVMe we're going to have to do a discovery
> to figure out whether we've got mutiple locking ranges per NS or just one global lr during
> initialization.

We might have to do discovery in the nvme driver - there also is the
ATA security feature supported by various consumer drives.  Jethro has
been doing some work in that direction, although driven by userspace
for now. 

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2016-12-28  8:40 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 19:35 [PATCH v3 0/5] SED OPAL Library Scott Bauer
2016-12-19 19:35 ` [PATCH v3 1/5] include: Add definitions for sed Scott Bauer
2016-12-20  6:46   ` Christoph Hellwig
2016-12-25 14:15   ` Jethro Beekman
2016-12-27 22:14     ` Scott Bauer
2016-12-19 19:35 ` [PATCH v3 2/5] lib: Add Sed-opal library Scott Bauer
2016-12-19 21:34   ` Keith Busch
2016-12-20  6:07     ` Christoph Hellwig
2016-12-20  3:21   ` kbuild test robot
2016-12-20  3:48   ` kbuild test robot
2016-12-20  6:50   ` Al Viro
2016-12-20  7:28   ` Christoph Hellwig
2016-12-20 21:55     ` Scott Bauer
2016-12-21  9:42       ` Christoph Hellwig
2016-12-20 22:07     ` Jon Derrick
2016-12-21  9:47       ` Christoph Hellwig
2016-12-19 19:35 ` [PATCH v3 3/5] fs: Wire up SED/Opal to ioctl Scott Bauer
2016-12-20  6:21   ` Christoph Hellwig
2016-12-19 19:35 ` [PATCH v3 4/5] nvme: Implement resume_from_suspend and SED Allocation code Scott Bauer
2016-12-19 21:59   ` Keith Busch
2016-12-19 22:23     ` Scott Bauer
2016-12-20  6:17       ` Christoph Hellwig
2016-12-20 15:49         ` Keith Busch
2016-12-20 15:46           ` Christoph Hellwig
2016-12-20 16:05             ` Scott Bauer
2016-12-21  9:01               ` Christoph Hellwig
2016-12-20 17:52             ` Scott Bauer
2016-12-21  9:37               ` Christoph Hellwig
2016-12-20  4:11   ` kbuild test robot
2016-12-20  6:21   ` Christoph Hellwig
2016-12-20  6:49   ` Christoph Hellwig
2016-12-25 14:15   ` Jethro Beekman
2016-12-27 22:12     ` Scott Bauer
2016-12-28  8:39       ` Christoph Hellwig
2016-12-19 19:35 ` [PATCH v3 5/5] Maintainers: Add Information for SED Opal library Scott Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).