Re: [ndctl PATCH v3 2/7] libcxl: Add CXL protocol errors

[Dave Jiang <dave.jiang@intel.com>]

On 10/21/25 11:31 AM, Ben Cheatham wrote:
> The v6.11 Linux kernel adds CXL protocl (CXL.cache & CXL.mem) error
> injection for platforms that implement the error types as according to
> the v6.5+ ACPI specification. The interface for injecting these errors
> are provided by the kernel under the CXL debugfs. The relevant files in
> the interface are the einj_types file, which provides the available CXL
> error types for injection, and the einj_inject file, which injects the
> error into a CXL VH root port or CXL RCH downstream port.
> 
> Add a library API to retrieve the CXL error types and inject them. This
> API will be used in a later commit by the 'cxl-inject-error' and
> 'cxl-list' commands.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> ---
>  cxl/lib/libcxl.c   | 174 +++++++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |   5 ++
>  cxl/lib/private.h  |  14 ++++
>  cxl/libcxl.h       |  13 ++++
>  4 files changed, 206 insertions(+)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index ea5831f..9486b0f 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -46,11 +46,13 @@ struct cxl_ctx {
>  	void *userdata;
>  	int memdevs_init;
>  	int buses_init;
> +	int perrors_init;
>  	unsigned long timeout;
>  	struct udev *udev;
>  	struct udev_queue *udev_queue;
>  	struct list_head memdevs;
>  	struct list_head buses;
> +	struct list_head perrors;
>  	struct kmod_ctx *kmod_ctx;
>  	struct daxctl_ctx *daxctl_ctx;
>  	void *private_data;
> @@ -205,6 +207,14 @@ static void free_bus(struct cxl_bus *bus, struct list_head *head)
>  	free(bus);
>  }
>  
> +static void free_protocol_error(struct cxl_protocol_error *perror,
> +				struct list_head *head)
> +{
> +	if (head)
> +		list_del_from(head, &perror->list);

I would go if (!head) return;

> +	free(perror);
> +}
> +
>  /**
>   * cxl_get_userdata - retrieve stored data pointer from library context
>   * @ctx: cxl library context
> @@ -328,6 +338,7 @@ CXL_EXPORT int cxl_new(struct cxl_ctx **ctx)
>  	*ctx = c;
>  	list_head_init(&c->memdevs);
>  	list_head_init(&c->buses);
> +	list_head_init(&c->perrors);
>  	c->kmod_ctx = kmod_ctx;
>  	c->daxctl_ctx = daxctl_ctx;
>  	c->udev = udev;
> @@ -369,6 +380,7 @@ CXL_EXPORT struct cxl_ctx *cxl_ref(struct cxl_ctx *ctx)
>   */
>  CXL_EXPORT void cxl_unref(struct cxl_ctx *ctx)
>  {
> +	struct cxl_protocol_error *perror, *_p;
>  	struct cxl_memdev *memdev, *_d;
>  	struct cxl_bus *bus, *_b;
>  
> @@ -384,6 +396,9 @@ CXL_EXPORT void cxl_unref(struct cxl_ctx *ctx)
>  	list_for_each_safe(&ctx->buses, bus, _b, port.list)
>  		free_bus(bus, &ctx->buses);
>  
> +	list_for_each_safe(&ctx->perrors, perror, _p, list)
> +		free_protocol_error(perror, &ctx->perrors);
> +
>  	udev_queue_unref(ctx->udev_queue);
>  	udev_unref(ctx->udev);
>  	kmod_unref(ctx->kmod_ctx);
> @@ -3416,6 +3431,165 @@ CXL_EXPORT int cxl_port_decoders_committed(struct cxl_port *port)
>  	return port->decoders_committed;
>  }
>  
> +const struct cxl_protocol_error cxl_protocol_errors[] = {
> +	CXL_PROTOCOL_ERROR(12, "cache-correctable"),
> +	CXL_PROTOCOL_ERROR(13, "cache-uncorrectable"),
> +	CXL_PROTOCOL_ERROR(14, "cache-fatal"),
> +	CXL_PROTOCOL_ERROR(15, "mem-correctable"),
> +	CXL_PROTOCOL_ERROR(16, "mem-uncorrectable"),
> +	CXL_PROTOCOL_ERROR(17, "mem-fatal")
> +};
> +
> +static struct cxl_protocol_error *create_cxl_protocol_error(struct cxl_ctx *ctx,
> +							    unsigned long n)

