On Tue, Dec 8, 2020 at 4:24 PM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: > > The CXL memory device send interface will have a number of supported > commands. The raw command is not such a command. Raw commands allow > userspace to send a specified opcode to the underlying hardware and > bypass all driver checks on the command. This is useful for a couple of > usecases, mainly: > 1. Undocumented vendor specific hardware commands > 2. Prototyping new hardware commands not yet supported by the driver > > While this all sounds very powerful it comes with a couple of caveats: > 1. Bug reports using raw commands will not get the same level of > attention as bug reports using supported commands (via taint). > 2. Supported commands will be rejected by the RAW command. > > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx> > --- > drivers/cxl/mem.c | 32 ++++++++++++++++++++++++++++++++ > include/uapi/linux/cxl_mem.h | 14 ++++++++++++-- > 2 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index 0bf03afc0c80..a2cea7ac7cc6 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -115,6 +115,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), Why is the taint indication in the ABI? It seems like it only needs to be documented. > }; > > static int cxl_mem_wait_for_doorbell(struct cxl_mem *cxlm) > @@ -326,6 +327,20 @@ static int cxl_mem_count_commands(void) > return n; > }; > > +static struct cxl_mem_command *cxl_mem_find_command(u16 opcode) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(mem_commands); i++) { > + struct cxl_mem_command *c = &mem_commands[i]; > + > + if (c->opcode == opcode) > + return c; > + } > + > + return NULL; > +}; > + > /** > * handle_mailbox_cmd_from_user() - Dispatch a mailbox command. > * @cxlmd: The CXL memory device to communicate with. > @@ -421,6 +436,23 @@ static int cxl_validate_cmd_from_user(struct cxl_send_command __user *user_cmd, > c = &mem_commands[cmd.id]; > info = &c->info; > > + /* Checks are bypassed for raw commands but along comes the taint! */ > + if (cmd.id == CXL_MEM_COMMAND_ID_RAW) { > + struct cxl_mem_command temp = > + CXL_CMD(RAW, NONE, cmd.size_in, cmd.size_out, "Raw", > + true, cmd.raw.opcode); Oh, I thought CXL_CMD() was only used to populate the mem_commands array. Feels out of place to use it here when all it is doing is updating the size_{in,out} and opcode fields. Mainly I'm interested in CXL_CMD() enforcing that the command-id is the mem_commands index. > + > + if (cmd.raw.rsvd) > + return -EINVAL; > + > + if (cxl_mem_find_command(cmd.raw.opcode)) > + return -EPERM; > + > + add_taint(TAINT_WARN, LOCKDEP_STILL_OK); TAINT_WARN seems the wrong value, especially since no WARN has occurred. I feel that this is more in the spirit of TAINT_PROPRIETARY_MODULE, TAINT_OVERRIDDEN_ACPI_TABLE, and TAINT_OOT_MODULE. How about a new TAINT_RAW_PASSTHROUGH? I could use this for the acpi/nfit driver as well to disclaim responsibility for system errors that can result from not using the nominal kernel-provided commands.