Re: [ndctl PATCH v2 1/1] ndctl/cxl: Show region locked status to user

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Apr 02, 2026 at 06:29:26PM +0800, Li Ming wrote:
> 
> 在 2026/4/2 03:06, Alison Schofield 写道:
> > On Wed, Apr 01, 2026 at 08:42:05PM +0800, Li Ming wrote:
> > > A region is not allowed to be destroyed if it is in locked status.
> > > cxl destroy-region command will fail and ask user to try it again as
> > > root, but it is not the real reason and it will confuse user if user is
> > > already a root user. This patch will show the region locked status in
> > > region information and output an explicit log to user that operation is
> > > not permitted like below.
> > > 
> > > Before the patch:
> > > cxl list -ir region0
> > > [
> > >    {
> > >      "region":"region0",
> > >      "resource":53955526656,
> > >      "size":536870912,
> > >      "type":"ram",
> > >      "interleave_ways":2,
> > >      "interleave_granularity":256,
> > >      "decode_state":"commit",
> > >      "state":"disabled",
> > >      "qos_class_mismatch":true
> > >    }
> > > ]
> > > 
> > > cxl destroy-region region0
> > > libcxl: write_attr: failed to write 0
> > >   to /sys/bus/cxl/devices/root0/decoder0.0/region0/commit: Operation not permitted hint: try running as root or using sudo
> > > cxl region: destroy_region: region0: failed to reset decode: Operation not permitted
> > > cxl region: decoder_region_action: region0: failed: Operation not permitted
> > > cxl region: region_action: one or more failures, last failure: Operation not permitted
> > > cxl region: cmd_destroy_region: destroyed 0 regions
> > > 
> > > After the patch:
> > > cxl list -ir region0
> > > [
> > >    {
> > >      "region":"region0",
> > >      "resource":53955526656,
> > >      "size":536870912,
> > >      "type":"ram",
> > >      "interleave_ways":2,
> > >      "interleave_granularity":256,
> > >      "decode_state":"commit",
> > >      "state":"disabled",
> > >      "locked":true,
> > >      "qos_class_mismatch":true
> > >    }
> > > ]
> > > 
> > > cxl destroy-region region0
> > > cxl region: destroy_region: region0: Cannot destroy a locked region.
> > > cxl region: decoder_region_action: region0: failed: Operation not permitted
> > > cxl region: region_action: one or more failures, last failure: Operation not permitted
> > > cxl region: cmd_destroy_region: destroyed 0 regions
> > Sorry Ming, I missed previously-
> > 
> > This always emits "locked":false on kernels without the new sysfs
> > attribute. Please make it skip the emit in that case.
> 
> Hi Alison, the case you mentioned requires an extra interface implementation
> for checking the new sysfs attribute exists. Or a new variable to represent
> if the new attribute exists.
> 
> How about just skpping the emit when locked == false? It works for both no
> the new sysfs attribute and the region is not locked cases.

Skipping the JSON emit when locked == false papers over cxl-cli output
while leaving libcxl unable to distinguish "attribute absent" from
"attribute present and false". Since libcxl is a public interface for
other tools, it must not collapse those states.

How about a tri-state approach exposed like this in libcxl.h:

enum cxl_region_locked_state {
        CXL_REGION_LOCKED_UNKNOWN = 0,
        CXL_REGION_UNLOCKED,
        CXL_REGION_LOCKED,
};

enum cxl_region_locked_state
cxl_region_locked_state(struct cxl_region *region);

Then cxl-list can omit field if UNKNOWN.

