Re: [PATCH 4/7] cxl: Add inject-error command

["Cheatham, Benjamin" <benjamin.cheatham@amd.com>]

On 1/9/2026 3:53 PM, Dave Jiang wrote:
> 
> 
> On 1/9/26 9:07 AM, Ben Cheatham wrote:
>> Add the 'cxl-inject-error' command. This command will provide CXL
>> protocol error injection for CXL VH root ports and CXL RCH downstream
>> ports, as well as poison injection for CXL memory devices.
>>
>> Add util_cxl_dport_filter() to find downstream ports by device name.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---

[snip]

>> +
>> +static int poison_action(struct cxl_ctx *ctx, const char *filter,
>> +			 const char *addr_str)
>> +{
>> +	struct cxl_memdev *memdev;
>> +	unsigned long long addr;
>> +	int rc;
>> +
>> +	memdev = find_cxl_memdev(ctx, filter);
>> +	if (!memdev)
>> +		return -ENODEV;
>> +
>> +	if (!cxl_memdev_has_poison_injection(memdev)) {
>> +		log_err(&iel, "%s does not support error injection\n",
>> +			cxl_memdev_get_devname(memdev));
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!addr_str) {
>> +		log_err(&iel, "no address provided\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	errno = 0;
> 
> Why does errno needs to be set here?

Alison suggested it last revision. It's been a while so my memory is a bit hazy,
but I think strtoull() doesn't reset errno and checking it below could cause a problem
if it was set to ERANGE by a previous function.

> 
>> +	addr = strtoull(addr_str, NULL, 0);
>> +	if (addr == ULLONG_MAX && errno == ERANGE) {
>> +		log_err(&iel, "invalid address %s", addr_str);
>> +		return -EINVAL;
>> +	}
>> +
>> +	rc = cxl_memdev_inject_poison(memdev, addr);
>> +	if (rc)
>> +		log_err(&iel, "failed to inject poison at %s:%s: %s\n",
>> +			cxl_memdev_get_devname(memdev), addr_str, strerror(-rc));
> 
> We don't error if poison fails to inject?

It does? The return code of cxl_memdev_inject_poison() is returned below, the only
thing that this if statement does is pick which message is emitted.

Thanks,
Ben

> 
> DJ
> 
>> +	else
>> +		log_info(&iel, "poison injected at %s:%s\n",
>> +			 cxl_memdev_get_devname(memdev), addr_str);
>> +
>> +	return rc;
>> +}
>> +
>> +static int inject_action(int argc, const char **argv, struct cxl_ctx *ctx,
>> +			 const struct option *options, const char *usage)
>> +{
>> +	struct cxl_protocol_error *perr;
>> +	const char * const u[] = {
>> +		usage,
>> +		NULL
>> +	};
>> +	int rc = -EINVAL;
>> +
>> +	log_init(&iel, "cxl inject-error", "CXL_INJECT_LOG");
>> +	argc = parse_options(argc, argv, options, u, 0);
>> +
>> +	if (debug) {
>> +		cxl_set_log_priority(ctx, LOG_DEBUG);
>> +		iel.log_priority = LOG_DEBUG;
>> +	} else {
>> +		iel.log_priority = LOG_INFO;
>> +	}
>> +
>> +	if (argc != 1 || inj_param.type == NULL) {
>> +		usage_with_options(u, options);
>> +		return rc;
>> +	}
>> +
>> +	if (strcmp(inj_param.type, "poison") == 0) {
>> +		rc = poison_action(ctx, argv[0], inj_param.address);
>> +		return rc;
>> +	}
>> +
>> +	perr = find_cxl_proto_err(ctx, inj_param.type);
>> +	if (perr) {
>> +		rc = inject_proto_err(ctx, argv[0], perr);
>> +		if (rc)
>> +			log_err(&iel, "Failed to inject error: %d\n", rc);
>> +	}
>> +
>> +	return rc;
>> +}
>> +
>> +int cmd_inject_error(int argc, const char **argv, struct cxl_ctx *ctx)
>> +{
>> +	int rc = inject_action(argc, argv, ctx, inject_options,
>> +			       "inject-error <device> -t <type> [<options>]");
>> +
>> +	return rc ? EXIT_FAILURE : EXIT_SUCCESS;
>> +}
>> diff --git a/cxl/meson.build b/cxl/meson.build
>> index b9924ae..92031b5 100644
>> --- a/cxl/meson.build
>> +++ b/cxl/meson.build
>> @@ -7,6 +7,7 @@ cxl_src = [
>>    'memdev.c',
>>    'json.c',
>>    'filter.c',
>> +  'inject-error.c',
>>    '../daxctl/json.c',
>>    '../daxctl/filter.c',
>>  ]
>