why unsigned long instead of int? are there that many errors?

> +{
> +	struct cxl_protocol_error *perror;
> +
> +	for (unsigned long i = 0; i < ARRAY_SIZE(cxl_protocol_errors); i++) {
> +		if (n != BIT(cxl_protocol_errors[i].num))
> +			continue;
> +
> +		perror = calloc(1, sizeof(*perror));
> +		if (!perror)
> +			return NULL;
> +
> +		*perror = cxl_protocol_errors[i];
> +		perror->ctx = ctx;
> +		return perror;
> +	}
> +
> +	return NULL;
> +}
> +
> +static void cxl_add_protocol_errors(struct cxl_ctx *ctx)
> +{
> +	struct cxl_protocol_error *perror;
> +	char *path, *num, *save;
> +	unsigned long n;
> +	size_t path_len;
> +	char buf[512];

Use SYSFS_ATTR_SIZE rather than 512

> +	int rc = 0;
> +
> +	if (!ctx->debugfs)
> +		return;
> +
> +	path_len = strlen(ctx->debugfs) + 100;
> +	path = calloc(1, path_len);
> +	if (!path)
> +		return;
> +
> +	snprintf(path, path_len, "%s/cxl/einj_types", ctx->debugfs);
> +	rc = access(path, F_OK);
> +	if (rc) {
> +		err(ctx, "failed to access %s: %s\n", path, strerror(-rc));
strerror(errno)? access() returns -1 and the actual error is in errno.
> +		goto err;
> +	}
> +
> +	rc = sysfs_read_attr(ctx, path, buf);
> +	if (rc) {
> +		err(ctx, "failed to read %s: %s\n", path, strerror(-rc));
> +		goto err;
> +	}
> +
> +	/*
> +	 * The format of the output of the einj_types attr is:
> +	 * <Error number in hex 1> <Error name 1>
> +	 * <Error number in hex 2> <Error name 2>
> +	 * ...
> +	 *
> +	 * We only need the number, so parse that and skip the rest of
> +	 * the line.
> +	 */
> +	num = strtok_r(buf, " \n", &save);
> +	while (num) {
> +		n = strtoul(num, NULL, 16);
> +		perror = create_cxl_protocol_error(ctx, n);
> +		if (perror)
> +			list_add(&ctx->perrors, &perror->list);
> +
> +		num = strtok_r(NULL, "\n", &save);
> +		if (!num)
> +			break;
> +
> +		num = strtok_r(NULL, " \n", &save);
> +	}
> +
> +err:
> +	free(path);
> +}
> +
> +static void cxl_protocol_errors_init(struct cxl_ctx *ctx)
> +{
> +	if (ctx->perrors_init)
> +		return;
> +
> +	ctx->perrors_init = 1;
> +	cxl_add_protocol_errors(ctx);
> +}
> +
> +CXL_EXPORT struct cxl_protocol_error *
> +cxl_protocol_error_get_first(struct cxl_ctx *ctx)
> +{
> +	cxl_protocol_errors_init(ctx);
> +
> +	return list_top(&ctx->perrors, struct cxl_protocol_error, list);
> +}
> +
> +CXL_EXPORT struct cxl_protocol_error *
> +cxl_protocol_error_get_next(struct cxl_protocol_error *perror)
> +{
> +	struct cxl_ctx *ctx = perror->ctx;
> +
> +	return list_next(&ctx->perrors, perror, list);
> +}
> +
> +CXL_EXPORT unsigned long
> +cxl_protocol_error_get_num(struct cxl_protocol_error *perror)
> +{
> +	return perror->num;
> +}
> +
> +CXL_EXPORT const char *
> +cxl_protocol_error_get_str(struct cxl_protocol_error *perror)
> +{
> +	return perror->string;
> +}
> +
> +CXL_EXPORT int cxl_dport_protocol_error_inject(struct cxl_dport *dport,
> +					       unsigned long error)
> +{
> +	struct cxl_ctx *ctx = dport->port->ctx;
> +	unsigned long path_len;
> +	char buf[32] = { 0 };
> +	char *path;
> +	int rc;
> +
> +	if (!ctx->debugfs)
> +		return -ENOENT;
> +
> +	path_len = strlen(ctx->debugfs) + 100;
> +	path = calloc(path_len, sizeof(char));
> +	if (!path)
> +		return -ENOMEM;
> +
> +	snprintf(path, path_len, "%s/cxl/%s/einj_inject", ctx->debugfs,
> +		 cxl_dport_get_devname(dport));

check return value

> +	rc = access(path, F_OK);
> +	if (rc) {
> +		err(ctx, "failed to access %s: %s\n", path, strerror(-rc));

errno

> +		free(path);
> +		return rc;
-errno instead of rc

> +	}
> +
> +	snprintf(buf, sizeof(buf), "0x%lx\n", error);

check return value?

DJ

> +	rc = sysfs_write_attr(ctx, path, buf);
> +	if (rc)
> +		err(ctx, "failed to write %s: %s\n", path, strerror(-rc));
> +
> +	free(path);
> +	return rc;
> +}
> +
>  static void *add_cxl_bus(void *parent, int id, const char *cxlbus_base)
>  {
>  	const char *devname = devpath_to_devname(cxlbus_base);
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index e01a676..02d5119 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -299,4 +299,9 @@ global:
>  LIBCXL_10 {
>  global:
>  	cxl_memdev_is_port_ancestor;
> +	cxl_protocol_error_get_first;
> +	cxl_protocol_error_get_next;
> +	cxl_protocol_error_get_num;
> +	cxl_protocol_error_get_str;
> +	cxl_dport_protocol_error_inject;
>  } LIBCXL_9;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index 7d5a1bc..4e881b6 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -108,6 +108,20 @@ struct cxl_port {
>  	struct list_head dports;
>  };
>  
> +struct cxl_protocol_error {
> +	unsigned long num;
> +	const char *string;
> +	struct cxl_ctx *ctx;
> +	struct list_node list;
> +};
> +
> +#define CXL_PROTOCOL_ERROR(n, str)	\
> +	((struct cxl_protocol_error){	\
> +		.num = (n),		\
> +		.string = (str),	\
> +		.ctx = NULL,		\
> +	})
> +
>  struct cxl_bus {
>  	struct cxl_port port;
>  };
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 54bc025..9026e05 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -496,6 +496,19 @@ int cxl_cmd_alert_config_set_enable_alert_actions(struct cxl_cmd *cmd,
>  						  int enable);
>  struct cxl_cmd *cxl_cmd_new_set_alert_config(struct cxl_memdev *memdev);
>  
> +struct cxl_protocol_error;
> +struct cxl_protocol_error *cxl_protocol_error_get_first(struct cxl_ctx *ctx);
> +struct cxl_protocol_error *
> +cxl_protocol_error_get_next(struct cxl_protocol_error *perror);
> +unsigned long cxl_protocol_error_get_num(struct cxl_protocol_error *perror);
> +const char *cxl_protocol_error_get_str(struct cxl_protocol_error *perror);
> +int cxl_dport_protocol_error_inject(struct cxl_dport *dport,
> +				    unsigned long error);
> +
> +#define cxl_protocol_error_foreach(ctx, perror)				       \
> +	for (perror = cxl_protocol_error_get_first(ctx); perror != NULL;       \
> +	     perror = cxl_protocol_error_get_next(perror))
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif