On Mon, Dec 15, 2025 at 03:36:27PM -0600, 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 either dport id
> or device name.

Does above comment match code? Does util_cxl_dport_filter() match by
'id' or 'name' ?


snip

> +#define EINJ_TYPES_BUF_SIZE 512

above appears unused


snip

> +static int poison_action(struct cxl_ctx *ctx, const char *filter,
> +                      const char *addr_str)
> +{
> +     struct cxl_memdev *memdev;
> +     size_t 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;
> +     }
> +
> +     addr = strtoull(addr_str, NULL, 0);
> +     if (addr == ULLONG_MAX && errno == ERANGE) {
> +             log_err(&iel, "invalid address %s", addr_str);
> +             return -EINVAL;
> +     }

errno best set to 0 before strtoull
there is a type mismatch btw addr of size_t and strtoull

snip

> +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) {
> +             usage_with_options(u, options);
> +             return rc;
> +     }

The above catches bad syntax like this where I omit type:
# cxl inject-error mem10 -t -a 0x0

We also need to catch this where I omit the option altogether:
# cxl inject-error mem10  -a 0x0
Segmentation fault (core dumped)

> +
> +     if (strcmp(inj_param.type, "poison") == 0) {
> +             rc = poison_action(ctx, argv[0], inj_param.address);
> +             return rc;
> +     }

snip


Reply via email to