Re: [RFC PATCH v2 07/20] cxl: Add Set Feature command support for user submission

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Thu, 21 Nov 2024 11:08:44 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> On 11/21/24 10:49 AM, Jonathan Cameron wrote:
> > On Fri, 15 Nov 2024 14:25:40 -0700
> > Dave Jiang <dave.jiang@intel.com> wrote:
> >   
> >> Add enumeration of Set Feature mailbox command for the kernel to recognize
> >> the command being passed in from user space.  
> > 
> > This looks unwise to make available on the ioctl.   
> 
> Do you mean the current memdev char dev ioctl? Maybe we can introduce a restricted_commands mask to block that on the legacy ioctl? Trying to make the two ioctl paths co-exist is a bit of a pain right now.
> 
Yes - I'd rather we didn't introduce new commands on the legacy ioctl
at all, but if we do keeping it to the definitely safe ones (GET_xxx etc)
would be better.

Another mask is a way to solve this, though feels like opt in is
going to be easier to maintain long term than opt out.
Mind you we can modify how exactly it is done later as not
critical yet.

Jonathan

> DJ
>  
> >   
> >>
> >> CXL spec r3.1 8.2.9.6.3 Set Feature (Opcode 0502h)
> >>
> >> The feature requested is identified by specific UUID.
> >>
> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> >> ---
> >>  drivers/cxl/core/mbox.c      | 1 +
> >>  include/uapi/linux/cxl_mem.h | 1 +
> >>  2 files changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> >> index 4b9abf9a5b2b..739444d34130 100644
> >> --- a/drivers/cxl/core/mbox.c
> >> +++ b/drivers/cxl/core/mbox.c
> >> @@ -69,6 +69,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
> >>  	CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0),
> >>  	CXL_CMD(GET_SUPPORTED_FEATURES, 0x8, CXL_VARIABLE_PAYLOAD, 0),
> >>  	CXL_CMD(GET_FEATURE, 0x15, CXL_VARIABLE_PAYLOAD, 0),
> >> +	CXL_CMD(SET_FEATURE, CXL_VARIABLE_PAYLOAD, 0, 0),
> >>  };
> >>  
> >>  /*
> >> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> >> index 90dcb9723997..469dd481795f 100644
> >> --- a/include/uapi/linux/cxl_mem.h
> >> +++ b/include/uapi/linux/cxl_mem.h
> >> @@ -52,6 +52,7 @@
> >>  	___C(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
> >>  	___C(GET_SUPPORTED_FEATURES, "Get Supported Features"),		  \
> >>  	___C(GET_FEATURE, "Get Feature"),				  \
> >> +	___C(SET_FEATURE, "Set Feature"),				  \
> >>  	___C(MAX, "invalid / last command")
> >>  
> >>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a  
> >   
> 
>