Vishal Verma wrote:
> In the common case, root decoders are expected to be either pmem
> capable, or volatile capable, but not necessarily both simultaneously.
> If a decoder only has one of pmem or volatile capabilities,
> cxl-create-region should just infer the type of the region (pmem
> or ram) based on this capability.
>
> Maintain the default behavior of cxl-create-region to choose type=pmem,
> but only as a fallback if the selected root decoder has multiple
> capabilities. If it is only capable of either pmem, or ram, then infer
> region type from this without requiring it to be specified explicitly.
>
> Cc: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
> ---
> Documentation/cxl/cxl-create-region.txt | 3 ++-
> cxl/region.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/cxl/cxl-create-region.txt
> b/Documentation/cxl/cxl-create-region.txt
> index ada0e52..f11a412 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -75,7 +75,8 @@ include::bus-option.txt[]
>
> -t::
> --type=::
> - Specify the region type - 'pmem' or 'ram'. Defaults to 'pmem'.
> + Specify the region type - 'pmem' or 'ram'. Default to root decoder
> + capability, and if that is ambiguous, default to 'pmem'.
>
> -U::
> --uuid=::
> diff --git a/cxl/region.c b/cxl/region.c
> index 9079b2d..1c8ccc7 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -448,6 +448,31 @@ static int validate_decoder(struct cxl_decoder *decoder,
> return 0;
> }
>
> +static void set_type_from_decoder(struct cxl_ctx *ctx, struct parsed_params
> *p)
> +{
> + int num_cap = 0;
> +
> + /* if param.type was explicitly specified, nothing to do here */
> + if (param.type)
> + return;
> +
> + /*
> + * if the root decoder only has one type of capability, default
> + * to that mode for the region.
> + */
> + if (cxl_decoder_is_pmem_capable(p->root_decoder))
> + num_cap++;
> + if (cxl_decoder_is_volatile_capable(p->root_decoder))
> + num_cap++;
> +
> + if (num_cap == 1) {
> + if (cxl_decoder_is_volatile_capable(p->root_decoder))
> + p->mode = CXL_DECODER_MODE_RAM;
> + else if (cxl_decoder_is_pmem_capable(p->root_decoder))
> + p->mode = CXL_DECODER_MODE_PMEM;
> + }
Is @num_cap needed? I.e. if this just does:
if (cxl_decoder_is_volatile_capable(p->root_decoder))
p->mode = CXL_DECODER_MODE_RAM;
if (cxl_decoder_is_pmem_capable(p->root_decoder))
p->mode = CXL_DECODER_MODE_PMEM;
...then it matches the changelog of defaulting to pmem if both types are
set, and otherwise the single capability dominates.
