All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv4 0/4] OPAL patches, cont'd
@ 2017-02-21 18:59 ` Jon Derrick
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)
  Cc: Jens Axboe, Rafael Antognolli, linux-nvme, linux-block,
	Jon Derrick, Christoph Hellwig, Scott Bauer

v3->v4:
Passes in 'lock_held' into opal_lock_unlock() so we don't need to
mutex_trylock(). I wasn't totally confident in that approach anyways.

v2->v3:
Squashed 5/5 into 4/5
Changed opal_step structs back to const
Cleaned up opal_lock_unlock

v1->v2:
Moved misplaced code from 5/5 to 4/5

The first three in the series have been reviewed already but I wanted to
squash them with the last patch since they haven't been pulled yet.

The last patch is an attempt to refactor some of the awkward func data
and state separation into a common struct

Jon Derrick (4):
  block/sed: Use ssize_t on atom parsers to return errors
  block/sed: Add helper to qualify response tokens
  block/sed: Check received header lengths
  block/sed: Embed function data into the function sequence

 block/sed-opal.c | 552 +++++++++++++++++++++++--------------------------------
 1 file changed, 227 insertions(+), 325 deletions(-)

-- 
1.8.3.1


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

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 0/4] OPAL patches, cont'd
@ 2017-02-21 18:59 ` Jon Derrick
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)


v3->v4:
Passes in 'lock_held' into opal_lock_unlock() so we don't need to
mutex_trylock(). I wasn't totally confident in that approach anyways.

v2->v3:
Squashed 5/5 into 4/5
Changed opal_step structs back to const
Cleaned up opal_lock_unlock

v1->v2:
Moved misplaced code from 5/5 to 4/5

The first three in the series have been reviewed already but I wanted to
squash them with the last patch since they haven't been pulled yet.

The last patch is an attempt to refactor some of the awkward func data
and state separation into a common struct

Jon Derrick (4):
  block/sed: Use ssize_t on atom parsers to return errors
  block/sed: Add helper to qualify response tokens
  block/sed: Check received header lengths
  block/sed: Embed function data into the function sequence

 block/sed-opal.c | 552 +++++++++++++++++++++++--------------------------------
 1 file changed, 227 insertions(+), 325 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors
  2017-02-21 18:59 ` Jon Derrick
  (?)
@ 2017-02-21 18:59 ` Jon Derrick
  -1 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)


The short atom parser can return an errno from decoding but does not
currently return the error as a signed value. Convert all of the parsers
to ssize_t.

Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
Reviewed-by: Scott Bauer <scott.bauer at intel.com>
---
 block/sed-opal.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index d1c52ba..4675fd8 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -706,8 +706,8 @@ static enum opal_token response_get_token(const struct parsed_resp *resp,
 	return tok->pos[0];
 }
 