> 
> 
> Ming
> 
> > 
> > > Signed-off-by: Li Ming <ming.li@zohomail.com>
> > > ---
> > > v2:
> > > * Move lock status checking to the beginning of destroy_region().
> > >    (Alison)
> > > ---
> > >   cxl/json.c         |  4 ++++
> > >   cxl/lib/libcxl.c   | 11 +++++++++++
> > >   cxl/lib/libcxl.sym |  5 +++++
> > >   cxl/lib/private.h  |  1 +
> > >   cxl/libcxl.h       |  1 +
> > >   cxl/region.c       |  6 ++++++
> > >   6 files changed, 28 insertions(+)
> > > 
> > > diff --git a/cxl/json.c b/cxl/json.c
> > > index a0dc343..3bbe07a 100644
> > > --- a/cxl/json.c
> > > +++ b/cxl/json.c
> > > @@ -1077,6 +1077,10 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
> > >   			json_object_object_add(jregion, "state", jobj);
> > >   	}
> > > +	jobj = json_object_new_boolean(cxl_region_is_locked(region));
> > > +	if (jobj)
> > > +		json_object_object_add(jregion, "locked", jobj);
> > > +
> > >   	if (flags & UTIL_JSON_MEDIA_ERRORS) {
> > >   		jobj = util_cxl_poison_list_to_json(region, NULL, flags);
> > >   		if (jobj)
> > > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > > index 5e8deb6..ce2b364 100644
> > > --- a/cxl/lib/libcxl.c
> > > +++ b/cxl/lib/libcxl.c
> > > @@ -472,6 +472,11 @@ CXL_EXPORT int cxl_region_is_enabled(struct cxl_region *region)
> > >   	return is_enabled(path);
> > >   }
> > > +CXL_EXPORT bool cxl_region_is_locked(struct cxl_region *region)
> > > +{
> > > +	return region->locked;
> > > +}
> > > +
> > >   CXL_EXPORT bool cxl_region_qos_class_mismatch(struct cxl_region *region)
> > >   {
> > >   	struct cxl_decoder *root_decoder = cxl_region_get_decoder(region);
> > > @@ -689,6 +694,12 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
> > >   	if (sysfs_read_attr(ctx, path, buf) == 0)
> > >   		region->module = util_modalias_to_module(ctx, buf);
> > > +	sprintf(path, "%s/locked", cxlregion_base);
> > > +	if (sysfs_read_attr(ctx, path, buf) < 0)
> > > +		region->locked = false;
> > > +	else
> > > +		region->locked = strtoul(buf, NULL, 0);
> > > +
> > >   	cxl_region_foreach_safe(decoder, region_dup, _r)
> > >   		if (region_dup->id == region->id) {
> > >   			list_del_from(&decoder->regions, &region_dup->list);
> > > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > > index 797e398..7c586f4 100644
> > > --- a/cxl/lib/libcxl.sym
> > > +++ b/cxl/lib/libcxl.sym
> > > @@ -315,3 +315,8 @@ global:
> > >   	cxl_memdev_clear_poison;
> > >   	cxl_debugfs_exists;
> > >   } LIBCXL_10;
> > > +
> > > +LIBCXL_12 {
> > > +global:
> > > +	cxl_region_is_locked;
> > > +} LIBCXL_11;
> > > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > > index 582eebf..d074122 100644
> > > --- a/cxl/lib/private.h
> > > +++ b/cxl/lib/private.h
> > > @@ -189,6 +189,7 @@ struct cxl_region {
> > >   	u64 start;
> > >   	u64 size;
> > >   	u64 cache_size;
> > > +	bool locked;
> > >   	unsigned int interleave_ways;
> > >   	unsigned int interleave_granularity;
> > >   	enum cxl_decode_state decode_state;
> > > diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> > > index a853ab6..6ff58de 100644
> > > --- a/cxl/libcxl.h
> > > +++ b/cxl/libcxl.h
> > > @@ -353,6 +353,7 @@ int cxl_region_clear_all_targets(struct cxl_region *region);
> > >   int cxl_region_decode_commit(struct cxl_region *region);
> > >   int cxl_region_decode_reset(struct cxl_region *region);
> > >   bool cxl_region_qos_class_mismatch(struct cxl_region *region);
> > > +bool cxl_region_is_locked(struct cxl_region *region);
> > >   #define cxl_region_foreach(decoder, region)                                    \
> > >   	for (region = cxl_region_get_first(decoder); region != NULL;           \
> > > diff --git a/cxl/region.c b/cxl/region.c
> > > index 207cf2d..8a85522 100644
> > > --- a/cxl/region.c
> > > +++ b/cxl/region.c
> > > @@ -837,6 +837,12 @@ static int destroy_region(struct cxl_region *region)
> > >   	unsigned int ways, i;
> > >   	int rc;
> > > +	if (cxl_region_is_locked(region)) {
> > > +		log_err(&rl, "%s: Cannot destroy a locked region.\n",
> > > +			devname);
> > > +		return -EPERM;
> > > +	}
> > > +
> > >   	/* First, unbind/disable the region if needed */
> > >   	if (cxl_region_is_enabled(region)) {
> > >   		if (param.force) {
> > > -- 
> > > 2.43.0
> > >