Re: [ndctl patch RFC 0/2] add support for IDENTIFY command

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Mar 07, 2023 at 05:21:00PM +0900, Jehoon Park wrote:
> From: jehoon park <jehoon.park@samsung.com>
> 
> This patchset supports CXL IDENTIFY mailbox command and corresponding
> cxl tool interface command.
> 
> CXL 3.0 Spec 8.2.9.8.1 defines IDENTIFY command which retrieves basic
> information about CXL memory device. The information consist of device's
> firmware version, capacity, LSA size, event log size, poison list size,
> inject poison limit, poison handling capabilities and QoS telemetry
> capabilities. Firmware version, capacity and LSA size are already supported
> and used for partition commands or sysfs attributes while others are not.
> Since patches about event log [1] and poison [2] are discussed recently,
> support for those information will be helpful.

Hi Jehoon,

Does this need to be a separate command? Identify fields can be included
in cxl list options. For example, the -I option to cxl list, issues the
identify command and includes the partition related entries in that json
output.

There are other identify fields that need to be picked up, like the
poison related fields.  They need to be added to the cxl list 
options. We may want to include some when we list the poison, and
some as an option in the memdev listing.

Is there some reasoning behind separating this out? If not, can we look
to add the missing fields to the various cxl-list options and add
new cxl-list options where needed?

Alison

> 
> Example:

Users expect json formatted output here.


> # cxl identify mem0
> FW Revision                              : BWFW VERSION 00
> Total Capacity                           : 1.00 GB
> Volatile Only Capacity                   : 1.00 GB
> Persistent Only Capacity                 : 0 B
> Partition Alignment                      : 0 B
> Informational Event Log Size             : 0
> Warning Event Log Size                   : 0
> Failure Event Log Size                   : 0
> Fatal Event Log Size                     : 0
> LSA Size                                 : 0 B
> Poison List Maximum Media Error Records  : 256
> Inject Poison Limit                      : 0
> Poison Handling Capabilities
> Injects Persistent Poison                : Not Supported
> Scans for Poison                         : Not Supported
> QoS Telemetry Capabilities
> Egress Port Congestion                   : Not Supported
> Temporary Throughput Reduction           : Not Supported
> cxl memdev: cmd_identify: identified 1 mem
> 
> This patch is RFC because some of the information are already provided by
> "list -m <memdev>" command and sysfs attributes.
> I think a separate cxl tool interface command for identify is useful to provide
> device's information clearly. In case of nvme-cli, there are separate
> interface commands for identifying controller and namespace: id-ctrl and id-ns.
> 
> [1] https://lore.kernel.org/linux-cxl/20221216-cxl-ev-log-v7-0-2316a5c8f7d8@intel.com/
> [2] https://lore.kernel.org/linux-cxl/cover.1676685180.git.alison.schofield@intel.com/
> 
> jehoon park (2):
>   libcxl: add accessors for IDENTIFY command
>   cxl: add identify command to cxl tool
> 
>  Documentation/cxl/cxl-identify.txt |  57 ++++++++++++
>  Documentation/cxl/meson.build      |   1 +
>  cxl/builtin.h                      |   1 +
>  cxl/cxl.c                          |   1 +
>  cxl/lib/libcxl.c                   |  73 +++++++++++++++
>  cxl/lib/libcxl.sym                 |  11 +++
>  cxl/lib/private.h                  |  11 +++
>  cxl/libcxl.h                       |  16 ++++
>  cxl/memdev.c                       | 141 +++++++++++++++++++++++++++++
>  9 files changed, 312 insertions(+)
>  create mode 100644 Documentation/cxl/cxl-identify.txt
> 
> 
> base-commit: b830c4af984e72e5849c0705669aad2ffa19db13
> -- 
> 2.17.1
>