diff mbox series

[RFC,14/14] WIP/cxl/mem: Add get firmware for testing

Message ID 20201209002418.1976362-15-ben.widawsky@intel.com
State New
Headers show
Series CXL 2.0 Support | expand

Commit Message

Ben Widawsky Dec. 9, 2020, 12:24 a.m. UTC
This also serves as an example how to add a new command

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 drivers/cxl/mem.c            | 22 ++++++++++++++++++++++
 include/uapi/linux/cxl_mem.h |  3 ++-
 2 files changed, 24 insertions(+), 1 deletion(-)

Comments

Dan Williams Dec. 10, 2020, 12:42 a.m. UTC | #1
On Tue, Dec 8, 2020 at 4:25 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>

> This also serves as an example how to add a new command

>

> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

> ---

>  drivers/cxl/mem.c            | 22 ++++++++++++++++++++++

>  include/uapi/linux/cxl_mem.h |  3 ++-

>  2 files changed, 24 insertions(+), 1 deletion(-)

>

> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c

> index 6b2f8d3776b5..76aa1e6e4117 100644

> --- a/drivers/cxl/mem.c

> +++ b/drivers/cxl/mem.c

> @@ -116,6 +116,7 @@ struct cxl_mem_command {

>  static struct cxl_mem_command mem_commands[] = {

>         CXL_CMD(INVALID, NONE, 0, 0, "Reserved", false, 0),

>         CXL_CMD(RAW, TAINT, ~0, ~0, "Raw", true, 0),

> +       CXL_CMD(GET_FW_INFO, NONE, 0, 0x50, "Get FW Info", false, 0x0200),


I think CXL_CMD() arguments can be put on a diet if the
mem-command-id-to-name was moved to its own table, and the opcode was
defined as something like:

#define CXL_MBOX_OP_GET_FW_INFO 0x200

...so that CXL_CMD can just do:

    .opcode = CXL_MBOX_OP_##_id

That @_enable arg wants a flag #define like CMD_ENABLE which reads
better than 'true'. I would then add CMD_KERNEL_EXCL alongside that
flag to indicate the commands that may be exclusive to the kernel when
the device is active in a PMEM memory region, or ones that have an
alternate ABI wrapped around them like the keys subsystem for security
or the firwmare activation sysfs interface.

>  };

>

>  static int cxl_mem_wait_for_doorbell(struct cxl_mem *cxlm)

> @@ -827,6 +828,23 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)

>         return cxl_register(dev);

>  }

>

> +static int cxl_mem_enable_commands(struct cxl_mem *cxlm)

> +{

> +       struct cxl_mem_command *c;

> +

> +       /*

> +        * For now we pretend Get FW info is supported.

> +        *

> +        * FIXME: Invoke GET LOG to get the Command Effect Logs (CEL).

> +        */

> +       c = cxl_mem_find_command(0x200);

> +       if (!c)

> +               return -ENOENT;

> +

> +       c->enable = true;

> +       return 0;

> +}

> +

>  /**

>   * cxl_mem_identify() - Send the IDENTIFY command to the device.

>   * @cxlm: The device to identify.

> @@ -936,6 +954,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)

>         if (rc)

>                 return rc;

>

> +       rc = cxl_mem_enable_commands(cxlm);

> +       if (rc)

> +               return rc;

> +

>         rc = cxl_mem_identify(cxlm);

>         if (rc)

>                 return rc;

> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h

> index f2fbb0dcda06..3ac39acf8fa7 100644

> --- a/include/uapi/linux/cxl_mem.h

> +++ b/include/uapi/linux/cxl_mem.h

> @@ -50,7 +50,8 @@ struct cxl_command_info {

>         __u32 id;

>  #define CXL_MEM_COMMAND_ID_INVALID 0

>  #define CXL_MEM_COMMAND_ID_RAW 1

> -#define CXL_MEM_COMMAND_ID_MAX (CXL_MEM_COMMAND_ID_RAW + 1)

> +#define CXL_MEM_COMMAND_ID_GET_FW_INFO 2

> +#define CXL_MEM_COMMAND_ID_MAX (CXL_MEM_COMMAND_ID_GET_FW_INFO + 1)


Seems like CXL_MEM_COMMAND_ID definitions want to be an enum so that
CXL_MEM_COMMAND_ID_MAX does not need to be touched every time.

>

>         __u32 flags;

>  #define CXL_MEM_COMMAND_FLAG_NONE 0

> --

> 2.29.2

>
diff mbox series

Patch

diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 6b2f8d3776b5..76aa1e6e4117 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -116,6 +116,7 @@  struct cxl_mem_command {
 static struct cxl_mem_command mem_commands[] = {
 	CXL_CMD(INVALID, NONE, 0, 0, "Reserved", false, 0),
 	CXL_CMD(RAW, TAINT, ~0, ~0, "Raw", true, 0),
+	CXL_CMD(GET_FW_INFO, NONE, 0, 0x50, "Get FW Info", false, 0x0200),
 };
 
 static int cxl_mem_wait_for_doorbell(struct cxl_mem *cxlm)
@@ -827,6 +828,23 @@  static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
 	return cxl_register(dev);
 }
 
+static int cxl_mem_enable_commands(struct cxl_mem *cxlm)
+{
+	struct cxl_mem_command *c;
+
+	/*
+	 * For now we pretend Get FW info is supported.
+	 *
+	 * FIXME: Invoke GET LOG to get the Command Effect Logs (CEL).
+	 */
+	c = cxl_mem_find_command(0x200);
+	if (!c)
+		return -ENOENT;
+
+	c->enable = true;
+	return 0;
+}
+
 /**
  * cxl_mem_identify() - Send the IDENTIFY command to the device.
  * @cxlm: The device to identify.
@@ -936,6 +954,10 @@  static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = cxl_mem_enable_commands(cxlm);
+	if (rc)
+		return rc;
+
 	rc = cxl_mem_identify(cxlm);
 	if (rc)
 		return rc;
diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index f2fbb0dcda06..3ac39acf8fa7 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -50,7 +50,8 @@  struct cxl_command_info {
 	__u32 id;
 #define CXL_MEM_COMMAND_ID_INVALID 0
 #define CXL_MEM_COMMAND_ID_RAW 1
-#define CXL_MEM_COMMAND_ID_MAX (CXL_MEM_COMMAND_ID_RAW + 1)
+#define CXL_MEM_COMMAND_ID_GET_FW_INFO 2
+#define CXL_MEM_COMMAND_ID_MAX (CXL_MEM_COMMAND_ID_GET_FW_INFO + 1)
 
 	__u32 flags;
 #define CXL_MEM_COMMAND_FLAG_NONE 0