On Fri, 2021-07-09 at 18:12 -0700, Dan Williams wrote:
> On Thu, Jul 1, 2021 at 1:10 PM Vishal Verma <[email protected]> wrote:
> >
> > CXL - or Compute eXpress Link - is a new interconnect that extends PCIe
> > to support a wide range of devices, including cache coherent memory
> > expanders. As such, these devices can be new sources of 'persistent
> > memory', and the 'ndctl' umbrella of tools and libraries needs to be able
> > to interact with them.
> >
> > Add a new utility and library for managing these CXL memory devices. This
> > is an initial bring-up for interacting with CXL devices, and only includes
> > adding the utility and library infrastructure, parsing device information
> > from sysfs for CXL devices, and providing a 'cxl-list' command to
> > display this information in JSON formatted output.
> >
> > Cc: Ben Widawsky <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Signed-off-by: Vishal Verma <[email protected]>
>
> Looks good, just a couple minor quibbles below:
>
> Reviewed-by: Dan Williams <[email protected]>
Thanks Dan - addressed as below.
>
> > ---
> > Documentation/cxl/cxl-list.txt | 64 +++++
> > Documentation/cxl/cxl.txt | 34 +++
> > Documentation/cxl/human-option.txt | 8 +
> > Documentation/cxl/verbose-option.txt | 5 +
> > configure.ac | 3 +
> > Makefile.am | 8 +-
> > Makefile.am.in | 4 +
> > cxl/lib/private.h | 29 +++
> > cxl/lib/libcxl.c | 345 +++++++++++++++++++++++++++
> > cxl/builtin.h | 8 +
> > cxl/libcxl.h | 55 +++++
> > util/filter.h | 2 +
> > util/json.h | 3 +
> > util/main.h | 3 +
> > cxl/cxl.c | 96 ++++++++
> > cxl/list.c | 113 +++++++++
> > util/filter.c | 20 ++
> > util/json.c | 26 ++
> > .clang-format | 1 +
> > .gitignore | 4 +-
> > Documentation/cxl/Makefile.am | 58 +++++
> > cxl/Makefile.am | 21 ++
> > cxl/lib/Makefile.am | 32 +++
> > cxl/lib/libcxl.pc.in | 11 +
> > cxl/lib/libcxl.sym | 29 +++
> > 25 files changed, 978 insertions(+), 4 deletions(-)
> > create mode 100644 Documentation/cxl/cxl-list.txt
> > create mode 100644 Documentation/cxl/cxl.txt
> > create mode 100644 Documentation/cxl/human-option.txt
> > create mode 100644 Documentation/cxl/verbose-option.txt
> > create mode 100644 cxl/lib/private.h
> > create mode 100644 cxl/lib/libcxl.c
> > create mode 100644 cxl/builtin.h
> > create mode 100644 cxl/libcxl.h
> > create mode 100644 cxl/cxl.c
> > create mode 100644 cxl/list.c
> > create mode 100644 Documentation/cxl/Makefile.am
> > create mode 100644 cxl/Makefile.am
> > create mode 100644 cxl/lib/Makefile.am
> > create mode 100644 cxl/lib/libcxl.pc.in
> > create mode 100644 cxl/lib/libcxl.sym
> >
> > diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> > new file mode 100644
> > index 0000000..4e2be87
> > --- /dev/null
> > +++ b/Documentation/cxl/cxl-list.txt
> > @@ -0,0 +1,64 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +cxl-list(1)
> > +===========
> > +
> > +NAME
> > +----
> > +cxl-list - List CXL capable memory devices, and their attributes in json.
>
> This will also show CXL port topology in the future. I'm fine to fix
> that up later when that support arrives.
Yep I actually removed it based on feedback from Ben saying we should
add those when the actual support for those is added.
>
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'cxl list' [<options>]
> > +
> > +Walk the CXL capable device hierarchy in the system and list all device
> > +instances along with some of their major attributes.
> > +
> > +Options can be specified to limit the output to specific devices.
> > +By default, 'cxl list' with no options is equivalent to:
> > +[verse]
> > +cxl list --devices
> > +
> > +EXAMPLE
> > +-------
> > +----
> > +# cxl list --devices
>
> Is this from an earlier version, should it be --memdevs?
Yep stale, fixed for v4.
>
> > +{
> > + "memdev":"mem0",
> > + "pmem_size":268435456,
> > + "ram_size":0,
> > +}
> > +----
> > +
> > +OPTIONS
> > +-------
> > +-d::
> > +--memdev=::
> > + Specify a cxl memory device name to filter the listing. For example:
> > +----
> > +# cxl list --memdev=mem0
> > +{
> > + "memdev":"mem0",
> > + "pmem_size":268435456,
> > + "ram_size":0,
> > +}
> > +----
> > +
> > +-D::
> > +--memdevs::
> > + Include all CXL memory devices in the listing
> > +
> > +-i::
> > +--idle::
> > + Include idle (not enabled / zero-sized) devices in the listing
> > +
> > +include::human-option.txt[]
> > +
> > +include::verbose-option.txt[]
> > +
> > +include::../copyright.txt[]
> > +
> > +SEE ALSO
> > +--------
> > +linkcxl:ndctl-list[1]
> > diff --git a/Documentation/cxl/cxl.txt b/Documentation/cxl/cxl.txt
> > new file mode 100644
> > index 0000000..e99e61b
> > --- /dev/null
> > +++ b/Documentation/cxl/cxl.txt
> > @@ -0,0 +1,34 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +cxl(1)
> > +======
> > +
> > +NAME
> > +----
> > +cxl - Provides enumeration and provisioning commands for CXL devices
>
> s/device/platforms/ since it should enumerate an entire topology.
Good catch, fixed.
