| Message ID | 20201209002418.1976362-15-ben.widawsky@intel.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | CXL 2.0 Support | expand |
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 --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
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(-)