-static size_t response_parse_tiny(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
+				   const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -723,8 +723,8 @@ static size_t response_parse_tiny(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_short(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_short(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = (pos[0] & SHORT_ATOM_LEN_MASK) + 1;
@@ -736,7 +736,7 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 		tok->type = OPAL_DTA_TOKENID_SINT;
 	} else {
 		u64 u_integer = 0;
-		int i, b = 0;
+		ssize_t i, b = 0;
 
 		tok->type = OPAL_DTA_TOKENID_UINT;
 		if (tok->len > 9) {
@@ -753,8 +753,8 @@ static size_t response_parse_short(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_medium(struct opal_resp_tok *tok,
-				    const u8 *pos)
+static ssize_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;
@@ -770,8 +770,8 @@ static size_t response_parse_medium(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_long(struct opal_resp_tok *tok,
-				  const u8 *pos)
+static ssize_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;
@@ -787,8 +787,8 @@ static size_t response_parse_long(struct opal_resp_tok *tok,
 	return tok->len;
 }
 
-static size_t response_parse_token(struct opal_resp_tok *tok,
-				   const u8 *pos)
+static ssize_t response_parse_token(struct opal_resp_tok *tok,
+				    const u8 *pos)
 {
 	tok->pos = pos;
 	tok->len = 1;
@@ -805,7 +805,7 @@ static int response_parse(const u8 *buf, size_t length,
 	struct opal_resp_tok *iter;
 	int num_entries = 0;
 	int total;
-	size_t token_length;
+	ssize_t token_length;
 	const u8 *pos;
 
 	if (!buf)
@@ -851,8 +851,8 @@ static int response_parse(const u8 *buf, size_t length,
 		else /* TOKEN */
 			token_length = response_parse_token(iter, pos);
 
-		if (token_length == -EINVAL)
-			return -EINVAL;
+		if (token_length < 0)
+			return token_length;
 
 		pos += token_length;
 		total -= token_length;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCHv4 2/4] block/sed: Add helper to qualify response tokens
  2017-02-21 18:59 ` Jon Derrick
  (?)
  (?)
@ 2017-02-21 18:59 ` Jon Derrick
  -1 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)


Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.

Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
Reviewed-by: Scott Bauer <scott.bauer at intel.com>
---
 block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4675fd8..d3d6db2 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -662,48 +662,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
 	return 0;
 }
 
-static enum opal_response_token token_type(const struct parsed_resp *resp,
-					   int n)
+static const struct opal_resp_tok *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 OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
 	tok = &resp->toks[n];
 	if (tok->len == 0) {
 		pr_err("Token length must be non-zero\n");
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
-	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];
+	return tok;
 }
 
 static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -922,20 +899,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
 	return resp->toks[n].stored.u;
 }
 
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+	if (IS_ERR(token) ||
+	    token->type != OPAL_DTA_TOKENID_TOKEN ||
+	    token->pos[0] != match)
+		return false;
+	return true;
+}
+
 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) {
+	const struct opal_resp_tok *tok;
+
+	tok = response_get_token(resp, 0);
+	if (response_token_matches(tok, 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)
+	tok = response_get_token(resp, resp->num - 5);
+	if (!response_token_matches(tok, OPAL_STARTLIST))
+		return DTAERROR_NO_METHOD_STATUS;
+
+	tok = response_get_token(resp, resp->num - 1);
+	if (!response_token_matches(tok, OPAL_ENDLIST))
 		return DTAERROR_NO_METHOD_STATUS;
 
 	return response_get_u64(resp, resp->num - 4);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCHv4 3/4] block/sed: Check received header lengths
  2017-02-21 18:59 ` Jon Derrick
                   ` (2 preceding siblings ...)
  (?)
@ 2017-02-21 18:59 ` Jon Derrick
  -1 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)


Add a buffer size check against discovery and response header lengths
before we loop over their buffers.

Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
Reviewed-by: Scott Bauer <scott.bauer at intel.com>
---
 block/sed-opal.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index d3d6db2..4fc4d7b 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -411,10 +411,17 @@ static int opal_discovery0_end(struct opal_dev *dev)
 	const struct d0_header *hdr = (struct d0_header *)dev->resp;
 	const u8 *epos = dev->resp, *cpos = dev->resp;
 	u16 comid = 0;
+	u32 hlen = be32_to_cpu(hdr->length);
 
-	print_buffer(dev->resp, be32_to_cpu(hdr->length));
+	print_buffer(dev->resp, hlen);
 
-	epos += be32_to_cpu(hdr->length); /* end of buffer */
+	if (hlen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+		pr_warn("Discovery length overflows buffer (%zu+%u)/%u\n",
+			sizeof(*hdr), hlen, IO_BUFFER_LENGTH);
+		return -EFAULT;
+	}
+
+	epos += hlen; /* end of buffer */
 	cpos += sizeof(*hdr); /* current position on buffer */
 
 	while (cpos < epos && supported) {
@@ -784,6 +791,7 @@ static int response_parse(const u8 *buf, size_t length,
 	int total;
 	ssize_t token_length;
 	const u8 *pos;
+	u32 clen, plen, slen;
 
 	if (!buf)
 		return -EFAULT;
@@ -795,17 +803,16 @@ static int response_parse(const u8 *buf, size_t length,
 	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));
+	clen = be32_to_cpu(hdr->cp.length);
+	plen = be32_to_cpu(hdr->pkt.length);
+	slen = be32_to_cpu(hdr->subpkt.length);
+	pr_debug("Response size: cp: %u, pkt: %u, subpkt: %u\n",
+		 clen, plen, slen);
+
+	if (clen == 0 || plen == 0 || slen == 0 ||
+	    slen > IO_BUFFER_LENGTH - sizeof(*hdr)) {
+		pr_err("Bad header length. cp: %u, pkt: %u, subpkt: %u\n",
+		       clen, plen, slen);
 		print_buffer(pos, sizeof(*hdr));
 		return -EINVAL;
 	}
@@ -814,7 +821,7 @@ static int response_parse(const u8 *buf, size_t length,
 		return -EFAULT;
 
 	iter = resp->toks;
-	total = be32_to_cpu(hdr->subpkt.length);
+	total = slen;
 	print_buffer(pos, total);
 	while (total > 0) {
 		if (pos[0] <= TINY_ATOM_BYTE) /* tiny atom */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-21 18:59 ` Jon Derrick
@ 2017-02-21 18:59   ` Jon Derrick
  -1 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)
  Cc: Jon Derrick, linux-block, linux-nvme, Scott Bauer,
	Rafael Antognolli, Jens Axboe, Christoph Hellwig

By embedding the function data with the function sequence, we can
eliminate the external function data and state variable code. It also
made obvious some other small cleanups.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 428 ++++++++++++++++++++++---------------------------------
 1 file changed, 167 insertions(+), 261 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4fc4d7b..bd8605b 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,7 +34,11 @@
 #define IO_BUFFER_LENGTH 2048
 #define MAX_TOKS 64
 
-typedef int (*opal_step)(struct opal_dev *dev);
+struct opal_step {
+	int (*fn)(struct opal_dev *dev, void *data);
+	void *data;
+};
+typedef int (cont_fn)(struct opal_dev *dev);
 
 enum opal_atom_width {
 	OPAL_WIDTH_TINY,
@@ -80,9 +84,7 @@ struct opal_dev {
 	void *data;
 	sec_send_recv *send_recv;
 
-	const opal_step *funcs;
-	void **func_data;
-	int state;
+	const struct opal_step *steps;
 	struct mutex dev_lock;
 	u16 comid;
 	u32 hsn;
@@ -213,8 +215,6 @@ struct opal_dev {
 		{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
 };
 
-typedef int (cont_fn)(struct opal_dev *dev);
-
 static int end_opal_session_error(struct opal_dev *dev);
 
 struct opal_suspend_data {
@@ -375,18 +375,18 @@ static void check_geometry(struct opal_dev *dev, const void *data)
 
 static int next(struct opal_dev *dev)
 {
-	opal_step func;
-	int error = 0;
+	const struct opal_step *step;
+	int state = 0, error = 0;
 
 	do {
-		func = dev->funcs[dev->state];
-		if (!func)
+		step = &dev->steps[state];
+		if (!step->fn)
 			break;
 
-		error = func(dev);
+		error = step->fn(dev, step->data);
 		if (error) {
 			pr_err("Error on step function: %d with error %d: %s\n",
-			       dev->state, error,
+			       state, error,
 			       opal_error_to_human(error));
 
 			/* For each OPAL command we do a discovery0 then we
@@ -396,10 +396,10 @@ static int next(struct opal_dev *dev)
 			 * session. Therefore we shouldn't attempt to terminate
 			 * a session, as one has not yet been created.
 			 */
-			if (dev->state > 1)
+			if (state > 1)
 				return end_opal_session_error(dev);
 		}
-		dev->state++;
+		state++;
 	} while (!error);
 
 	return error;
@@ -483,7 +483,7 @@ static int opal_discovery0_end(struct opal_dev *dev)
 	return 0;
 }
 
-static int opal_discovery0(struct opal_dev *dev)
+static int opal_discovery0(struct opal_dev *dev, void *data)
 {
 	int ret;
 
@@ -1018,7 +1018,7 @@ static int finalize_and_send(struct opal_dev *dev, cont_fn cont)
 	return opal_send_recv(dev, cont);
 }
 
-static int gen_key(struct opal_dev *dev)
+static int gen_key(struct opal_dev *dev, void *data)
 {
 	const u8 *method;
 	u8 uid[OPAL_UID_LENGTH];
@@ -1072,15 +1072,14 @@ static int get_active_key_cont(struct opal_dev *dev)
 	return 0;
 }
 
-static int get_active_key(struct opal_dev *dev)
+static int get_active_key(struct opal_dev *dev, void *data)
 {
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
-	u8 *lr;
+	u8 *lr = data;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
-	lr = dev->func_data[dev->state];
 
 	err = build_locking_range(uid, sizeof(uid), *lr);
 	if (err)
@@ -1163,17 +1162,16 @@ static inline int enable_global_lr(struct opal_dev *dev, u8 *uid,
 	return err;
 }
 
-static int setup_locking_range(struct opal_dev *dev)
+static int setup_locking_range(struct opal_dev *dev, void *data)
 {
 	u8 uid[OPAL_UID_LENGTH];
-	struct opal_user_lr_setup *setup;
+	struct opal_user_lr_setup *setup = data;
 	u8 lr;
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	setup = dev->func_data[dev->state];
 	lr = setup->session.opal_key.lr;
 	err = build_locking_range(uid, sizeof(uid), lr);
 	if (err)
@@ -1286,20 +1284,19 @@ static int start_generic_opal_session(struct opal_dev *dev,
 	return finalize_and_send(dev, start_opal_session_cont);
 }
 
-static int start_anybodyASP_opal_session(struct opal_dev *dev)
+static int start_anybodyASP_opal_session(struct opal_dev *dev, void *data)
 {
 	return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
 					  OPAL_ADMINSP_UID, NULL, 0);
 }
 
-static int start_SIDASP_opal_session(struct opal_dev *dev)
+static int start_SIDASP_opal_session(struct opal_dev *dev, void *data)
 {
 	int ret;
 	const u8 *key = dev->prev_data;
-	struct opal_key *okey;
 
 	if (!key) {
-		okey = dev->func_data[dev->state];
+		const struct opal_key *okey = data;
 		ret = start_generic_opal_session(dev, OPAL_SID_UID,
 						 OPAL_ADMINSP_UID,
 						 okey->key,
@@ -1314,22 +1311,21 @@ static int start_SIDASP_opal_session(struct opal_dev *dev)
 	return ret;
 }
 
-static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+static int start_admin1LSP_opal_session(struct opal_dev *dev, void *data)
 {
-	struct opal_key *key = dev->func_data[dev->state];
-
+	struct opal_key *key = data;
 	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)
+static int start_auth_opal_session(struct opal_dev *dev, void *data)
 {
+	struct opal_session_info *session = data;
 	u8 lk_ul_user[OPAL_UID_LENGTH];
+	size_t keylen = session->opal_key.key_len;
 	int err = 0;
 
-	struct opal_session_info *session = dev->func_data[dev->state];
-	size_t keylen = session->opal_key.key_len;
 	u8 *key = session->opal_key.key;
 	u32 hsn = GENERIC_HOST_SESSION_NUM;
 
@@ -1379,7 +1375,7 @@ static int start_auth_opal_session(struct opal_dev *dev)
 	return finalize_and_send(dev, start_opal_session_cont);
 }
 
-static int revert_tper(struct opal_dev *dev)
+static int revert_tper(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
@@ -1401,9 +1397,9 @@ static int revert_tper(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int internal_activate_user(struct opal_dev *dev)
+static int internal_activate_user(struct opal_dev *dev, void *data)
 {
-	struct opal_session_info *session = dev->func_data[dev->state];
+	struct opal_session_info *session = data;
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
 
@@ -1436,15 +1432,14 @@ static int internal_activate_user(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int erase_locking_range(struct opal_dev *dev)
+static int erase_locking_range(struct opal_dev *dev, void *data)
 {
-	struct opal_session_info *session;
+	struct opal_session_info *session = data;
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
-	session = dev->func_data[dev->state];
 
 	if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
 		return -ERANGE;
@@ -1463,9 +1458,9 @@ static int erase_locking_range(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_mbr_done(struct opal_dev *dev)
+static int set_mbr_done(struct opal_dev *dev, void *data)
 {
-	u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state];
+	u8 *mbr_done_tf = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
@@ -1481,7 +1476,7 @@ static int set_mbr_done(struct opal_dev *dev)
 	add_token_u8(&err, dev, OPAL_STARTLIST);
 	add_token_u8(&err, dev, OPAL_STARTNAME);
 	add_token_u8(&err, dev, 2); /* Done */
-	add_token_u8(&err, dev, mbr_done_tf); /* Done T or F */
+	add_token_u8(&err, dev, *mbr_done_tf); /* Done T or F */
 	add_token_u8(&err, dev, OPAL_ENDNAME);
 	add_token_u8(&err, dev, OPAL_ENDLIST);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1495,9 +1490,9 @@ static int set_mbr_done(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_mbr_enable_disable(struct opal_dev *dev)
+static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
 {
-	u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state];
+	u8 *mbr_en_dis = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
@@ -1513,7 +1508,7 @@ static int set_mbr_enable_disable(struct opal_dev *dev)
 	add_token_u8(&err, dev, OPAL_STARTLIST);
 	add_token_u8(&err, dev, OPAL_STARTNAME);
 	add_token_u8(&err, dev, 1);
-	add_token_u8(&err, dev, mbr_en_dis);
+	add_token_u8(&err, dev, *mbr_en_dis);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
 	add_token_u8(&err, dev, OPAL_ENDLIST);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1554,11 +1549,10 @@ static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
 	return err;
 }
 
-static int set_new_pw(struct opal_dev *dev)
+static int set_new_pw(struct opal_dev *dev, void *data)
 {
 	u8 cpin_uid[OPAL_UID_LENGTH];
-	struct opal_session_info *usr = dev->func_data[dev->state];
-
+	struct opal_session_info *usr = data;
 
 	memcpy(cpin_uid, opaluid[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
 
@@ -1579,10 +1573,10 @@ static int set_new_pw(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_sid_cpin_pin(struct opal_dev *dev)
+static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
 {
 	u8 cpin_uid[OPAL_UID_LENGTH];
-	struct opal_key *key = dev->func_data[dev->state];
+	struct opal_key *key = data;
 
 	memcpy(cpin_uid, opaluid[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
 
@@ -1593,18 +1587,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int add_user_to_lr(struct opal_dev *dev)
+static int add_user_to_lr(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	u8 user_uid[OPAL_UID_LENGTH];
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	lkul = dev->func_data[dev->state];
-
 	memcpy(lr_buffer, opaluid[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
 	       OPAL_UID_LENGTH);
 
@@ -1671,11 +1663,11 @@ static int add_user_to_lr(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int lock_unlock_locking_range(struct opal_dev *dev)
+static int lock_unlock_locking_range(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	const u8 *method;
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	u8 read_locked = 1, write_locked = 1;
 	int err = 0;
 
@@ -1683,7 +1675,6 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
 	set_comid(dev, dev->comid);
 
 	method = opalmethod[OPAL_SET];
-	lkul = dev->func_data[dev->state];
 	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
 				lkul->session.opal_key.lr) < 0)
 		return -ERANGE;
@@ -1735,19 +1726,18 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
 }
 
 
-static int lock_unlock_locking_range_sum(struct opal_dev *dev)
+static int lock_unlock_locking_range_sum(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	u8 read_locked = 1, write_locked = 1;
 	const u8 *method;
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	int ret;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
 	method = opalmethod[OPAL_SET];
-	lkul = dev->func_data[dev->state];
 	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
 				lkul->session.opal_key.lr) < 0)
 		return -ERANGE;
@@ -1778,9 +1768,9 @@ static int lock_unlock_locking_range_sum(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int activate_lsp(struct opal_dev *dev)
+static int activate_lsp(struct opal_dev *dev, void *data)
 {
-	struct opal_lr_act *opal_act;
+	struct opal_lr_act *opal_act = data;
 	u8 user_lr[OPAL_UID_LENGTH];
 	u8 uint_3 = 0x83;
 	int err = 0, i;
@@ -1788,8 +1778,6 @@ static int activate_lsp(struct opal_dev *dev)
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	opal_act = dev->func_data[dev->state];
-
 	add_token_u8(&err, dev, OPAL_CALL);
 	add_token_bytestring(&err, dev, opaluid[OPAL_LOCKINGSP_UID],
 			     OPAL_UID_LENGTH);
@@ -1854,7 +1842,7 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
 }
 
 /* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev)
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
@@ -1915,14 +1903,13 @@ static int get_msid_cpin_pin_cont(struct opal_dev *dev)
 	return 0;
 }
 
-static int get_msid_cpin_pin(struct opal_dev *dev)
+static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-
 	add_token_u8(&err, dev, OPAL_CALL);
 	add_token_bytestring(&err, dev, opaluid[OPAL_C_PIN_MSID],
 			     OPAL_UID_LENGTH);
@@ -1952,58 +1939,48 @@ static int get_msid_cpin_pin(struct opal_dev *dev)
 	return finalize_and_send(dev, get_msid_cpin_pin_cont);
 }
 
-static int build_end_opal_session(struct opal_dev *dev)
+static int end_opal_session(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
 	clear_opal_cmd(dev);
-
 	set_comid(dev, dev->comid);
 	add_token_u8(&err, dev, OPAL_ENDOFSESSION);
-	return err;
-}
 
-static int end_opal_session(struct opal_dev *dev)
-{
-	int ret = build_end_opal_session(dev);
-
-	if (ret < 0)
-		return ret;
+	if (err < 0)
+		return err;
 	return finalize_and_send(dev, end_session_cont);
 }
 
 static int end_opal_session_error(struct opal_dev *dev)
 {
-	const opal_step error_end_session[] = {
-		end_opal_session,
-		NULL,
+	const struct opal_step error_end_session[] = {
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	dev->funcs = error_end_session;
-	dev->state = 0;
+	dev->steps = error_end_session;
 	return next(dev);
 }
 
 static inline void setup_opal_dev(struct opal_dev *dev,
-				  const opal_step *funcs)
+				  const struct opal_step *steps)
 {
-	dev->state = 0;
-	dev->funcs = funcs;
+	dev->steps = steps;
 	dev->tsn = 0;
 	dev->hsn = 0;
-	dev->func_data = NULL;
 	dev->prev_data = NULL;
 }
 
 static int check_opal_support(struct opal_dev *dev)
 {
-	static const opal_step funcs[] = {
-		opal_discovery0,
-		NULL
+	const struct opal_step steps[] = {
+		{ opal_discovery0, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, funcs);
+	setup_opal_dev(dev, steps);
 	ret = next(dev);
 	dev->supported = !ret;
 	mutex_unlock(&dev->dev_lock);
@@ -2034,24 +2011,18 @@ struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
 static int opal_secure_erase_locking_range(struct opal_dev *dev,
 					   struct opal_session_info *opal_session)
 {
-	void *data[3] = { NULL };
-	static const opal_step erase_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		get_active_key,
-		gen_key,
-		end_opal_session,
-		NULL,
+	const struct opal_step erase_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, opal_session },
+		{ get_active_key, &opal_session->opal_key.lr },
+		{ gen_key, },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, erase_funcs);
-
-	dev->func_data = data;
-	dev->func_data[1] = opal_session;
-	dev->func_data[2] = &opal_session->opal_key.lr;
-
+	setup_opal_dev(dev, erase_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2060,23 +2031,17 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
 static int opal_erase_locking_range(struct opal_dev *dev,
 				    struct opal_session_info *opal_session)
 {
-	void *data[3] = { NULL };
-	static const opal_step erase_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		erase_locking_range,
-		end_opal_session,
-		NULL,
+	const struct opal_step erase_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, opal_session },
+		{ erase_locking_range, opal_session },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, erase_funcs);
-
-	dev->func_data = data;
-	dev->func_data[1] = opal_session;
-	dev->func_data[2] = opal_session;
-
+	setup_opal_dev(dev, erase_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2085,16 +2050,15 @@ static int opal_erase_locking_range(struct opal_dev *dev,
 static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
 					  struct opal_mbr_data *opal_mbr)
 {
-	void *func_data[6] = { NULL };
-	static 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,
+	const struct opal_step mbr_steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &opal_mbr->key },
+		{ set_mbr_done, &opal_mbr->enable_disable },
+		{ end_opal_session, },
+		{ start_admin1LSP_opal_session, &opal_mbr->key },
+		{ set_mbr_enable_disable, &opal_mbr->enable_disable },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2103,12 +2067,7 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, mbr_funcs);
-	dev->func_data = func_data;
-	dev->func_data[1] = &opal_mbr->key;
-	dev->func_data[2] = &opal_mbr->enable_disable;
-	dev->func_data[4] = &opal_mbr->key;
-	dev->func_data[5] = &opal_mbr->enable_disable;
+	setup_opal_dev(dev, mbr_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2135,13 +2094,12 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
 static int opal_add_user_to_lr(struct opal_dev *dev,
 			       struct opal_lock_unlock *lk_unlk)
 {
-	void *func_data[3] = { NULL };
-	static const opal_step funcs[] = {
-		opal_discovery0,
-		start_admin1LSP_opal_session,
-		add_user_to_lr,
-		end_opal_session,
-		NULL
+	const struct opal_step steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
+		{ add_user_to_lr, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2163,10 +2121,7 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
 	}
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, funcs);
-	dev->func_data = func_data;
-	dev->func_data[1] = &lk_unlk->session.opal_key;
-	dev->func_data[2] = lk_unlk;
+	setup_opal_dev(dev, steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2174,99 +2129,75 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
 
 static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
 {
-	void *data[2] = { NULL };
-	static const opal_step revert_funcs[] = {
-		opal_discovery0,
-		start_SIDASP_opal_session,
-		revert_tper, /* controller will terminate session */
-		NULL,
+	const struct opal_step revert_steps[] = {
+		{ opal_discovery0, },
+		{ start_SIDASP_opal_session, opal },
+		{ revert_tper, }, /* controller will terminate session */
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, revert_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = opal;
+	setup_opal_dev(dev, revert_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
 }
 
-static int __opal_lock_unlock_sum(struct opal_dev *dev)
+static int opal_lock_unlock(struct opal_dev *dev,
+			    struct opal_lock_unlock *lk_unlk, bool lock_held)
 {
-	static const opal_step ulk_funcs_sum[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		lock_unlock_locking_range_sum,
-		end_opal_session,
-		NULL
+	const struct opal_step unlock_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &lk_unlk->session },
+		{ lock_unlock_locking_range, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-
-	dev->funcs = ulk_funcs_sum;
-	return next(dev);
-}
-
-static int __opal_lock_unlock(struct opal_dev *dev)
-{
-	static const opal_step _unlock_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		lock_unlock_locking_range,
-		end_opal_session,
-		NULL
+	const struct opal_step unlock_sum_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &lk_unlk->session },
+		{ lock_unlock_locking_range_sum, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-
-	dev->funcs = _unlock_funcs;
-	return next(dev);
-}
-
-static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
-{
-	void *func_data[3] = { NULL };
 	int ret;
 
 	if (lk_unlk->session.who < OPAL_ADMIN1 ||
 	    lk_unlk->session.who > OPAL_USER9)
 		return -EINVAL;
 
-	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, NULL);
-	dev->func_data = func_data;
-	dev->func_data[1] = &lk_unlk->session;
-	dev->func_data[2] = lk_unlk;
+	if (!lock_held)
+		mutex_lock(&dev->dev_lock);
 
-	if (lk_unlk->session.sum)
-		ret = __opal_lock_unlock_sum(dev);
-	else
-		ret = __opal_lock_unlock(dev);
+	setup_opal_dev(dev, lk_unlk->session.sum ?
+		       unlock_sum_steps : unlock_steps);
+	ret = next(dev);
 
-	mutex_unlock(&dev->dev_lock);
+	if (!lock_held)
+		mutex_unlock(&dev->dev_lock);
 	return ret;
 }
 
 static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
 {
-	static 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
+	const struct opal_step owner_steps[] = {
+		{ opal_discovery0, },
+		{ start_anybodyASP_opal_session, },
+		{ get_msid_cpin_pin, },
+		{ end_opal_session, },
+		{ start_SIDASP_opal_session, opal },
+		{ set_sid_cpin_pin, opal },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	void *data[6] = { NULL };
 	int ret;
 
 	if (!dev)
 		return -ENODEV;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, owner_funcs);
-	dev->func_data = data;
-	dev->func_data[4] = opal;
-	dev->func_data[5] = opal;
+	setup_opal_dev(dev, owner_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2274,14 +2205,13 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
 
 static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_act)
 {
-	void *data[4] = { NULL };
-	static 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
+	const struct opal_step active_steps[] = {
+		{ opal_discovery0, },
+		{ start_SIDASP_opal_session, &opal_lr_act->key },
+		{ get_lsp_lifecycle, },
+		{ activate_lsp, opal_lr_act },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2289,10 +2219,7 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, active_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_lr_act->key;
-	dev->func_data[3] = opal_lr_act;
+	setup_opal_dev(dev, active_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2301,21 +2228,17 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
 static int opal_setup_locking_range(struct opal_dev *dev,
 				    struct opal_user_lr_setup *opal_lrs)
 {
-	void *data[3] = { NULL };
-	static const opal_step lr_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		setup_locking_range,
-		end_opal_session,
-		NULL,
+	const struct opal_step lr_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &opal_lrs->session },
+		{ setup_locking_range, opal_lrs },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, lr_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_lrs->session;
-	dev->func_data[2] = opal_lrs;
+	setup_opal_dev(dev, lr_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2323,14 +2246,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
 
 static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 {
-	static const opal_step pw_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		set_new_pw,
-		end_opal_session,
-		NULL
+	const struct opal_step pw_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &opal_pw->session },
+		{ set_new_pw, &opal_pw->new_user_pw },
+		{ end_opal_session, },
+		{ NULL }
 	};
-	void *data[3] = { NULL };
 	int ret;
 
 	if (opal_pw->session.who < OPAL_ADMIN1 ||
@@ -2340,11 +2262,7 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, pw_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = (void *) &opal_pw->session;
-	dev->func_data[2] = (void *) &opal_pw->new_user_pw;
-
+	setup_opal_dev(dev, pw_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2353,14 +2271,13 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 static int opal_activate_user(struct opal_dev *dev,
 			      struct opal_session_info *opal_session)
 {
-	static const opal_step act_funcs[] = {
-		opal_discovery0,
-		start_admin1LSP_opal_session,
-		internal_activate_user,
-		end_opal_session,
-		NULL
+	const struct opal_step act_steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &opal_session->opal_key },
+		{ internal_activate_user, opal_session },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	void *data[3] = { NULL };
 	int ret;
 
 	/* We can't activate Admin1 it's active as manufactured */
@@ -2371,10 +2288,7 @@ static int opal_activate_user(struct opal_dev *dev,
 	}
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, act_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_session->opal_key;
-	dev->func_data[2] = opal_session;
+	setup_opal_dev(dev, act_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2383,7 +2297,6 @@ static int opal_activate_user(struct opal_dev *dev,
 bool opal_unlock_from_suspend(struct opal_dev *dev)
 {
 	struct opal_suspend_data *suspend;
-	void *func_data[3] = { NULL };
 	bool was_failure = false;
 	int ret = 0;
 
@@ -2394,19 +2307,12 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
 
 	mutex_lock(&dev->dev_lock);
 	setup_opal_dev(dev, NULL);
-	dev->func_data = func_data;
 
 	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;
 		dev->tsn = 0;
 		dev->hsn = 0;
 
-		if (suspend->unlk.session.sum)
-			ret = __opal_lock_unlock_sum(dev);
-		else
-			ret = __opal_lock_unlock(dev);
+		ret = opal_lock_unlock(dev, &suspend->unlk, true);
 		if (ret) {
 			pr_warn("Failed to unlock LR %hhu with sum %d\n",
 				suspend->unlk.session.opal_key.lr,
@@ -2433,7 +2339,7 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 		return -ENOTSUPP;
 	}
 
-	p = memdup_user(arg,  _IOC_SIZE(cmd));
+	p = memdup_user(arg, _IOC_SIZE(cmd));
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
@@ -2442,7 +2348,7 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 		ret = opal_save(dev, p);
 		break;
 	case IOC_OPAL_LOCK_UNLOCK:
-		ret = opal_lock_unlock(dev, p);
+		ret = opal_lock_unlock(dev, p, false);
 		break;
 	case IOC_OPAL_TAKE_OWNERSHIP:
 		ret = opal_take_ownership(dev, p);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-21 18:59   ` Jon Derrick
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-21 18:59 UTC (permalink / raw)


By embedding the function data with the function sequence, we can
eliminate the external function data and state variable code. It also
made obvious some other small cleanups.

Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
---
 block/sed-opal.c | 428 ++++++++++++++++++++++---------------------------------
 1 file changed, 167 insertions(+), 261 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4fc4d7b..bd8605b 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,7 +34,11 @@
 #define IO_BUFFER_LENGTH 2048
 #define MAX_TOKS 64
 
-typedef int (*opal_step)(struct opal_dev *dev);
+struct opal_step {
+	int (*fn)(struct opal_dev *dev, void *data);
+	void *data;
+};
+typedef int (cont_fn)(struct opal_dev *dev);
 
 enum opal_atom_width {
 	OPAL_WIDTH_TINY,
@@ -80,9 +84,7 @@ struct opal_dev {
 	void *data;
 	sec_send_recv *send_recv;
 
-	const opal_step *funcs;
-	void **func_data;
-	int state;
+	const struct opal_step *steps;
 	struct mutex dev_lock;
 	u16 comid;
 	u32 hsn;
@@ -213,8 +215,6 @@ struct opal_dev {
 		{ 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x08, 0x03 },
 };
 
-typedef int (cont_fn)(struct opal_dev *dev);
-
 static int end_opal_session_error(struct opal_dev *dev);
 
 struct opal_suspend_data {
@@ -375,18 +375,18 @@ static void check_geometry(struct opal_dev *dev, const void *data)
 
 static int next(struct opal_dev *dev)
 {
-	opal_step func;
-	int error = 0;
+	const struct opal_step *step;
+	int state = 0, error = 0;
 
 	do {
-		func = dev->funcs[dev->state];
-		if (!func)
+		step = &dev->steps[state];
+		if (!step->fn)
 			break;
 
-		error = func(dev);
+		error = step->fn(dev, step->data);
 		if (error) {
 			pr_err("Error on step function: %d with error %d: %s\n",
-			       dev->state, error,
+			       state, error,
 			       opal_error_to_human(error));
 
 			/* For each OPAL command we do a discovery0 then we
@@ -396,10 +396,10 @@ static int next(struct opal_dev *dev)
 			 * session. Therefore we shouldn't attempt to terminate
 			 * a session, as one has not yet been created.
 			 */
-			if (dev->state > 1)
+			if (state > 1)
 				return end_opal_session_error(dev);
 		}
-		dev->state++;
+		state++;
 	} while (!error);
 
 	return error;
@@ -483,7 +483,7 @@ static int opal_discovery0_end(struct opal_dev *dev)
 	return 0;
 }
 
-static int opal_discovery0(struct opal_dev *dev)
+static int opal_discovery0(struct opal_dev *dev, void *data)
 {
 	int ret;
 
@@ -1018,7 +1018,7 @@ static int finalize_and_send(struct opal_dev *dev, cont_fn cont)
 	return opal_send_recv(dev, cont);
 }
 
-static int gen_key(struct opal_dev *dev)
+static int gen_key(struct opal_dev *dev, void *data)
 {
 	const u8 *method;
 	u8 uid[OPAL_UID_LENGTH];
@@ -1072,15 +1072,14 @@ static int get_active_key_cont(struct opal_dev *dev)
 	return 0;
 }
 
-static int get_active_key(struct opal_dev *dev)
+static int get_active_key(struct opal_dev *dev, void *data)
 {
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
-	u8 *lr;
+	u8 *lr = data;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
-	lr = dev->func_data[dev->state];
 
 	err = build_locking_range(uid, sizeof(uid), *lr);
 	if (err)
@@ -1163,17 +1162,16 @@ static inline int enable_global_lr(struct opal_dev *dev, u8 *uid,
 	return err;
 }
 
-static int setup_locking_range(struct opal_dev *dev)
+static int setup_locking_range(struct opal_dev *dev, void *data)
 {
 	u8 uid[OPAL_UID_LENGTH];
-	struct opal_user_lr_setup *setup;
+	struct opal_user_lr_setup *setup = data;
 	u8 lr;
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	setup = dev->func_data[dev->state];
 	lr = setup->session.opal_key.lr;
 	err = build_locking_range(uid, sizeof(uid), lr);
 	if (err)
@@ -1286,20 +1284,19 @@ static int start_generic_opal_session(struct opal_dev *dev,
 	return finalize_and_send(dev, start_opal_session_cont);
 }
 
-static int start_anybodyASP_opal_session(struct opal_dev *dev)
+static int start_anybodyASP_opal_session(struct opal_dev *dev, void *data)
 {
 	return start_generic_opal_session(dev, OPAL_ANYBODY_UID,
 					  OPAL_ADMINSP_UID, NULL, 0);
 }
 
-static int start_SIDASP_opal_session(struct opal_dev *dev)
+static int start_SIDASP_opal_session(struct opal_dev *dev, void *data)
 {
 	int ret;
 	const u8 *key = dev->prev_data;
-	struct opal_key *okey;
 
 	if (!key) {
-		okey = dev->func_data[dev->state];
+		const struct opal_key *okey = data;
 		ret = start_generic_opal_session(dev, OPAL_SID_UID,
 						 OPAL_ADMINSP_UID,
 						 okey->key,
@@ -1314,22 +1311,21 @@ static int start_SIDASP_opal_session(struct opal_dev *dev)
 	return ret;
 }
 
-static inline int start_admin1LSP_opal_session(struct opal_dev *dev)
+static int start_admin1LSP_opal_session(struct opal_dev *dev, void *data)
 {
-	struct opal_key *key = dev->func_data[dev->state];
-
+	struct opal_key *key = data;
 	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)
+static int start_auth_opal_session(struct opal_dev *dev, void *data)
 {
+	struct opal_session_info *session = data;
 	u8 lk_ul_user[OPAL_UID_LENGTH];
+	size_t keylen = session->opal_key.key_len;
 	int err = 0;
 
-	struct opal_session_info *session = dev->func_data[dev->state];
-	size_t keylen = session->opal_key.key_len;
 	u8 *key = session->opal_key.key;
 	u32 hsn = GENERIC_HOST_SESSION_NUM;
 
@@ -1379,7 +1375,7 @@ static int start_auth_opal_session(struct opal_dev *dev)
 	return finalize_and_send(dev, start_opal_session_cont);
 }
 
-static int revert_tper(struct opal_dev *dev)
+static int revert_tper(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
@@ -1401,9 +1397,9 @@ static int revert_tper(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int internal_activate_user(struct opal_dev *dev)
+static int internal_activate_user(struct opal_dev *dev, void *data)
 {
-	struct opal_session_info *session = dev->func_data[dev->state];
+	struct opal_session_info *session = data;
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
 
@@ -1436,15 +1432,14 @@ static int internal_activate_user(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int erase_locking_range(struct opal_dev *dev)
+static int erase_locking_range(struct opal_dev *dev, void *data)
 {
-	struct opal_session_info *session;
+	struct opal_session_info *session = data;
 	u8 uid[OPAL_UID_LENGTH];
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
-	session = dev->func_data[dev->state];
 
 	if (build_locking_range(uid, sizeof(uid), session->opal_key.lr) < 0)
 		return -ERANGE;
@@ -1463,9 +1458,9 @@ static int erase_locking_range(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_mbr_done(struct opal_dev *dev)
+static int set_mbr_done(struct opal_dev *dev, void *data)
 {
-	u8 mbr_done_tf = *(u8 *)dev->func_data[dev->state];
+	u8 *mbr_done_tf = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
@@ -1481,7 +1476,7 @@ static int set_mbr_done(struct opal_dev *dev)
 	add_token_u8(&err, dev, OPAL_STARTLIST);
 	add_token_u8(&err, dev, OPAL_STARTNAME);
 	add_token_u8(&err, dev, 2); /* Done */
-	add_token_u8(&err, dev, mbr_done_tf); /* Done T or F */
+	add_token_u8(&err, dev, *mbr_done_tf); /* Done T or F */
 	add_token_u8(&err, dev, OPAL_ENDNAME);
 	add_token_u8(&err, dev, OPAL_ENDLIST);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1495,9 +1490,9 @@ static int set_mbr_done(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_mbr_enable_disable(struct opal_dev *dev)
+static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
 {
-	u8 mbr_en_dis = *(u8 *)dev->func_data[dev->state];
+	u8 *mbr_en_dis = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
@@ -1513,7 +1508,7 @@ static int set_mbr_enable_disable(struct opal_dev *dev)
 	add_token_u8(&err, dev, OPAL_STARTLIST);
 	add_token_u8(&err, dev, OPAL_STARTNAME);
 	add_token_u8(&err, dev, 1);
-	add_token_u8(&err, dev, mbr_en_dis);
+	add_token_u8(&err, dev, *mbr_en_dis);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
 	add_token_u8(&err, dev, OPAL_ENDLIST);
 	add_token_u8(&err, dev, OPAL_ENDNAME);
@@ -1554,11 +1549,10 @@ static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
 	return err;
 }
 
-static int set_new_pw(struct opal_dev *dev)
+static int set_new_pw(struct opal_dev *dev, void *data)
 {
 	u8 cpin_uid[OPAL_UID_LENGTH];
-	struct opal_session_info *usr = dev->func_data[dev->state];
-
+	struct opal_session_info *usr = data;
 
 	memcpy(cpin_uid, opaluid[OPAL_C_PIN_ADMIN1], OPAL_UID_LENGTH);
 
@@ -1579,10 +1573,10 @@ static int set_new_pw(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int set_sid_cpin_pin(struct opal_dev *dev)
+static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
 {
 	u8 cpin_uid[OPAL_UID_LENGTH];
-	struct opal_key *key = dev->func_data[dev->state];
+	struct opal_key *key = data;
 
 	memcpy(cpin_uid, opaluid[OPAL_C_PIN_SID], OPAL_UID_LENGTH);
 
@@ -1593,18 +1587,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int add_user_to_lr(struct opal_dev *dev)
+static int add_user_to_lr(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	u8 user_uid[OPAL_UID_LENGTH];
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	lkul = dev->func_data[dev->state];
-
 	memcpy(lr_buffer, opaluid[OPAL_LOCKINGRANGE_ACE_RDLOCKED],
 	       OPAL_UID_LENGTH);
 
@@ -1671,11 +1663,11 @@ static int add_user_to_lr(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int lock_unlock_locking_range(struct opal_dev *dev)
+static int lock_unlock_locking_range(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	const u8 *method;
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	u8 read_locked = 1, write_locked = 1;
 	int err = 0;
 
@@ -1683,7 +1675,6 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
 	set_comid(dev, dev->comid);
 
 	method = opalmethod[OPAL_SET];
-	lkul = dev->func_data[dev->state];
 	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
 				lkul->session.opal_key.lr) < 0)
 		return -ERANGE;
@@ -1735,19 +1726,18 @@ static int lock_unlock_locking_range(struct opal_dev *dev)
 }
 
 
-static int lock_unlock_locking_range_sum(struct opal_dev *dev)
+static int lock_unlock_locking_range_sum(struct opal_dev *dev, void *data)
 {
 	u8 lr_buffer[OPAL_UID_LENGTH];
 	u8 read_locked = 1, write_locked = 1;
 	const u8 *method;
-	struct opal_lock_unlock *lkul;
+	struct opal_lock_unlock *lkul = data;
 	int ret;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
 	method = opalmethod[OPAL_SET];
-	lkul = dev->func_data[dev->state];
 	if (build_locking_range(lr_buffer, sizeof(lr_buffer),
 				lkul->session.opal_key.lr) < 0)
 		return -ERANGE;
@@ -1778,9 +1768,9 @@ static int lock_unlock_locking_range_sum(struct opal_dev *dev)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
-static int activate_lsp(struct opal_dev *dev)
+static int activate_lsp(struct opal_dev *dev, void *data)
 {
-	struct opal_lr_act *opal_act;
+	struct opal_lr_act *opal_act = data;
 	u8 user_lr[OPAL_UID_LENGTH];
 	u8 uint_3 = 0x83;
 	int err = 0, i;
@@ -1788,8 +1778,6 @@ static int activate_lsp(struct opal_dev *dev)
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-	opal_act = dev->func_data[dev->state];
-
 	add_token_u8(&err, dev, OPAL_CALL);
 	add_token_bytestring(&err, dev, opaluid[OPAL_LOCKINGSP_UID],
 			     OPAL_UID_LENGTH);
@@ -1854,7 +1842,7 @@ static int get_lsp_lifecycle_cont(struct opal_dev *dev)
 }
 
 /* Determine if we're in the Manufactured Inactive or Active state */
-static int get_lsp_lifecycle(struct opal_dev *dev)
+static int get_lsp_lifecycle(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
@@ -1915,14 +1903,13 @@ static int get_msid_cpin_pin_cont(struct opal_dev *dev)
 	return 0;
 }
 
-static int get_msid_cpin_pin(struct opal_dev *dev)
+static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
 	clear_opal_cmd(dev);
 	set_comid(dev, dev->comid);
 
-
 	add_token_u8(&err, dev, OPAL_CALL);
 	add_token_bytestring(&err, dev, opaluid[OPAL_C_PIN_MSID],
 			     OPAL_UID_LENGTH);
@@ -1952,58 +1939,48 @@ static int get_msid_cpin_pin(struct opal_dev *dev)
 	return finalize_and_send(dev, get_msid_cpin_pin_cont);
 }
 
-static int build_end_opal_session(struct opal_dev *dev)
+static int end_opal_session(struct opal_dev *dev, void *data)
 {
 	int err = 0;
 
 	clear_opal_cmd(dev);
-
 	set_comid(dev, dev->comid);
 	add_token_u8(&err, dev, OPAL_ENDOFSESSION);
-	return err;
-}
 
-static int end_opal_session(struct opal_dev *dev)
-{
-	int ret = build_end_opal_session(dev);
-
-	if (ret < 0)
-		return ret;
+	if (err < 0)
+		return err;
 	return finalize_and_send(dev, end_session_cont);
 }
 
 static int end_opal_session_error(struct opal_dev *dev)
 {
-	const opal_step error_end_session[] = {
-		end_opal_session,
-		NULL,
+	const struct opal_step error_end_session[] = {
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	dev->funcs = error_end_session;
-	dev->state = 0;
+	dev->steps = error_end_session;
 	return next(dev);
 }
 
 static inline void setup_opal_dev(struct opal_dev *dev,
-				  const opal_step *funcs)
+				  const struct opal_step *steps)
 {
-	dev->state = 0;
-	dev->funcs = funcs;
+	dev->steps = steps;
 	dev->tsn = 0;
 	dev->hsn = 0;
-	dev->func_data = NULL;
 	dev->prev_data = NULL;
 }
 
 static int check_opal_support(struct opal_dev *dev)
 {
-	static const opal_step funcs[] = {
-		opal_discovery0,
-		NULL
+	const struct opal_step steps[] = {
+		{ opal_discovery0, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, funcs);
+	setup_opal_dev(dev, steps);
 	ret = next(dev);
 	dev->supported = !ret;
 	mutex_unlock(&dev->dev_lock);
@@ -2034,24 +2011,18 @@ struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
 static int opal_secure_erase_locking_range(struct opal_dev *dev,
 					   struct opal_session_info *opal_session)
 {
-	void *data[3] = { NULL };
-	static const opal_step erase_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		get_active_key,
-		gen_key,
-		end_opal_session,
-		NULL,
+	const struct opal_step erase_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, opal_session },
+		{ get_active_key, &opal_session->opal_key.lr },
+		{ gen_key, },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, erase_funcs);
-
-	dev->func_data = data;
-	dev->func_data[1] = opal_session;
-	dev->func_data[2] = &opal_session->opal_key.lr;
-
+	setup_opal_dev(dev, erase_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2060,23 +2031,17 @@ static int opal_secure_erase_locking_range(struct opal_dev *dev,
 static int opal_erase_locking_range(struct opal_dev *dev,
 				    struct opal_session_info *opal_session)
 {
-	void *data[3] = { NULL };
-	static const opal_step erase_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		erase_locking_range,
-		end_opal_session,
-		NULL,
+	const struct opal_step erase_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, opal_session },
+		{ erase_locking_range, opal_session },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, erase_funcs);
-
-	dev->func_data = data;
-	dev->func_data[1] = opal_session;
-	dev->func_data[2] = opal_session;
-
+	setup_opal_dev(dev, erase_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2085,16 +2050,15 @@ static int opal_erase_locking_range(struct opal_dev *dev,
 static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
 					  struct opal_mbr_data *opal_mbr)
 {
-	void *func_data[6] = { NULL };
-	static 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,
+	const struct opal_step mbr_steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &opal_mbr->key },
+		{ set_mbr_done, &opal_mbr->enable_disable },
+		{ end_opal_session, },
+		{ start_admin1LSP_opal_session, &opal_mbr->key },
+		{ set_mbr_enable_disable, &opal_mbr->enable_disable },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2103,12 +2067,7 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, mbr_funcs);
-	dev->func_data = func_data;
-	dev->func_data[1] = &opal_mbr->key;
-	dev->func_data[2] = &opal_mbr->enable_disable;
-	dev->func_data[4] = &opal_mbr->key;
-	dev->func_data[5] = &opal_mbr->enable_disable;
+	setup_opal_dev(dev, mbr_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2135,13 +2094,12 @@ static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
 static int opal_add_user_to_lr(struct opal_dev *dev,
 			       struct opal_lock_unlock *lk_unlk)
 {
-	void *func_data[3] = { NULL };
-	static const opal_step funcs[] = {
-		opal_discovery0,
-		start_admin1LSP_opal_session,
-		add_user_to_lr,
-		end_opal_session,
-		NULL
+	const struct opal_step steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &lk_unlk->session.opal_key },
+		{ add_user_to_lr, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2163,10 +2121,7 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
 	}
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, funcs);
-	dev->func_data = func_data;
-	dev->func_data[1] = &lk_unlk->session.opal_key;
-	dev->func_data[2] = lk_unlk;
+	setup_opal_dev(dev, steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2174,99 +2129,75 @@ static int opal_add_user_to_lr(struct opal_dev *dev,
 
 static int opal_reverttper(struct opal_dev *dev, struct opal_key *opal)
 {
-	void *data[2] = { NULL };
-	static const opal_step revert_funcs[] = {
-		opal_discovery0,
-		start_SIDASP_opal_session,
-		revert_tper, /* controller will terminate session */
-		NULL,
+	const struct opal_step revert_steps[] = {
+		{ opal_discovery0, },
+		{ start_SIDASP_opal_session, opal },
+		{ revert_tper, }, /* controller will terminate session */
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, revert_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = opal;
+	setup_opal_dev(dev, revert_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
 }
 
-static int __opal_lock_unlock_sum(struct opal_dev *dev)
+static int opal_lock_unlock(struct opal_dev *dev,
+			    struct opal_lock_unlock *lk_unlk, bool lock_held)
 {
-	static const opal_step ulk_funcs_sum[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		lock_unlock_locking_range_sum,
-		end_opal_session,
-		NULL
+	const struct opal_step unlock_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &lk_unlk->session },
+		{ lock_unlock_locking_range, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-
-	dev->funcs = ulk_funcs_sum;
-	return next(dev);
-}
-
-static int __opal_lock_unlock(struct opal_dev *dev)
-{
-	static const opal_step _unlock_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		lock_unlock_locking_range,
-		end_opal_session,
-		NULL
+	const struct opal_step unlock_sum_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &lk_unlk->session },
+		{ lock_unlock_locking_range_sum, lk_unlk },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-
-	dev->funcs = _unlock_funcs;
-	return next(dev);
-}
-
-static int opal_lock_unlock(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
-{
-	void *func_data[3] = { NULL };
 	int ret;
 
 	if (lk_unlk->session.who < OPAL_ADMIN1 ||
 	    lk_unlk->session.who > OPAL_USER9)
 		return -EINVAL;
 
-	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, NULL);
-	dev->func_data = func_data;
-	dev->func_data[1] = &lk_unlk->session;
-	dev->func_data[2] = lk_unlk;
+	if (!lock_held)
+		mutex_lock(&dev->dev_lock);
 
-	if (lk_unlk->session.sum)
-		ret = __opal_lock_unlock_sum(dev);
-	else
-		ret = __opal_lock_unlock(dev);
+	setup_opal_dev(dev, lk_unlk->session.sum ?
+		       unlock_sum_steps : unlock_steps);
+	ret = next(dev);
 
-	mutex_unlock(&dev->dev_lock);
+	if (!lock_held)
+		mutex_unlock(&dev->dev_lock);
 	return ret;
 }
 
 static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
 {
-	static 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
+	const struct opal_step owner_steps[] = {
+		{ opal_discovery0, },
+		{ start_anybodyASP_opal_session, },
+		{ get_msid_cpin_pin, },
+		{ end_opal_session, },
+		{ start_SIDASP_opal_session, opal },
+		{ set_sid_cpin_pin, opal },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	void *data[6] = { NULL };
 	int ret;
 
 	if (!dev)
 		return -ENODEV;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, owner_funcs);
-	dev->func_data = data;
-	dev->func_data[4] = opal;
-	dev->func_data[5] = opal;
+	setup_opal_dev(dev, owner_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2274,14 +2205,13 @@ static int opal_take_ownership(struct opal_dev *dev, struct opal_key *opal)
 
 static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_act)
 {
-	void *data[4] = { NULL };
-	static 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
+	const struct opal_step active_steps[] = {
+		{ opal_discovery0, },
+		{ start_SIDASP_opal_session, &opal_lr_act->key },
+		{ get_lsp_lifecycle, },
+		{ activate_lsp, opal_lr_act },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
@@ -2289,10 +2219,7 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, active_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_lr_act->key;
-	dev->func_data[3] = opal_lr_act;
+	setup_opal_dev(dev, active_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2301,21 +2228,17 @@ static int opal_activate_lsp(struct opal_dev *dev, struct opal_lr_act *opal_lr_a
 static int opal_setup_locking_range(struct opal_dev *dev,
 				    struct opal_user_lr_setup *opal_lrs)
 {
-	void *data[3] = { NULL };
-	static const opal_step lr_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		setup_locking_range,
-		end_opal_session,
-		NULL,
+	const struct opal_step lr_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &opal_lrs->session },
+		{ setup_locking_range, opal_lrs },
+		{ end_opal_session, },
+		{ NULL, }
 	};
 	int ret;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, lr_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_lrs->session;
-	dev->func_data[2] = opal_lrs;
+	setup_opal_dev(dev, lr_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2323,14 +2246,13 @@ static int opal_setup_locking_range(struct opal_dev *dev,
 
 static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 {
-	static const opal_step pw_funcs[] = {
-		opal_discovery0,
-		start_auth_opal_session,
-		set_new_pw,
-		end_opal_session,
-		NULL
+	const struct opal_step pw_steps[] = {
+		{ opal_discovery0, },
+		{ start_auth_opal_session, &opal_pw->session },
+		{ set_new_pw, &opal_pw->new_user_pw },
+		{ end_opal_session, },
+		{ NULL }
 	};
-	void *data[3] = { NULL };
 	int ret;
 
 	if (opal_pw->session.who < OPAL_ADMIN1 ||
@@ -2340,11 +2262,7 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, pw_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = (void *) &opal_pw->session;
-	dev->func_data[2] = (void *) &opal_pw->new_user_pw;
-
+	setup_opal_dev(dev, pw_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2353,14 +2271,13 @@ static int opal_set_new_pw(struct opal_dev *dev, struct opal_new_pw *opal_pw)
 static int opal_activate_user(struct opal_dev *dev,
 			      struct opal_session_info *opal_session)
 {
-	static const opal_step act_funcs[] = {
-		opal_discovery0,
-		start_admin1LSP_opal_session,
-		internal_activate_user,
-		end_opal_session,
-		NULL
+	const struct opal_step act_steps[] = {
+		{ opal_discovery0, },
+		{ start_admin1LSP_opal_session, &opal_session->opal_key },
+		{ internal_activate_user, opal_session },
+		{ end_opal_session, },
+		{ NULL, }
 	};
-	void *data[3] = { NULL };
 	int ret;
 
 	/* We can't activate Admin1 it's active as manufactured */
@@ -2371,10 +2288,7 @@ static int opal_activate_user(struct opal_dev *dev,
 	}
 
 	mutex_lock(&dev->dev_lock);
-	setup_opal_dev(dev, act_funcs);
-	dev->func_data = data;
-	dev->func_data[1] = &opal_session->opal_key;
-	dev->func_data[2] = opal_session;
+	setup_opal_dev(dev, act_steps);
 	ret = next(dev);
 	mutex_unlock(&dev->dev_lock);
 	return ret;
@@ -2383,7 +2297,6 @@ static int opal_activate_user(struct opal_dev *dev,
 bool opal_unlock_from_suspend(struct opal_dev *dev)
 {
 	struct opal_suspend_data *suspend;
-	void *func_data[3] = { NULL };
 	bool was_failure = false;
 	int ret = 0;
 
@@ -2394,19 +2307,12 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
 
 	mutex_lock(&dev->dev_lock);
 	setup_opal_dev(dev, NULL);
-	dev->func_data = func_data;
 
 	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;
 		dev->tsn = 0;
 		dev->hsn = 0;
 
-		if (suspend->unlk.session.sum)
-			ret = __opal_lock_unlock_sum(dev);
-		else
-			ret = __opal_lock_unlock(dev);
+		ret = opal_lock_unlock(dev, &suspend->unlk, true);
 		if (ret) {
 			pr_warn("Failed to unlock LR %hhu with sum %d\n",
 				suspend->unlk.session.opal_key.lr,
@@ -2433,7 +2339,7 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 		return -ENOTSUPP;
 	}
 
-	p = memdup_user(arg,  _IOC_SIZE(cmd));
+	p = memdup_user(arg, _IOC_SIZE(cmd));
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
@@ -2442,7 +2348,7 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 		ret = opal_save(dev, p);
 		break;
 	case IOC_OPAL_LOCK_UNLOCK:
-		ret = opal_lock_unlock(dev, p);
+		ret = opal_lock_unlock(dev, p, false);
 		break;
 	case IOC_OPAL_TAKE_OWNERSHIP:
 		ret = opal_take_ownership(dev, p);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-21 18:59   ` Jon Derrick
@ 2017-02-21 23:03     ` Scott Bauer
  -1 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-21 23:03 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-block, linux-nvme, Rafael Antognolli, Jens Axboe,
	Christoph Hellwig

On Tue, Feb 21, 2017 at 11:59:16AM -0700, Jon Derrick wrote:
> By embedding the function data with the function sequence, we can
> eliminate the external function data and state variable code. It also
> made obvious some other small cleanups.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
Reviewed-by: Scott Bauer <scott.bauer@intel.com>

Also Christoph had reviewed-by on 1-3 on friday, I dont think we need a respin,
but wanted to point that out since his tag isnt on 1-3 v4:
https://marc.info/?l=linux-block&m=148725565212351&w=2
https://marc.info/?l=linux-block&m=148725603312475&w=2
https://marc.info/?l=linuxppc-embedded&m=148725612212503&w=2

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-21 23:03     ` Scott Bauer
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-21 23:03 UTC (permalink / raw)


On Tue, Feb 21, 2017@11:59:16AM -0700, Jon Derrick wrote:
> By embedding the function data with the function sequence, we can
> eliminate the external function data and state variable code. It also
> made obvious some other small cleanups.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick at intel.com>
Reviewed-by: Scott Bauer <scott.bauer at intel.com>

Also Christoph had reviewed-by on 1-3 on friday, I dont think we need a respin,
but wanted to point that out since his tag isnt on 1-3 v4:
https://marc.info/?l=linux-block&m=148725565212351&w=2
https://marc.info/?l=linux-block&m=148725603312475&w=2
https://marc.info/?l=linuxppc-embedded&m=148725612212503&w=2

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 0/4] OPAL patches, cont'd
  2017-02-21 18:59 ` Jon Derrick
@ 2017-02-22  2:42   ` Jens Axboe
  -1 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22  2:42 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli,
	Christoph Hellwig

On 02/21/2017 11:59 AM, Jon Derrick wrote:
> v3->v4:
> Passes in 'lock_held' into opal_lock_unlock() so we don't need to
> mutex_trylock(). I wasn't totally confident in that approach anyways.
> 
> v2->v3:
> Squashed 5/5 into 4/5
> Changed opal_step structs back to const
> Cleaned up opal_lock_unlock
> 
> v1->v2:
> Moved misplaced code from 5/5 to 4/5

Guys, where are we at with this? I'm shipping off some more fixes
for -rc1 soon, and I'd really like to get this wrapped up sooner
rather than later.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 0/4] OPAL patches, cont'd
@ 2017-02-22  2:42   ` Jens Axboe
  0 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22  2:42 UTC (permalink / raw)


On 02/21/2017 11:59 AM, Jon Derrick wrote:
> v3->v4:
> Passes in 'lock_held' into opal_lock_unlock() so we don't need to
> mutex_trylock(). I wasn't totally confident in that approach anyways.
> 
> v2->v3:
> Squashed 5/5 into 4/5
> Changed opal_step structs back to const
> Cleaned up opal_lock_unlock
> 
> v1->v2:
> Moved misplaced code from 5/5 to 4/5

Guys, where are we at with this? I'm shipping off some more fixes
for -rc1 soon, and I'd really like to get this wrapped up sooner
rather than later.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 0/4] OPAL patches, cont'd
  2017-02-22  2:42   ` Jens Axboe
@ 2017-02-22  3:14     ` Scott Bauer
  -1 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-22  3:14 UTC (permalink / raw)
  To: Jens Axboe, Jon Derrick
  Cc: linux-block, Rafael Antognolli, Scott Bauer, linux-nvme,
	Christoph Hellwig



On 02/21/2017 07:42 PM, Jens Axboe wrote:
> On 02/21/2017 11:59 AM, Jon Derrick wrote:
>> v3->v4:
>> Passes in 'lock_held' into opal_lock_unlock() so we don't need to
>> mutex_trylock(). I wasn't totally confident in that approach anyways.
>>
>> v2->v3:
>> Squashed 5/5 into 4/5
>> Changed opal_step structs back to const
>> Cleaned up opal_lock_unlock
>>
>> v1->v2:
>> Moved misplaced code from 5/5 to 4/5
> 
> Guys, where are we at with this? I'm shipping off some more fixes
> for -rc1 soon, and I'd really like to get this wrapped up sooner
> rather than later.
> 

We just need Christoph to look at patch 4/4. Once he reviews it it the series
is good to go. Once this goes in I have one patch I previously sent and one from 
Christoph I took over. After that the opal code should be quiet for the rest of the
release candidates. We've ironed out all the kinks and clean ups it seems. 

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 0/4] OPAL patches, cont'd
@ 2017-02-22  3:14     ` Scott Bauer
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-22  3:14 UTC (permalink / raw)




On 02/21/2017 07:42 PM, Jens Axboe wrote:
> On 02/21/2017 11:59 AM, Jon Derrick wrote:
>> v3->v4:
>> Passes in 'lock_held' into opal_lock_unlock() so we don't need to
>> mutex_trylock(). I wasn't totally confident in that approach anyways.
>>
>> v2->v3:
>> Squashed 5/5 into 4/5
>> Changed opal_step structs back to const
>> Cleaned up opal_lock_unlock
>>
>> v1->v2:
>> Moved misplaced code from 5/5 to 4/5
> 
> Guys, where are we at with this? I'm shipping off some more fixes
> for -rc1 soon, and I'd really like to get this wrapped up sooner
> rather than later.
> 

We just need Christoph to look at patch 4/4. Once he reviews it it the series
is good to go. Once this goes in I have one patch I previously sent and one from 
Christoph I took over. After that the opal code should be quiet for the rest of the
release candidates. We've ironed out all the kinks and clean ups it seems. 

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-21 18:59   ` Jon Derrick
@ 2017-02-22  7:13     ` Christoph Hellwig
  -1 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2017-02-22  7:13 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli,
	Jens Axboe, Christoph Hellwig

> +	if (!lock_held)
> +		mutex_lock(&dev->dev_lock);

No conditional locking, please.  I guess I causesd this by asking you
to remove __opal_lock_unlock, but it seems we'd either need to keep it
in the end.

Except for that the series looks fine to me.

Jens: given that 1-3 are the important fixes how about you pick those
up ASAP?  They all also had my Reviewed-by for previous postings.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22  7:13     ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2017-02-22  7:13 UTC (permalink / raw)


> +	if (!lock_held)
> +		mutex_lock(&dev->dev_lock);

No conditional locking, please.  I guess I causesd this by asking you
to remove __opal_lock_unlock, but it seems we'd either need to keep it
in the end.

Except for that the series looks fine to me.

Jens: given that 1-3 are the important fixes how about you pick those
up ASAP?  They all also had my Reviewed-by for previous postings.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-22  7:13     ` Christoph Hellwig
@ 2017-02-22 14:35       ` Jon Derrick
  -1 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-22 14:35 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli, Jens Axboe

On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> +	if (!lock_held)
>> +		mutex_lock(&dev->dev_lock);
> 
> No conditional locking, please.  I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
> 
> Except for that the series looks fine to me.
> 
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP?  They all also had my Reviewed-by for previous postings.
> 
Thanks

I'll respin just 4/4 shortly with __opal_lock_unlock

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22 14:35       ` Jon Derrick
  0 siblings, 0 replies; 25+ messages in thread
From: Jon Derrick @ 2017-02-22 14:35 UTC (permalink / raw)


On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> +	if (!lock_held)
>> +		mutex_lock(&dev->dev_lock);
> 
> No conditional locking, please.  I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
> 
> Except for that the series looks fine to me.
> 
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP?  They all also had my Reviewed-by for previous postings.
> 
Thanks

I'll respin just 4/4 shortly with __opal_lock_unlock

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-22  7:13     ` Christoph Hellwig
@ 2017-02-22 16:10       ` Jens Axboe
  -1 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22 16:10 UTC (permalink / raw)
  To: Christoph Hellwig, Jon Derrick
  Cc: linux-block, linux-nvme, Scott Bauer, Rafael Antognolli

On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> +	if (!lock_held)
>> +		mutex_lock(&dev->dev_lock);
> 
> No conditional locking, please.  I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
> 
> Except for that the series looks fine to me.
> 
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP?  They all also had my Reviewed-by for previous postings.

I picked up 1-3, and re-added your reviewed by. #4 should be sorted
before -rc1, though.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22 16:10       ` Jens Axboe
  0 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22 16:10 UTC (permalink / raw)


On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>> +	if (!lock_held)
>> +		mutex_lock(&dev->dev_lock);
> 
> No conditional locking, please.  I guess I causesd this by asking you
> to remove __opal_lock_unlock, but it seems we'd either need to keep it
> in the end.
> 
> Except for that the series looks fine to me.
> 
> Jens: given that 1-3 are the important fixes how about you pick those
> up ASAP?  They all also had my Reviewed-by for previous postings.

I picked up 1-3, and re-added your reviewed by. #4 should be sorted
before -rc1, though.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-22 16:10       ` Jens Axboe
@ 2017-02-22 16:13         ` Scott Bauer
  -1 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-22 16:13 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Christoph Hellwig, Jon Derrick, linux-block, linux-nvme,
	Rafael Antognolli

On Wed, Feb 22, 2017 at 09:10:31AM -0700, Jens Axboe wrote:
> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
> >> +	if (!lock_held)
> >> +		mutex_lock(&dev->dev_lock);
> > 
> > No conditional locking, please.  I guess I causesd this by asking you
> > to remove __opal_lock_unlock, but it seems we'd either need to keep it
> > in the end.
> > 
> > Except for that the series looks fine to me.
> > 
> > Jens: given that 1-3 are the important fixes how about you pick those
> > up ASAP?  They all also had my Reviewed-by for previous postings.
> 
> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
> before -rc1, though.
> 

#4 Is good to go as well. It was resent this morning under
[PATCH] block/sed: Embed function data into the function sequence
And contains the changes Christoph requested, I'll re-add my sign-off.
Once that gets In I can rebase mine and get them out today too.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22 16:13         ` Scott Bauer
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Bauer @ 2017-02-22 16:13 UTC (permalink / raw)


On Wed, Feb 22, 2017@09:10:31AM -0700, Jens Axboe wrote:
> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
> >> +	if (!lock_held)
> >> +		mutex_lock(&dev->dev_lock);
> > 
> > No conditional locking, please.  I guess I causesd this by asking you
> > to remove __opal_lock_unlock, but it seems we'd either need to keep it
> > in the end.
> > 
> > Except for that the series looks fine to me.
> > 
> > Jens: given that 1-3 are the important fixes how about you pick those
> > up ASAP?  They all also had my Reviewed-by for previous postings.
> 
> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
> before -rc1, though.
> 

#4 Is good to go as well. It was resent this morning under
[PATCH] block/sed: Embed function data into the function sequence
And contains the changes Christoph requested, I'll re-add my sign-off.
Once that gets In I can rebase mine and get them out today too.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-22 16:13         ` Scott Bauer
@ 2017-02-22 16:47           ` Jens Axboe
  -1 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22 16:47 UTC (permalink / raw)
  To: Scott Bauer
  Cc: Christoph Hellwig, Jon Derrick, linux-block, linux-nvme,
	Rafael Antognolli

On 02/22/2017 09:13 AM, Scott Bauer wrote:
> On Wed, Feb 22, 2017 at 09:10:31AM -0700, Jens Axboe wrote:
>> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>>>> +	if (!lock_held)
>>>> +		mutex_lock(&dev->dev_lock);
>>>
>>> No conditional locking, please.  I guess I causesd this by asking you
>>> to remove __opal_lock_unlock, but it seems we'd either need to keep it
>>> in the end.
>>>
>>> Except for that the series looks fine to me.
>>>
>>> Jens: given that 1-3 are the important fixes how about you pick those
>>> up ASAP?  They all also had my Reviewed-by for previous postings.
>>
>> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
>> before -rc1, though.
>>
> 
> #4 Is good to go as well. It was resent this morning under
> [PATCH] block/sed: Embed function data into the function sequence
> And contains the changes Christoph requested, I'll re-add my sign-off.
> Once that gets In I can rebase mine and get them out today too.

I see, I found it now. Guys, let's get this process streamlined a bit
more. This whole thing has been a flurry of patches and patchseries,
posted by either you or Jon. Previous patch series was 1-4 patches
posted by you, and then patch #4 is replaced by a single patch from Jon,
posted outside of that thread. Honestly, I feel like this should have
been pushed to 4.12 instead, it clearly wasn't ready before the merge
window.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22 16:47           ` Jens Axboe
  0 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2017-02-22 16:47 UTC (permalink / raw)


On 02/22/2017 09:13 AM, Scott Bauer wrote:
> On Wed, Feb 22, 2017@09:10:31AM -0700, Jens Axboe wrote:
>> On 02/22/2017 12:13 AM, Christoph Hellwig wrote:
>>>> +	if (!lock_held)
>>>> +		mutex_lock(&dev->dev_lock);
>>>
>>> No conditional locking, please.  I guess I causesd this by asking you
>>> to remove __opal_lock_unlock, but it seems we'd either need to keep it
>>> in the end.
>>>
>>> Except for that the series looks fine to me.
>>>
>>> Jens: given that 1-3 are the important fixes how about you pick those
>>> up ASAP?  They all also had my Reviewed-by for previous postings.
>>
>> I picked up 1-3, and re-added your reviewed by. #4 should be sorted
>> before -rc1, though.
>>
> 
> #4 Is good to go as well. It was resent this morning under
> [PATCH] block/sed: Embed function data into the function sequence
> And contains the changes Christoph requested, I'll re-add my sign-off.
> Once that gets In I can rebase mine and get them out today too.

I see, I found it now. Guys, let's get this process streamlined a bit
more. This whole thing has been a flurry of patches and patchseries,
posted by either you or Jon. Previous patch series was 1-4 patches
posted by you, and then patch #4 is replaced by a single patch from Jon,
posted outside of that thread. Honestly, I feel like this should have
been pushed to 4.12 instead, it clearly wasn't ready before the merge
window.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCHv4 4/4] block/sed: Embed function data into the function sequence
  2017-02-22 16:47           ` Jens Axboe
@ 2017-02-22 17:22             ` Keith Busch
  -1 siblings, 0 replies; 25+ messages in thread
From: Keith Busch @ 2017-02-22 17:22 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Scott Bauer, linux-block, Rafael Antognolli, linux-nvme,
	Christoph Hellwig, Jon Derrick

On Wed, Feb 22, 2017 at 09:47:53AM -0700, Jens Axboe wrote:
> I see, I found it now. Guys, let's get this process streamlined a bit
> more. This whole thing has been a flurry of patches and patchseries,
> posted by either you or Jon. Previous patch series was 1-4 patches
> posted by you, and then patch #4 is replaced by a single patch from Jon,
> posted outside of that thread. Honestly, I feel like this should have
> been pushed to 4.12 instead, it clearly wasn't ready before the merge
> window.

Sorry, I'll run interference to help ensure future patch series are
complete and coherent in one thread.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCHv4 4/4] block/sed: Embed function data into the function sequence
@ 2017-02-22 17:22             ` Keith Busch
  0 siblings, 0 replies; 25+ messages in thread
From: Keith Busch @ 2017-02-22 17:22 UTC (permalink / raw)


On Wed, Feb 22, 2017@09:47:53AM -0700, Jens Axboe wrote:
> I see, I found it now. Guys, let's get this process streamlined a bit
> more. This whole thing has been a flurry of patches and patchseries,
> posted by either you or Jon. Previous patch series was 1-4 patches
> posted by you, and then patch #4 is replaced by a single patch from Jon,
> posted outside of that thread. Honestly, I feel like this should have
> been pushed to 4.12 instead, it clearly wasn't ready before the merge
> window.

Sorry, I'll run interference to help ensure future patch series are
complete and coherent in one thread.

^ permalink raw reply	[flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-02-22 17:22 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 18:59 [PATCHv4 0/4] OPAL patches, cont'd Jon Derrick
2017-02-21 18:59 ` Jon Derrick
2017-02-21 18:59 ` [PATCHv4 1/4] block/sed: Use ssize_t on atom parsers to return errors Jon Derrick
2017-02-21 18:59 ` [PATCHv4 2/4] block/sed: Add helper to qualify response tokens Jon Derrick
2017-02-21 18:59 ` [PATCHv4 3/4] block/sed: Check received header lengths Jon Derrick
2017-02-21 18:59 ` [PATCHv4 4/4] block/sed: Embed function data into the function sequence Jon Derrick
2017-02-21 18:59   ` Jon Derrick
2017-02-21 23:03   ` Scott Bauer
2017-02-21 23:03     ` Scott Bauer
2017-02-22  7:13   ` Christoph Hellwig
2017-02-22  7:13     ` Christoph Hellwig
2017-02-22 14:35     ` Jon Derrick
2017-02-22 14:35       ` Jon Derrick
2017-02-22 16:10     ` Jens Axboe
2017-02-22 16:10       ` Jens Axboe
2017-02-22 16:13       ` Scott Bauer
2017-02-22 16:13         ` Scott Bauer
2017-02-22 16:47         ` Jens Axboe
2017-02-22 16:47           ` Jens Axboe
2017-02-22 17:22           ` Keith Busch
2017-02-22 17:22             ` Keith Busch
2017-02-22  2:42 ` [PATCHv4 0/4] OPAL patches, cont'd Jens Axboe
2017-02-22  2:42   ` Jens Axboe
2017-02-22  3:14   ` Scott Bauer
2017-02-22  3:14     ` Scott Bauer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.