[PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables

linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables
@ 2019-08-21 19:10 Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 1/3] block: sed-opal: Expose enum opal_uid and opaluid definitions to the users by moving it to "include/uapi/linux/sed-opal.h" Revanth Rajashekar
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Revanth Rajashekar @ 2019-08-21 19:10 UTC (permalink / raw)
  To: linux-block; +Cc: Jonathan Derrick, Scott Bauer

This series of patches aims at extending SED Opal support:
1. Exposing enum opal_uid and opaluid definitions to the users to select
   the desired opal table UID.
2. Generalizing write data to any opal table
3. Adding an IOCTL for reading/writing any Opal Table with Admin-1 authority

Datastore feature described in:
https://trustedcomputinggroup.org/wp-content/uploads/TCG_Storage-Opal_Feature_Set-Additional_DataStore_Tables_v1_00_r1_00_Final.pdf

Opal Application Note:
https://trustedcomputinggroup.org/wp-content/uploads/TCG_Storage_Opal_SSC_Application_Note_1-00_1-00-Final.pdf

This feature has been successfully tested on OPAL Datastore and MBR table using
internal tools with a Intel SSD and Intel Optane.


Revanth Rajashekar (3):
  block: sed-opal: Expose enum opal_uid and opaluid definitions to the
    users by moving it to "include/uapi/linux/sed-opal.h"
  block: sed-opal: Generalizing write data to any opal table
  block: sed-opal: Add support to read/write opal tables generically

 block/opal_proto.h            |  39 ----
 block/sed-opal.c              | 349 +++++++++++++++++++++-------------
 include/linux/sed-opal.h      |   1 +
 include/uapi/linux/sed-opal.h | 126 ++++++++++++
 4 files changed, 341 insertions(+), 174 deletions(-)

--
2.17.1


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

* [PATCH 1/3] block: sed-opal: Expose enum opal_uid and opaluid definitions to the users by moving it to "include/uapi/linux/sed-opal.h"
  2019-08-21 19:10 [PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables Revanth Rajashekar
@ 2019-08-21 19:10 ` Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 2/3] block: sed-opal: Generalizing write data to any opal table Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically Revanth Rajashekar
  2 siblings, 0 replies; 7+ messages in thread
From: Revanth Rajashekar @ 2019-08-21 19:10 UTC (permalink / raw)
  To: linux-block; +Cc: Jonathan Derrick, Scott Bauer, Revanth Rajashekar

The intent of this patch is to let the user select the desired
table UID.

Signed-off-by: Revanth Rajashekar <revanth.rajashekar@intel.com>
---
 block/opal_proto.h            |  39 ------------
 block/sed-opal.c              |  71 ----------------------
 include/uapi/linux/sed-opal.h | 110 ++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+), 110 deletions(-)

diff --git a/block/opal_proto.h b/block/opal_proto.h
index 5532412d567c..d6697fd4d178 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -76,49 +76,10 @@ enum opal_response_token {
  * Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
  * Section: 6.3 Assigned UIDs
  */
-#define OPAL_UID_LENGTH 8
 #define OPAL_METHOD_LENGTH 8
 #define OPAL_MSID_KEYLEN 15
 #define 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_TABLE_TABLE,
-	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,
-};
-
 /* Enum for indexing the OPALMETHOD array */
 enum opal_method {
 	OPAL_PROPERTIES,
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 4e95a9792162..f04b83eae407 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -98,77 +98,6 @@ struct opal_dev {
 	struct list_head unlk_lst;
 };

-
-static const u8 opaluid[][OPAL_UID_LENGTH] = {
-	/* users */
-	[OPAL_SMUID_UID] =
-		{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff },
-	[OPAL_THISSP_UID] =
-		{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_ADMINSP_UID] =
-		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_LOCKINGSP_UID] =
-		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x02 },
-	[OPAL_ENTERPRISE_LOCKINGSP_UID] =
-		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x01, 0x00, 0x01 },
-	[OPAL_ANYBODY_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_SID_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x06 },
-	[OPAL_ADMIN1_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0x00, 0x01 },
-	[OPAL_USER1_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x01 },
-	[OPAL_USER2_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x02 },
-	[OPAL_PSID_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0xff, 0x01 },
-	[OPAL_ENTERPRISE_BANDMASTER0_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x80, 0x01 },
-	[OPAL_ENTERPRISE_ERASEMASTER_UID] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x84, 0x01 },
-
-	/* tables */
-	[OPAL_TABLE_TABLE]
-		{ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_LOCKINGRANGE_GLOBAL] =
-		{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
-		{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE0, 0x01 },
-	[OPAL_LOCKINGRANGE_ACE_WRLOCKED] =
-		{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE8, 0x01 },
-	[OPAL_MBRCONTROL] =
-		{ 0x00, 0x00, 0x08, 0x03, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_MBR] =
-		{ 0x00, 0x00, 0x08, 0x04, 0x00, 0x00, 0x00, 0x00 },
-	[OPAL_AUTHORITY_TABLE] =
-		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00},
-	[OPAL_C_PIN_TABLE] =
-		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x00},
-	[OPAL_LOCKING_INFO_TABLE] =
-		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x01 },
-	[OPAL_ENTERPRISE_LOCKING_INFO_TABLE] =
-		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x00 },
-
-	/* C_PIN_TABLE object ID's */
-	[OPAL_C_PIN_MSID] =
-		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x84, 0x02},
-	[OPAL_C_PIN_SID] =
-		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x01},
-	[OPAL_C_PIN_ADMIN1] =
-		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x01, 0x00, 0x01},
-
-	/* half UID's (only first 4 bytes used) */
-	[OPAL_HALF_UID_AUTHORITY_OBJ_REF] =
-		{ 0x00, 0x00, 0x0C, 0x05, 0xff, 0xff, 0xff, 0xff },
-	[OPAL_HALF_UID_BOOLEAN_ACE] =
-		{ 0x00, 0x00, 0x04, 0x0E, 0xff, 0xff, 0xff, 0xff },
-
-	/* special value for omitted optional parameter */
-	[OPAL_UID_HEXFF] =
-		{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
-};
-
 /*
  * TCG Storage SSC Methods.
  * Derived from: TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index c6d035fa1b6c..59eed0bdffd3 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -38,6 +38,116 @@ enum opal_user {
 	OPAL_USER9 = 0x09,
 };

+/* 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_TABLE_TABLE,
+	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,
+};
+
+#define OPAL_UID_LENGTH 8
+
+static const __u8 opaluid[][OPAL_UID_LENGTH] = {
+	/* users */
+	[OPAL_SMUID_UID] =
+		{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff },
+	[OPAL_THISSP_UID] =
+		{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_ADMINSP_UID] =
+		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_LOCKINGSP_UID] =
+		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, 0x02 },
+	[OPAL_ENTERPRISE_LOCKINGSP_UID] =
+		{ 0x00, 0x00, 0x02, 0x05, 0x00, 0x01, 0x00, 0x01 },
+	[OPAL_ANYBODY_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_SID_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x06 },
+	[OPAL_ADMIN1_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0x00, 0x01 },
+	[OPAL_USER1_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x01 },
+	[OPAL_USER2_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x03, 0x00, 0x02 },
+	[OPAL_PSID_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0xff, 0x01 },
+	[OPAL_ENTERPRISE_BANDMASTER0_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x80, 0x01 },
+	[OPAL_ENTERPRISE_ERASEMASTER_UID] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x84, 0x01 },
+
+	/* tables */
+	[OPAL_TABLE_TABLE]
+		{ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_LOCKINGRANGE_GLOBAL] =
+		{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
+		{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE0, 0x01 },
+	[OPAL_LOCKINGRANGE_ACE_WRLOCKED] =
+		{ 0x00, 0x00, 0x00, 0x08, 0x00, 0x03, 0xE8, 0x01 },
+	[OPAL_MBRCONTROL] =
+		{ 0x00, 0x00, 0x08, 0x03, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_MBR] =
+		{ 0x00, 0x00, 0x08, 0x04, 0x00, 0x00, 0x00, 0x00 },
+	[OPAL_AUTHORITY_TABLE] =
+		{ 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00 },
+	[OPAL_C_PIN_TABLE] =
+		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x00 },
+	[OPAL_LOCKING_INFO_TABLE] =
+		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_ENTERPRISE_LOCKING_INFO_TABLE] =
+		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x00 },
+
+	/* C_PIN_TABLE object ID's */
+	[OPAL_C_PIN_MSID] =
+		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x84, 0x02 },
+	[OPAL_C_PIN_SID] =
+		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x00, 0x00, 0x01 },
+	[OPAL_C_PIN_ADMIN1] =
+		{ 0x00, 0x00, 0x00, 0x0B, 0x00, 0x01, 0x00, 0x01 },
+
+	/* half UID's (only first 4 bytes used) */
+	[OPAL_HALF_UID_AUTHORITY_OBJ_REF] =
+		{ 0x00, 0x00, 0x0C, 0x05, 0xff, 0xff, 0xff, 0xff },
+	[OPAL_HALF_UID_BOOLEAN_ACE] =
+		{ 0x00, 0x00, 0x04, 0x0E, 0xff, 0xff, 0xff, 0xff },
+
+	/* special value for omitted optional parameter */
+	[OPAL_UID_HEXFF] =
+		{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
+};
+
 enum opal_lock_state {
 	OPAL_RO = 0x01, /* 0001 */
 	OPAL_RW = 0x02, /* 0010 */
--
2.17.1


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

* [PATCH 2/3] block: sed-opal: Generalizing write data to any opal table
  2019-08-21 19:10 [PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 1/3] block: sed-opal: Expose enum opal_uid and opaluid definitions to the users by moving it to "include/uapi/linux/sed-opal.h" Revanth Rajashekar
@ 2019-08-21 19:10 ` Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically Revanth Rajashekar
  2 siblings, 0 replies; 7+ messages in thread
From: Revanth Rajashekar @ 2019-08-21 19:10 UTC (permalink / raw)
  To: linux-block; +Cc: Jonathan Derrick, Scott Bauer, Revanth Rajashekar

This patch refactors the existing "write_shadowmbr" func and
creates a new generalized function "generic_table_write_data",
to write data to any opal table

Signed-off-by: Revanth Rajashekar <revanth.rajashekar@intel.com>
---
 block/sed-opal.c | 138 +++++++++++++++++++++++++----------------------
 1 file changed, 74 insertions(+), 64 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index f04b83eae407..7179582730b6 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1068,11 +1068,11 @@ static int generic_get_column(struct opal_dev *dev, const u8 *table,
  *
  * the result is provided in dev->resp->tok[4]
  */
-static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
+static int generic_get_table_info(struct opal_dev *dev, const u8 *table_uid,
 				  u64 column)
 {
 	u8 uid[OPAL_UID_LENGTH];
-	const unsigned int half = OPAL_UID_LENGTH/2;
+	const unsigned int half = OPAL_UID_LENGTH_HALF;

 	/* sed-opal UIDs can be split in two halves:
 	 *  first:  actual table index
@@ -1081,7 +1081,7 @@ static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
 	 * first part of the target table as relative index into that table
 	 */
 	memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
-	memcpy(uid+half, opaluid[table], half);
+	memcpy(uid + half, table_uid, half);

 	return generic_get_column(dev, uid, column);
 }
@@ -1150,6 +1150,75 @@ static int get_active_key(struct opal_dev *dev, void *data)
 	return get_active_key_cont(dev);
 }

+static int generic_table_write_data(struct opal_dev *dev, const u64 data, u64 offset,
+				    u64 size, const u8 *uid)
+{
+	const u8 __user *src;
+	u8 *dst;
+	u64 len;
+	size_t off = 0;
+	int err = 0;
+
+	/* do we fit in the available space? */
+	err = generic_get_table_info(dev, uid, OPAL_TABLE_ROWS);
+	if (err) {
+		pr_debug("Couldn't get the table size\n");
+		return err;
+	}
+
+	len = response_get_u64(&dev->parsed, 4);
+	if (size > len || offset > len - size) {
+		pr_debug("Does not fit in the table (%llu vs. %llu)\n",
+			  offset + size, len);
+		return -ENOSPC;
+	}
+
+	/* do the actual transmission(s) */
+	src = (u8 __user *)(uintptr_t)data;
+	while (off < size) {
+		err = cmd_start(dev, uid, opalmethod[OPAL_SET]);
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_WHERE);
+		add_token_u64(&err, dev, offset + off);
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_VALUES);
+
+		/*
+		 * The bytestring header is either 1 or 2 bytes, so assume 2.
+		 * There also needs to be enough space to accommodate the
+		 * trailing OPAL_ENDNAME (1 byte) and tokens added by
+		 * cmd_finalize.
+		 */
+		len = min(remaining_size(dev) - (2+1+CMD_FINALIZE_BYTES_NEEDED),
+			  (size_t)(size - off));
+		pr_debug("Write bytes %zu+%llu/%llu\n",
+			  off, len, size);
+
+		dst = add_bytestring_header(&err, dev, len);
+		if (!dst)
+			break;
+
+		if (copy_from_user(dst, src + off, len))
+			err = -EFAULT;
+
+		dev->pos += len;
+
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+		if (err)
+			break;
+
+		err = finalize_and_send(dev, parse_and_check_status);
+		if (err)
+			break;
+
+		off += len;
+	}
+
+	return err;
+}
+
 static int generic_lr_enable_disable(struct opal_dev *dev,
 				     u8 *uid, bool rle, bool wle,
 				     bool rl, bool wl)
