linux-cxl.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "Widawsky, Ben" <ben.widawsky@intel.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
	"Schofield, Alison" <alison.schofield@intel.com>,
	"Weiny, Ira" <ira.weiny@intel.com>
Subject: Re: [ndctl PATCH v3 02/21] cxl: add a cxl utility and libcxl library
Date: Tue, 13 Jul 2021 21:14:53 +0000	[thread overview]
Message-ID: <16e7814b0d3f1345dd59a38ccb8dc2f93a069f88.camel@intel.com> (raw)
In-Reply-To: <CAPcyv4guuNXZSUK6s3vT8a0e9M69kGxpu14dbBXZOeJvyF2S6Q@mail.gmail.com>

On Fri, 2021-07-09 at 18:12 -0700, Dan Williams wrote:
> On Thu, Jul 1, 2021 at 1:10 PM Vishal Verma <vishal.l.verma@intel.com> 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 <ben.widawsky@intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> 
> Looks good, just a couple minor quibbles below:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

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.


  reply	other threads:[~2021-07-13 21:15 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 20:09 [ndctl PATCH v3 00/21] Initial CXL support Vishal Verma
2021-07-01 20:09 ` [ndctl PATCH v3 01/21] ndctl: add .clang-format Vishal Verma
2021-07-10  1:12   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 02/21] cxl: add a cxl utility and libcxl library Vishal Verma
2021-07-10  1:12   ` Dan Williams
2021-07-13 21:14     ` Verma, Vishal L [this message]
2021-07-01 20:09 ` [ndctl PATCH v3 03/21] cxl: add a local copy of the cxl_mem UAPI header Vishal Verma
2021-07-10  1:13   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 04/21] libcxl: add support for command query and submission Vishal Verma
2021-07-13  5:12   ` Dan Williams
2021-07-13 21:17     ` Verma, Vishal L
2021-07-13 21:39       ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 05/21] libcxl: add support for the 'Identify Device' command Vishal Verma
2021-07-15 23:50   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 06/21] test: rename 'ndctl_test' to 'test_ctx' Vishal Verma
2021-07-15 23:51   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 07/21] test: rename 'ndctl_test_*' helpers to 'test_*' Vishal Verma
2021-07-15 23:51   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 08/21] test: introduce a libcxl unit test Vishal Verma
2021-07-15 23:53   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 09/21] libcxl: add GET_HEALTH_INFO mailbox command and accessors Vishal Verma
2021-07-16  1:11   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 10/21] libcxl: add support for the 'GET_LSA' command Vishal Verma
2021-07-16  2:24   ` Dan Williams
2021-07-01 20:09 ` [ndctl PATCH v3 11/21] util/hexdump: Add a util helper to print a buffer in hex Vishal Verma
2021-07-01 20:09 ` [ndctl PATCH v3 12/21] test/libcxl: add a test for {set, get}_lsa commands Vishal Verma
2021-07-01 20:09 ` [ndctl PATCH v3 13/21] test/libcxl: introduce a command size fuzzing test Vishal Verma
2021-07-01 20:09 ` [ndctl PATCH v3 14/21] libcxl: add lsa_size to cxl_memdev, and an API to retrieve it Vishal Verma
2021-07-01 20:09 ` [ndctl PATCH v3 15/21] libcxl: PLACEHOLDER: add an interface to determine whether a memdev is active Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 16/21] libcxl: add interfaces for label operations Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 17/21] test/libcxl: add a test for cxl_memdev_{get,set}_lsa Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 18/21] cxl: add commands to read, write, and zero labels Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 19/21] Documentation/cxl: add library API documentation Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 20/21] ndctl: Add CXL packages to the RPM spec Vishal Verma
2021-07-01 20:10 ` [ndctl PATCH v3 21/21] cxl-cli: add bash completion Vishal Verma
2021-07-01 20:13 ` [ndctl PATCH v3 00/21] Initial CXL support Verma, Vishal L

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16e7814b0d3f1345dd59a38ccb8dc2f93a069f88.camel@intel.com \
    --to=vishal.l.verma@intel.com \
    --cc=alison.schofield@intel.com \
    --cc=ben.widawsky@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --subject='Re: [ndctl PATCH v3 02/21] cxl: add a cxl utility and libcxl library' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).