Re: [RFC PATCH 0/13] fwctl/cxl: Add CXL feature commands support via fwctl

Dave Jiang
2024-07-19T15:48:31Z

On 7/18/24 11:23 PM, Alejandro Lucero Palau wrote:
>
> On 7/18/24 22:32, Dave Jiang wrote:
>> This series add support for CXL feature commands using the FWCTL framework [1].
>> The code is untested and I'm looking for architectural and implementation feedback.
>> While CXL currently has a chardev for user ioctls to send some mailbox
>> commands to a memory device, the fwctl framework provides more security policies
>> that can be a potential vehicle to move CXL ioctl path to that.
>>
>> For this RFC, the mailbox commands "Get Supported Features", "Get Feature", and
>> "Set Feature" commands are implemented. The "get" commands under the
>> FWCTL_RPC_DEBUG_READ_ONLY policy, the "set" command checks the policy depending
>> on the effect of the feature. All mailbox commands for CXL provides an effects
>> table that describes the effects of a command when performed on the device.
>> For CXL features, there is also an effects field that describes the effects
>> a feature write operation has on the device per feature. The security policy
>> is checked against this feature specific effects field. Looking for discussion
>> on matching the CXL spec defined effects with the FWCTL security policy.
>>
>> The code is based off of the latest FWCTL series [1] posted by Jason on top of v6.10.
>>
>> [1]: https://lore.kernel.org/linux-cxl/20240624161802.1b7c962d@kernel.org/T/#t
>>
>> ---
>>
>> Dave Jiang (13):
>>        cxl: Move mailbox related bits to the same context
>>        cxl: Fix comment regarding cxl_query_cmd() return data
>>        cxl: Refactor user ioctl command path from mds to mailbox
>>        cxl: Add Get Supported Features command for kernel usage
>>        cxl/test: Add Get Supported Features mailbox command support
>>        cxl: Add Get Feature command support
>>        cxl: Add Set Feature command support
>>        fwctl/cxl: Add driver for CXL mailbox for handling CXL features commands
>>        fwctl/cxl: Add support for get driver information
>>        fwctl/cxl: Add support for fwctl RPC command to enable CXL feature commands
>>        fwctl/cxl: Add query commands software command for ->fw_rpc()
>>        cxl/test: Add Get Feature support to cxl_test
>>        cxl/test: Add Set Feature support to cxl_test
>>
>>   MAINTAINERS                  |   8 +
>>   drivers/cxl/core/core.h      |   9 +-
>>   drivers/cxl/core/mbox.c      | 607 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>   drivers/cxl/core/memdev.c    |  78 +++++---
>>   drivers/cxl/cxlmem.h         | 141 +++------------
>>   drivers/cxl/pci.c            |  68 ++++---
>>   drivers/cxl/pmem.c           |  10 +-
>>   drivers/cxl/security.c       |  18 +-
>>   drivers/fwctl/Kconfig        |   9 +
>>   drivers/fwctl/Makefile       |   1 +
>>   drivers/fwctl/cxl/Makefile   |   4 +
>>   drivers/fwctl/cxl/cxl.c      | 274 ++++++++++++++++++++++++++++
>>   include/linux/cxl/mailbox.h  | 175 ++++++++++++++++++
>>   include/uapi/fwctl/cxl.h     |  92 ++++++++++
>>   include/uapi/fwctl/fwctl.h   |   1 +
>>   include/uapi/linux/cxl_mem.h |   3 +
>>   tools/testing/cxl/test/mem.c | 292 +++++++++++++++++++++++++++++-
>>   17 files changed, 1515 insertions(+), 275 deletions(-)
>>
>
> I can not see any documentation change in that file list.
>
> I would say for something like what this patchset does, it is mandatory. Isn't it?

Yeah I will write some documentation to detail the new ioctl API. Thanks for the reminder.
>