@@ -1512,68 +1581,9 @@ static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
 static int write_shadow_mbr(struct opal_dev *dev, void *data)
 {
 	struct opal_shadow_mbr *shadow = data;
-	const u8 __user *src;
-	u8 *dst;
-	size_t off = 0;
-	u64 len;
-	int err = 0;
-
-	/* do we fit in the available shadow mbr space? */
-	err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
-	if (err) {
-		pr_debug("MBR: could not get shadow size\n");
-		return err;
-	}
-
-	len = response_get_u64(&dev->parsed, 4);
-	if (shadow->size > len || shadow->offset > len - shadow->size) {
-		pr_debug("MBR: does not fit in shadow (%llu vs. %llu)\n",
-			 shadow->offset + shadow->size, len);
-		return -ENOSPC;
-	}

-	/* do the actual transmission(s) */
-	src = (u8 __user *)(uintptr_t)shadow->data;
-	while (off < shadow->size) {
-		err = cmd_start(dev, opaluid[OPAL_MBR], opalmethod[OPAL_SET]);
-		add_token_u8(&err, dev, OPAL_STARTNAME);
-		add_token_u8(&err, dev, OPAL_WHERE);
-		add_token_u64(&err, dev, shadow->offset + off);
-		add_token_u8(&err, dev, OPAL_ENDNAME);
-
-		add_token_u8(&err, dev, OPAL_STARTNAME);
-		add_token_u8(&err, dev, OPAL_VALUES);
-
-		/*
-		 * The bytestring header is either 1 or 2 bytes, so assume 2.
-		 * There also needs to be enough space to accommodate the
-		 * trailing OPAL_ENDNAME (1 byte) and tokens added by
-		 * cmd_finalize.
-		 */
-		len = min(remaining_size(dev) - (2+1+CMD_FINALIZE_BYTES_NEEDED),
-			  (size_t)(shadow->size - off));
-		pr_debug("MBR: write bytes %zu+%llu/%llu\n",
-			 off, len, shadow->size);
-
-		dst = add_bytestring_header(&err, dev, len);
-		if (!dst)
-			break;
-		if (copy_from_user(dst, src + off, len))
-			err = -EFAULT;
-		dev->pos += len;
-
-		add_token_u8(&err, dev, OPAL_ENDNAME);
-		if (err)
-			break;
-
-		err = finalize_and_send(dev, parse_and_check_status);
-		if (err)
-			break;
-
-		off += len;
-	}
-
-	return err;
+	return generic_table_write_data(dev, shadow->data, shadow->offset,
+					shadow->size, opaluid[OPAL_MBR]);
 }

 static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
