On Thu, Mar 14, 2024 at 08:35:01PM -0700, Dan Williams wrote:
> Alison Schofield wrote:
> > On Fri, Mar 15, 2024 at 10:09:44AM +0900, Wonjae Lee wrote:
> > > alison.schofi...@intel.com wrote:
> > > > From: Alison Schofield <alison.schofi...@intel.com>
> > > >
> > > > The --media-errors option to 'cxl list' retrieves poison lists from
> > > > memory devices supporting the capability and displays the returned
> > > > media_error records in the cxl list json. This option can apply to
> > > > memdevs or regions.
> > > >
> > > > Include media-errors in the -vvv verbose option.
> > > >
> > > > Example usage in the Documentation/cxl/cxl-list.txt update.
> > > >
> > > > Signed-off-by: Alison Schofield <alison.schofi...@intel.com>
> > > > ---
> > > > Documentation/cxl/cxl-list.txt 62 +++++++++++++++++++++++++++++++++-
> > > > cxl/filter.h 3 ++
> > > > cxl/list.c 3 ++
> > > > 3 files changed, 67 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/cxl/cxl-list.txt
> > > > b/Documentation/cxl/cxl-list.txt
> > > > index 838de4086678..6d3ef92c29e8 100644
> > > > --- a/Documentation/cxl/cxl-list.txt
> > > > +++ b/Documentation/cxl/cxl-list.txt
> > >
> > > [snip]
> > >
> > > +----
> > > +In the above example, region mappings can be found using:
> > > +"cxl list -p mem9 --decoders"
> > > +----
> > >
> > > Hi, isn't it '-m mem9' instead of -p? FYI, it's also on patch's
> > > cover letter, too.
> >
> > Thanks for the review! I went with -p because it gives only
> > the endpoint decoder while -m gives all the decoders up to
> > the root - more than needed to discover the region.
>
> The first thing that comes to mind to list memory devices with their
> decoders is:
>
> cxl list -MD -d endpoint
>
> ...however the problem is that endpoint ports connect memdevs to their
> parent port, so the above results in:
>
> Warning: no matching devices found
>
> I think I want to special case "-d endpoint" when both -M and -D are
> specified to also imply -E, "endpoint ports". However that also seems to
> have a bug at present:
>
> # cxl list -EDM -d endpoint -iu
> {
> "endpoint":"endpoint2",
> "host":"mem0",
> "parent_dport":"0000:34:00.0",
> "depth":2
> }
>
> That needs to be fixed up to merge:
>
> # cxl list -ED -d endpoint -iu
> {
> "endpoint":"endpoint2",
> "host":"mem0",
> "parent_dport":"0000:34:00.0",
> "depth":2,
> "decoders:endpoint2":[
> {
> "decoder":"decoder2.0",
> "interleave_ways":1,
> "state":"disabled"
> }
> ]
> }
>
> ...and:
>
> # cxl list -EMu
> {
> "endpoint":"endpoint2",
> "host":"mem0",
> "parent_dport":"0000:34:00.0",
> "depth":2,
> "memdev":{
> "memdev":"mem0",
> "pmem_size":"512.00 MiB (536.87 MB)",
> "serial":"0",
> "host":"0000:35:00.0"
> }
> }
>
> ...so that one can get a nice listing of just endpoint ports, their
> decoders (with media errors) and their memdevs.
>
> The reason that "cxl list -p mem9 -D" works is subtle because it filters
> the endpoint decoders by an endpoint port filter, but I think most users
> would expect to not need to enable endpoint-port listings to see their
> decoders the natural key to filter endpoint decoders is by memdev.
Wonjae, Dan,
This feedback inspires me to seek more input from future users. This
tool should be adding a convenience and I don't want to proceed without
more user feedback confirming this implementation is more convenient
than the currently available method (trace & trigger). We also want to
avoid working with or around some awkward json output for eternity.
I'm following this response with a reply to the cover letter seeking
more inputs.
Thanks,
Alison
