Re: [ndctl 2/3] cxl: add inject-poison command to cxl tool

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Tue, 2023-02-28 at 18:43 +0900, Junhyeok Im wrote:
> On Sun, Feb 26, 2023 at 07:21:44PM -0800, Alison Schofield wrote:
> 
> 
> > 
<..>

> > > Also since it is expected no use case of injecting poison into the
> > > same address for multiple devices, this command targets only one
> > > memdev, like write-labels command.
> > > 
> > >  usage: cxl inject-poison <memdev> -a <dpa> [<options>]
> > 
> > maybe -m memdev (to be like others)
> 
> AFAIK, memdev is to be specified without "-m" except for 'list' cmd,
> but I think the reason you said is to distinguish memdev from region(e.g."-r").
> (I also found in your previous patch for GET_POISON_LIST that 
> memdev and region were separated.)
> If so, codes and documentation should be modified like:
>     cxl inject-poison -m <memdev> -a <dpa> [<options>]

Just wanted to chime in on this quickly before having gone through the
rest of the series -

The convention has been if a command can only have one type of
'target', then no need for a -m, or -r etc.

e.g. cxl enable-region region0

But if it can (potentially) have different types of targets, then an
option can be used to determine the target type. e.g. cxl-create-region
was originally intending to allow both memdev and endpoint targets,
hence the -m option to say that the non-option arguments should be
treated as memdevs.

The other place where you'd have options like -m="mem0,mem1" etc is for
filtering or restricting an operation. cxl-list uses the different
options to filter results, but also other commands:

  e.g. cxl disable-region --bus=cxl_test all

will disable all regions under cxl_test, but leave alone regions under
another bus.