--
2.17.1


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

* [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically
  2019-08-21 19:10 [PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 1/3] block: sed-opal: Expose enum opal_uid and opaluid definitions to the users by moving it to "include/uapi/linux/sed-opal.h" Revanth Rajashekar
  2019-08-21 19:10 ` [PATCH 2/3] block: sed-opal: Generalizing write data to any opal table Revanth Rajashekar
@ 2019-08-21 19:10 ` Revanth Rajashekar
  2019-08-23 16:58   ` Derrick, Jonathan
  2019-08-25 20:08   ` Scott Bauer
  2 siblings, 2 replies; 7+ messages in thread
From: Revanth Rajashekar @ 2019-08-21 19:10 UTC (permalink / raw)
  To: linux-block; +Cc: Jonathan Derrick, Scott Bauer, Revanth Rajashekar

This feature gives the user RW access to any opal table with admin1
authority.

The flags described in the new structure determines if the user
wants to read/write the data. Flags are checked for valid values in
order to allow future features to be added to the ioctl.

Previously exposed opal UIDs allows the user to easily select the
desired table to retrieve its UID.

The ioctl provides a size and offset field and internally will loop
data accesses to return the full data block.

The ioctl provides a private field with the intentiont to accommodate
any future expansions to the ioctl.

Signed-off-by: Revanth Rajashekar <revanth.rajashekar@intel.com>
---
 block/sed-opal.c              | 140 ++++++++++++++++++++++++++++++++++
 include/linux/sed-opal.h      |   1 +
 include/uapi/linux/sed-opal.h |  16 ++++
 3 files changed, 157 insertions(+)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 7179582730b6..3f41fc56f3cb 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1896,6 +1896,108 @@ static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
 	return 0;
 }

+static int write_table_data(struct opal_dev *dev, void *data)
+{
+	struct opal_read_write_table *write_tbl = data;
+
+	return generic_table_write_data(dev, write_tbl->data, write_tbl->offset,
+					write_tbl->size, write_tbl->table_uid);
+}
+
+static int read_table_data_cont(struct opal_dev *dev)
+{
+	int err = 0;
+	const char *data_read;
+
+	err = parse_and_check_status(dev);
+	if (err)
+		return err;
+
+	dev->prev_d_len = response_get_string(&dev->parsed, 1, &data_read);
+
+	dev->prev_data = data_read;
+	if (!dev->prev_data) {
+		pr_debug("%s: Couldn't read data from the table.\n", __func__);
+		return OPAL_INVAL_PARAM;
+	}
+
+	return 0;
+}
+
+/*
+ * IO_BUFFER_LENGTH = 2048
+ * sizeof(header) = 56
+ * No. of Token Bytes in the Response = 11
+ * MAX size of data that can be carried in response buffer
+ * at a time is : 2048 - (56 + 11) = 1981 = 0x7BD.
+ */
+#define OPAL_MAX_READ_TABLE (0x7BD)
+
+static int read_table_data(struct opal_dev *dev, void *data)
+{
+	struct opal_read_write_table *read_tbl = data;
+	int err = 0;
+	size_t off = 0, max_read_size = OPAL_MAX_READ_TABLE;
+	u64 table_len = 0, len = 0;
+	u64 offset = read_tbl->offset, read_size = read_tbl->size - 1;
+	u8 __user *dst;
+
+	err = generic_get_table_info(dev, read_tbl->table_uid, OPAL_TABLE_ROWS);
+	if (err) {
+		pr_debug("Couldn't get the table size\n");
+		return err;
+	}
+
+	table_len = response_get_u64(&dev->parsed, 4);
+
+	/* Check if the user is trying to read from the table limits */
+	if (read_size > table_len || offset > table_len - read_size) {
+		pr_debug("Read size exceeds the Table size limits (%llu vs. %llu)\n",
+			  offset + read_size, table_len);
+		return -EINVAL;
+	}
+
+	while (off < read_size) {
+		err = cmd_start(dev, read_tbl->table_uid, opalmethod[OPAL_GET]);
+
+		add_token_u8(&err, dev, OPAL_STARTLIST);
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_STARTROW);
+		add_token_u64(&err, dev, offset + off); /* start row value */
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_ENDROW);
+
+		len = min(max_read_size, (size_t)(read_size - off));
+		add_token_u64(&err, dev, offset + off + len); /* end row value */
+
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+		add_token_u8(&err, dev, OPAL_ENDLIST);
+
+		if (err) {
+			pr_debug("Error building read table data command.\n");
+			break;
+		}
+
+		err = finalize_and_send(dev, read_table_data_cont);
+		if (err)
+			break;
+
+		dst = (u8 __user *)(uintptr_t)read_tbl->data;
+		if (copy_to_user(dst + off, dev->prev_data, dev->prev_d_len)) {
+			pr_debug("Error copying data to userspace\n");
+			err = -EFAULT;
+			break;
+		}
+		dev->prev_data = NULL;
+
+		off += len;
+	}
+
+	return err;
+}
+
 static int end_opal_session(struct opal_dev *dev, void *data)
 {
 	int err = 0;
@@ -2382,6 +2484,41 @@ bool opal_unlock_from_suspend(struct opal_dev *dev)
 }
 EXPORT_SYMBOL(opal_unlock_from_suspend);

+static int opal_generic_read_write_table(struct opal_dev *dev,
+                                         struct opal_read_write_table *rw_tbl)
+{
+	const struct opal_step write_table_steps[] = {
+		{ start_admin1LSP_opal_session, &rw_tbl->key },
+		{ write_table_data, rw_tbl },
+		{ end_opal_session, }
+	};
+
+	const struct opal_step read_table_steps[] = {
+		{ start_admin1LSP_opal_session, &rw_tbl->key },
+		{ read_table_data, rw_tbl },
+		{ end_opal_session, }
+	};
+	int ret = 0;
+
+	mutex_lock(&dev->dev_lock);
+	setup_opal_dev(dev);
+	if (rw_tbl->flags & OPAL_TABLE_READ) {
+		if (rw_tbl->size > 0)
+			ret = execute_steps(dev, read_table_steps,
+					    ARRAY_SIZE(read_table_steps));
+	} else if (rw_tbl->flags & OPAL_TABLE_WRITE) {
+		if (rw_tbl->size > 0)
+			ret = execute_steps(dev, write_table_steps,
+					    ARRAY_SIZE(write_table_steps));
+	} else {
+		pr_debug("Invalid bit set in the flag.\n");
+		ret = -EINVAL;
+	}
+	mutex_unlock(&dev->dev_lock);
+
+	return ret;
+}
+
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 {
 	void *p;
@@ -2444,6 +2581,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 	case IOC_OPAL_PSID_REVERT_TPR:
 		ret = opal_reverttper(dev, p, true);
 		break;
+	case IOC_OPAL_GENERIC_TABLE_RW:
+		ret = opal_generic_read_write_table(dev, p);
+		break;
 	default:
 		break;
 	}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 53c28d750a45..1ac0d712a9c3 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -42,6 +42,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	case IOC_OPAL_PSID_REVERT_TPR:
 	case IOC_OPAL_MBR_DONE:
 	case IOC_OPAL_WRITE_SHADOW_MBR:
