Re: [PATCH v4 06/09] cxl/mem: Enable commands via CEL

2021-02-14 Thread Ben Widawsky
On 21-02-13 21:46:33, Ben Widawsky wrote:
> CXL devices identified by the memory-device class code must implement
> the Device Command Interface (described in 8.2.9 of the CXL 2.0 spec).
> While the driver already maintains a list of commands it supports, there
> is still a need to be able to distinguish between commands that the
> driver knows about from commands that are optionally supported by the
> hardware.
> 
> The Command Effects Log (CEL) is specified in the CXL 2.0 specification.
> The CEL is one of two types of logs, the other being vendor specific.
> They are distinguished in hardware/spec via UUID. The CEL is useful for
> 2 things:
> 1. Determine which optional commands are supported by the CXL device.
> 2. Enumerate any vendor specific commands
> 
> The CEL is used by the driver to determine which commands are available
> in the hardware and therefore which commands userspace is allowed to
> execute. The set of enabled commands might be a subset of commands which
> are advertised in UAPI via CXL_MEM_SEND_COMMAND IOCTL.
> 
> With the CEL enabling comes a internal flag to indicate a base set of
> commands that are enabled regardless of CEL. Such commands are required
> for basic interaction with the hardware and thus can be useful in debug
> cases, for example if the CEL is corrupted.
> 
> The implementation leaves the statically defined table of commands and
> supplements it with a bitmap to determine commands that are enabled.
> This organization was chosen for the following reasons:
> - Smaller memory footprint. Doesn't need a table per device.
> - Reduce memory allocation complexity.
> - Fixed command IDs to opcode mapping for all devices makes development
>   and debugging easier.
> - Certain helpers are easily achievable, like cxl_for_each_cmd().
> 
> Signed-off-by: Ben Widawsky 
> Reviewed-by: Dan Williams 

See below for explanation on why the v4...

> ---
>  drivers/cxl/cxl.h|   2 +
>  drivers/cxl/mem.c| 213 ++-
>  include/uapi/linux/cxl_mem.h |   1 +
>  3 files changed, 213 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 63d7f7e01b83..a1aa9f787a2a 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -67,6 +67,7 @@ struct cxl_memdev;
>   *(CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register)
>   * @mbox_mutex: Mutex to synchronize mailbox access.
>   * @firmware_version: Firmware version for the memory device.
> + * @enabled_commands: Hardware commands found enabled in CEL.
>   * @pmem: Persistent memory capacity information.
>   * @ram: Volatile memory capacity information.
>   */
> @@ -82,6 +83,7 @@ struct cxl_mem {
>   size_t payload_size;
>   struct mutex mbox_mutex; /* Protects device mailbox and firmware */
>   char firmware_version[0x10];
> + unsigned long *enabled_cmds;
>  
>   struct range pmem_range;
>   struct range ram_range;
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index a819f090ffe2..712a8ccc496a 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -45,6 +45,8 @@ enum opcode {
>   CXL_MBOX_OP_INVALID = 0x,
>   CXL_MBOX_OP_RAW = CXL_MBOX_OP_INVALID,
>   CXL_MBOX_OP_ACTIVATE_FW = 0x0202,
> + CXL_MBOX_OP_GET_SUPPORTED_LOGS  = 0x0400,
> + CXL_MBOX_OP_GET_LOG = 0x0401,
>   CXL_MBOX_OP_IDENTIFY= 0x4000,
>   CXL_MBOX_OP_SET_PARTITION_INFO  = 0x4101,
>   CXL_MBOX_OP_SET_LSA = 0x4103,
> @@ -103,10 +105,28 @@ static DEFINE_IDA(cxl_memdev_ida);
>  static struct dentry *cxl_debugfs;
>  static bool cxl_raw_allow_all;
>  
> +enum {
> + CEL_UUID,
> + VENDOR_DEBUG_UUID,
> +};
> +
> +/* See CXL 2.0 Table 170. Get Log Input Payload */
> +static const uuid_t log_uuid[] = {
> + [CEL_UUID] = UUID_INIT(0xda9c0b5, 0xbf41, 0x4b78, 0x8f, 0x79, 0x96,
> +0xb1, 0x62, 0x3b, 0x3f, 0x17),
> + [VENDOR_DEBUG_UUID] = UUID_INIT(0xe1819d9, 0x11a9, 0x400c, 0x81, 0x1f,
> + 0xd6, 0x07, 0x19, 0x40, 0x3d, 0x86),
> +};
> +
>  /**
>   * struct cxl_mem_command - Driver representation of a memory device command
>   * @info: Command information as it exists for the UAPI
>   * @opcode: The actual bits used for the mailbox protocol
> + * @flags: Set of flags effecting driver behavior.
> + *
> + *  * %CXL_CMD_FLAG_FORCE_ENABLE: In cases of error, commands with this flag
> + *will be enabled by the driver regardless of what hardware may have
> + *advertised.
>   *
>   * The cxl_mem_command is the driver's internal representation of commands 
> that
>   * are supported by the driver. Some of these commands may not be supported 
> by
> @@ -118,9 +138,12 @@ static bool cxl_raw_allow_all;
>  struct cxl_mem_command {
>   struct cxl_command_info info;
>   enum opcode opcode;
> + u32 flags;
> +#define CXL_CMD_FLAG_NONE 0
> +#define CXL_CMD_FLAG_FORCE_ENA

[PATCH v4 06/09] cxl/mem: Enable commands via CEL

2021-02-13 Thread Ben Widawsky
CXL devices identified by the memory-device class code must implement
the Device Command Interface (described in 8.2.9 of the CXL 2.0 spec).
While the driver already maintains a list of commands it supports, there
is still a need to be able to distinguish between commands that the
driver knows about from commands that are optionally supported by the
hardware.

The Command Effects Log (CEL) is specified in the CXL 2.0 specification.
The CEL is one of two types of logs, the other being vendor specific.
They are distinguished in hardware/spec via UUID. The CEL is useful for
2 things:
1. Determine which optional commands are supported by the CXL device.
2. Enumerate any vendor specific commands

The CEL is used by the driver to determine which commands are available
in the hardware and therefore which commands userspace is allowed to
execute. The set of enabled commands might be a subset of commands which
are advertised in UAPI via CXL_MEM_SEND_COMMAND IOCTL.

With the CEL enabling comes a internal flag to indicate a base set of
commands that are enabled regardless of CEL. Such commands are required
for basic interaction with the hardware and thus can be useful in debug
cases, for example if the CEL is corrupted.

The implementation leaves the statically defined table of commands and
supplements it with a bitmap to determine commands that are enabled.
This organization was chosen for the following reasons:
- Smaller memory footprint. Doesn't need a table per device.
- Reduce memory allocation complexity.
- Fixed command IDs to opcode mapping for all devices makes development
  and debugging easier.
- Certain helpers are easily achievable, like cxl_for_each_cmd().

Signed-off-by: Ben Widawsky 
Reviewed-by: Dan Williams 
---
 drivers/cxl/cxl.h|   2 +
 drivers/cxl/mem.c| 213 ++-
 include/uapi/linux/cxl_mem.h |   1 +
 3 files changed, 213 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 63d7f7e01b83..a1aa9f787a2a 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -67,6 +67,7 @@ struct cxl_memdev;
  *(CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register)
  * @mbox_mutex: Mutex to synchronize mailbox access.
  * @firmware_version: Firmware version for the memory device.
+ * @enabled_commands: Hardware commands found enabled in CEL.
  * @pmem: Persistent memory capacity information.
  * @ram: Volatile memory capacity information.
  */
@@ -82,6 +83,7 @@ struct cxl_mem {
size_t payload_size;
struct mutex mbox_mutex; /* Protects device mailbox and firmware */
char firmware_version[0x10];
+   unsigned long *enabled_cmds;
 
struct range pmem_range;
struct range ram_range;
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index a819f090ffe2..712a8ccc496a 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -45,6 +45,8 @@ enum opcode {
CXL_MBOX_OP_INVALID = 0x,
CXL_MBOX_OP_RAW = CXL_MBOX_OP_INVALID,
CXL_MBOX_OP_ACTIVATE_FW = 0x0202,
+   CXL_MBOX_OP_GET_SUPPORTED_LOGS  = 0x0400,
+   CXL_MBOX_OP_GET_LOG = 0x0401,
CXL_MBOX_OP_IDENTIFY= 0x4000,
CXL_MBOX_OP_SET_PARTITION_INFO  = 0x4101,
CXL_MBOX_OP_SET_LSA = 0x4103,
@@ -103,10 +105,28 @@ static DEFINE_IDA(cxl_memdev_ida);
 static struct dentry *cxl_debugfs;
 static bool cxl_raw_allow_all;
 
+enum {
+   CEL_UUID,
+   VENDOR_DEBUG_UUID,
+};
+
+/* See CXL 2.0 Table 170. Get Log Input Payload */
+static const uuid_t log_uuid[] = {
+   [CEL_UUID] = UUID_INIT(0xda9c0b5, 0xbf41, 0x4b78, 0x8f, 0x79, 0x96,
+  0xb1, 0x62, 0x3b, 0x3f, 0x17),
+   [VENDOR_DEBUG_UUID] = UUID_INIT(0xe1819d9, 0x11a9, 0x400c, 0x81, 0x1f,
+   0xd6, 0x07, 0x19, 0x40, 0x3d, 0x86),
+};
+
 /**
  * struct cxl_mem_command - Driver representation of a memory device command
  * @info: Command information as it exists for the UAPI
  * @opcode: The actual bits used for the mailbox protocol
+ * @flags: Set of flags effecting driver behavior.
+ *
+ *  * %CXL_CMD_FLAG_FORCE_ENABLE: In cases of error, commands with this flag
+ *will be enabled by the driver regardless of what hardware may have
+ *advertised.
  *
  * The cxl_mem_command is the driver's internal representation of commands that
  * are supported by the driver. Some of these commands may not be supported by
@@ -118,9 +138,12 @@ static bool cxl_raw_allow_all;
 struct cxl_mem_command {
struct cxl_command_info info;
enum opcode opcode;
+   u32 flags;
+#define CXL_CMD_FLAG_NONE 0
+#define CXL_CMD_FLAG_FORCE_ENABLE BIT(0)
 };
 
-#define CXL_CMD(_id, sin, sout)
\
+#define CXL_CMD(_id, sin, sout, _flags)
\
[CXL_MEM_COMMAND_ID_##_id] = { \