+	case IOC_OPAL_GENERIC_TABLE_RW:
 		return true;
 	}
 	return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 59eed0bdffd3..a803ed0534da 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -65,6 +65,7 @@ enum opal_uid {
 	OPAL_C_PIN_TABLE,
 	OPAL_LOCKING_INFO_TABLE,
 	OPAL_ENTERPRISE_LOCKING_INFO_TABLE,
+	OPAL_DATASTORE,
 	/* C_PIN_TABLE object ID's */
 	OPAL_C_PIN_MSID,
 	OPAL_C_PIN_SID,
@@ -128,6 +129,8 @@ static const __u8 opaluid[][OPAL_UID_LENGTH] = {
 		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x01 },
 	[OPAL_ENTERPRISE_LOCKING_INFO_TABLE] =
 		{ 0x00, 0x00, 0x08, 0x01, 0x00, 0x00, 0x00, 0x00 },
+	[OPAL_DATASTORE] =
+		{ 0x00, 0x00, 0x10, 0x01, 0x00, 0x00, 0x00, 0x00 },

 	/* C_PIN_TABLE object ID's */
 	[OPAL_C_PIN_MSID] =
@@ -223,6 +226,18 @@ struct opal_shadow_mbr {
 	__u64 size;
 };

+struct opal_read_write_table {
+	struct opal_key key;
+	const __u64 data;
+	const __u8 table_uid[OPAL_UID_LENGTH];
+	__u64 offset;
+	__u64 size;
+	#define OPAL_TABLE_READ (1 << 0)
+	#define OPAL_TABLE_WRITE (1 << 1)
+	__u64 flags;
+	__u64 priv;
+};
+
 #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
@@ -238,5 +253,6 @@ struct opal_shadow_mbr {
 #define IOC_OPAL_PSID_REVERT_TPR    _IOW('p', 232, struct opal_key)
 #define IOC_OPAL_MBR_DONE           _IOW('p', 233, struct opal_mbr_done)
 #define IOC_OPAL_WRITE_SHADOW_MBR   _IOW('p', 234, struct opal_shadow_mbr)
+#define IOC_OPAL_GENERIC_TABLE_RW   _IOW('p', 235, struct opal_read_write_table)

 #endif /* _UAPI_SED_OPAL_H */
--
2.17.1


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

* Re: [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically
  2019-08-21 19:10 ` [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically Revanth Rajashekar
@ 2019-08-23 16:58   ` Derrick, Jonathan
  2019-08-25 20:08   ` Scott Bauer
  1 sibling, 0 replies; 7+ messages in thread
From: Derrick, Jonathan @ 2019-08-23 16:58 UTC (permalink / raw)
  To: Rajashekar, Revanth, linux-block; +Cc: zub, sbauer

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

+David

On Wed, 2019-08-21 at 13:10 -0600, Revanth Rajashekar wrote:
> This feature gives the user RW access to any opal table with admin1
> authority.
> 
> The flags described in the new structure determines if the user
> wants to read/write the data. Flags are checked for valid values in
> order to allow future features to be added to the ioctl.
> 
> Previously exposed opal UIDs allows the user to easily select the
> desired table to retrieve its UID.
> 
> The ioctl provides a size and offset field and internally will loop
> data accesses to return the full data block.
> 
> The ioctl provides a private field with the intentiont to accommodate
> any future expansions to the ioctl.
> 
> Signed-off-by: Revanth Rajashekar <revanth.rajashekar@intel.com>
> ---
>  block/sed-opal.c              | 140 ++++++++++++++++++++++++++++++++++
>  include/linux/sed-opal.h      |   1 +
>  include/uapi/linux/sed-opal.h |  16 ++++
>  3 files changed, 157 insertions(+)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 7179582730b6..3f41fc56f3cb 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -1896,6 +1896,108 @@ static int get_msid_cpin_pin(struct opal_dev *dev, void *data)
>  	return 0;
>  }
> 
> +static int write_table_data(struct opal_dev *dev, void *data)
> +{
> +	struct opal_read_write_table *write_tbl = data;
> +
> +	return generic_table_write_data(dev, write_tbl->data, write_tbl->offset,
> +					write_tbl->size, write_tbl->table_uid);
> +}
> +
> +static int read_table_data_cont(struct opal_dev *dev)
> +{
> +	int err = 0;
> +	const char *data_read;
> +
> +	err = parse_and_check_status(dev);
> +	if (err)
> +		return err;
> +
> +	dev->prev_d_len = response_get_string(&dev->parsed, 1, &data_read);
> +
> +	dev->prev_data = data_read;
> +	if (!dev->prev_data) {
> +		pr_debug("%s: Couldn't read data from the table.\n", __func__);
> +		return OPAL_INVAL_PARAM;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * IO_BUFFER_LENGTH = 2048
> + * sizeof(header) = 56
> + * No. of Token Bytes in the Response = 11
> + * MAX size of data that can be carried in response buffer
> + * at a time is : 2048 - (56 + 11) = 1981 = 0x7BD.
> + */
> +#define OPAL_MAX_READ_TABLE (0x7BD)
This is the only part I'm concerned about, but I'm not aware of any
condition in the spec allowing for the response to have extra fields
that would overflow the buffer.



[snip]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

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

* Re: [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically
  2019-08-21 19:10 ` [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically Revanth Rajashekar
  2019-08-23 16:58   ` Derrick, Jonathan
@ 2019-08-25 20:08   ` Scott Bauer
  2019-08-26 19:27     ` Derrick, Jonathan
  1 sibling, 1 reply; 7+ messages in thread
From: Scott Bauer @ 2019-08-25 20:08 UTC (permalink / raw)
  To: Revanth Rajashekar; +Cc: linux-block, Jonathan Derrick, zub

On Wed, Aug 21, 2019 at 01:10:51PM -0600, Revanth Rajashekar wrote:

[snip]

> The ioctl provides a private field with the intentiont to accommodate
> any future expansions to the ioctl.

spelling (intentiont) 

[snip]

> + * IO_BUFFER_LENGTH = 2048
> + * sizeof(header) = 56
> + * No. of Token Bytes in the Response = 11
> + * MAX size of data that can be carried in response buffer
> + * at a time is : 2048 - (56 + 11) = 1981 = 0x7BD.
> + */
> +#define OPAL_MAX_READ_TABLE (0x7BD)
> +
> +static int read_table_data(struct opal_dev *dev, void *data)
> +{
> +		dst = (u8 __user *)(uintptr_t)read_tbl->data;
> +		if (copy_to_user(dst + off, dev->prev_data, dev->prev_d_len)) {
> +			pr_debug("Error copying data to userspace\n");
> +			err = -EFAULT;
> +			break;
> +		}

I'm with Jon on this one. Even though the spec says we have a max size, lets not put our trust in firmware engineers.
A simple if check is easy to place before the CTU and will solve any future wtf debugging on a userland program.





> +static int opal_generic_read_write_table(struct opal_dev *dev,
> +                                         struct opal_read_write_table *rw_tbl)
> +{
> +	const struct opal_step write_table_steps[] = {
> +		{ start_admin1LSP_opal_session, &rw_tbl->key },
> +		{ write_table_data, rw_tbl },
> +		{ end_opal_session, }
> +	};
> +
> +	const struct opal_step read_table_steps[] = {
> +		{ start_admin1LSP_opal_session, &rw_tbl->key },
> +		{ read_table_data, rw_tbl },
> +		{ end_opal_session, }
> +	};
> +	int ret = 0;
> +
> +	mutex_lock(&dev->dev_lock);
> +	setup_opal_dev(dev);
> +	if (rw_tbl->flags & OPAL_TABLE_READ) {
> +		if (rw_tbl->size > 0)
> +			ret = execute_steps(dev, read_table_steps,
> +					    ARRAY_SIZE(read_table_steps));
> +	} else if (rw_tbl->flags & OPAL_TABLE_WRITE) {
> +		if (rw_tbl->size > 0)
> +			ret = execute_steps(dev, write_table_steps,
> +					    ARRAY_SIZE(write_table_steps));
> +	} else {
> +		pr_debug("Invalid bit set in the flag.\n");
> +		ret = -EINVAL;
> +	}
> +	mutex_unlock(&dev->dev_lock);
> +
> +	return ret;
> +}

Do we expect to add more flags in the future? I ask because this function can quickly get out
of hand with regard to the else if chain and the function table list above. If we think we're going
to add more flags in the future lets slap a switch statement in here to call opal_table_write() and
opal_table_read(). We can deal with that in the future I guess, I just don't want a 3000 line function.




> diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
> index 59eed0bdffd3..a803ed0534da 100644
> --- a/include/uapi/linux/sed-opal.h
> +++ b/include/uapi/linux/sed-opal.h
> +struct opal_read_write_table {
> +	struct opal_key key;
> +	const __u64 data;
> +	const __u8 table_uid[OPAL_UID_LENGTH];
> +	__u64 offset;
> +	__u64 size;
> +	#define OPAL_TABLE_READ (1 << 0)
> +	#define OPAL_TABLE_WRITE (1 << 1)
> +	__u64 flags;
> +	__u64 priv;
> +};

Two things, can you double check the pahole on this struct (Google it or ask Jon he knows).
Second, can you lift those defines into Enumerations or out of the struct? Is there a reason
they're in there?


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

* Re: [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically
  2019-08-25 20:08   ` Scott Bauer
@ 2019-08-26 19:27     ` Derrick, Jonathan
  0 siblings, 0 replies; 7+ messages in thread
From: Derrick, Jonathan @ 2019-08-26 19:27 UTC (permalink / raw)
  To: Rajashekar, Revanth, sbauer; +Cc: zub, linux-block

On Sun, 2019-08-25 at 16:08 -0400, Scott Bauer wrote:
> On Wed, Aug 21, 2019 at 01:10:51PM -0600, Revanth Rajashekar wrote:
> 
> [snip]
> 
> > The ioctl provides a private field with the intentiont to accommodate
> > any future expansions to the ioctl.
> 
> spelling (intentiont) 
> 
> [snip]
> 
> > + * IO_BUFFER_LENGTH = 2048
> > + * sizeof(header) = 56
> > + * No. of Token Bytes in the Response = 11
> > + * MAX size of data that can be carried in response buffer
> > + * at a time is : 2048 - (56 + 11) = 1981 = 0x7BD.
> > + */
> > +#define OPAL_MAX_READ_TABLE (0x7BD)
> > +
> > +static int read_table_data(struct opal_dev *dev, void *data)
> > +{
> > +		dst = (u8 __user *)(uintptr_t)read_tbl->data;
> > +		if (copy_to_user(dst + off, dev->prev_data, dev->prev_d_len)) {
> > +			pr_debug("Error copying data to userspace\n");
> > +			err = -EFAULT;
> > +			break;
> > +		}
> 
> I'm with Jon on this one. Even though the spec says we have a max size, lets not put our trust in firmware engineers.
> A simple if check is easy to place before the CTU and will solve any future wtf debugging on a userland program.
> 
> 
I think we could do that as well as specify the
MaxResponseComPacketSize=IO_BUFFER_LENGTH in the command
https://trustedcomputinggroup.org/wp-content/uploads/TCG_Storage_Opal_SSC_Application_Note_1-00_1-00-Final.pdf
3.2.1.2.1 Host to TPer Properties invocation

> 
> 
> 
> > +static int opal_generic_read_write_table(struct opal_dev *dev,
> > +                                         struct opal_read_write_table *rw_tbl)
> > +{
> > +	const struct opal_step write_table_steps[] = {
> > +		{ start_admin1LSP_opal_session, &rw_tbl->key },
> > +		{ write_table_data, rw_tbl },
> > +		{ end_opal_session, }
> > +	};
> > +
> > +	const struct opal_step read_table_steps[] = {
> > +		{ start_admin1LSP_opal_session, &rw_tbl->key },
> > +		{ read_table_data, rw_tbl },
> > +		{ end_opal_session, }
> > +	};
> > +	int ret = 0;
> > +
> > +	mutex_lock(&dev->dev_lock);
> > +	setup_opal_dev(dev);
> > +	if (rw_tbl->flags & OPAL_TABLE_READ) {
> > +		if (rw_tbl->size > 0)
> > +			ret = execute_steps(dev, read_table_steps,
> > +					    ARRAY_SIZE(read_table_steps));
> > +	} else if (rw_tbl->flags & OPAL_TABLE_WRITE) {
> > +		if (rw_tbl->size > 0)
> > +			ret = execute_steps(dev, write_table_steps,
> > +					    ARRAY_SIZE(write_table_steps));
> > +	} else {
> > +		pr_debug("Invalid bit set in the flag.\n");
> > +		ret = -EINVAL;
> > +	}
> > +	mutex_unlock(&dev->dev_lock);
> > +
> > +	return ret;
> > +}
> 
> Do we expect to add more flags in the future? I ask because this function can quickly get out
> of hand with regard to the else if chain and the function table list above. If we think we're going
> to add more flags in the future lets slap a switch statement in here to call opal_table_write() and
> opal_table_read(). We can deal with that in the future I guess, I just don't want a 3000 line function.
> 
> 
I had imagined potentially chaining ACS settings in the read/write

You could add a flag that says 'private' is another or multiple table
read/writes, and the private points to a descriptor equal to the ioctl
struct.

I'm ok with changing if/else to switch. Whichever looks better.


> 
> 
> > diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
> > index 59eed0bdffd3..a803ed0534da 100644
> > --- a/include/uapi/linux/sed-opal.h
> > +++ b/include/uapi/linux/sed-opal.h
> > +struct opal_read_write_table {
> > +	struct opal_key key;
> > +	const __u64 data;
> > +	const __u8 table_uid[OPAL_UID_LENGTH];
> > +	__u64 offset;
> > +	__u64 size;
> > +	#define OPAL_TABLE_READ (1 << 0)
> > +	#define OPAL_TABLE_WRITE (1 << 1)
> > +	__u64 flags;
> > +	__u64 priv;
> > +};
> 
> Two things, can you double check the pahole on this struct (Google it or ask Jon he knows).
I'll make sure we don't break padding and alignment for v2

> Second, can you lift those defines into Enumerations or out of the struct? Is there a reason
> they're in there?
Just seems to be common coding style for flags, ex fd.h

> 

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

end of thread, other threads:[~2019-08-26 19:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 19:10 [PATCH 0/3] block: sed-opal - Generic Read/Write Opal Tables Revanth Rajashekar
2019-08-21 19:10 ` [PATCH 1/3] block: sed-opal: Expose enum opal_uid and opaluid definitions to the users by moving it to "include/uapi/linux/sed-opal.h" Revanth Rajashekar
2019-08-21 19:10 ` [PATCH 2/3] block: sed-opal: Generalizing write data to any opal table Revanth Rajashekar
2019-08-21 19:10 ` [PATCH 3/3] block: sed-opal: Add support to read/write opal tables generically Revanth Rajashekar
2019-08-23 16:58   ` Derrick, Jonathan
2019-08-25 20:08   ` Scott Bauer
2019-08-26 19:27     ` Derrick, Jonathan

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).