On 10/23/25 1:15 PM, Cheatham, Benjamin wrote:
> On 10/22/2025 12:06 PM, Dave Jiang wrote:
>>
>>
>> On 10/21/25 11:31 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.
>>>
>>> Signed-off-by: Ben Cheatham <[email protected]>
>>> ---
>>> cxl/builtin.h | 1 +
>>> cxl/cxl.c | 1 +
>>> cxl/inject-error.c | 195 +++++++++++++++++++++++++++++++++++++++++++++
>>> cxl/meson.build | 1 +
>>> 4 files changed, 198 insertions(+)
>>> create mode 100644 cxl/inject-error.c
>>>
>>> diff --git a/cxl/builtin.h b/cxl/builtin.h
>>> index c483f30..e82fcb5 100644
>>> --- a/cxl/builtin.h
>>> +++ b/cxl/builtin.h
>>> @@ -25,6 +25,7 @@ int cmd_create_region(int argc, const char **argv, struct
>>> cxl_ctx *ctx);
>>> int cmd_enable_region(int argc, const char **argv, struct cxl_ctx *ctx);
>>> int cmd_disable_region(int argc, const char **argv, struct cxl_ctx *ctx);
>>> int cmd_destroy_region(int argc, const char **argv, struct cxl_ctx *ctx);
>>> +int cmd_inject_error(int argc, const char **argv, struct cxl_ctx *ctx);
>>> #ifdef ENABLE_LIBTRACEFS
>>> int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx);
>>> #else
>>> diff --git a/cxl/cxl.c b/cxl/cxl.c
>>> index 1643667..a98bd6b 100644
>>> --- a/cxl/cxl.c
>>> +++ b/cxl/cxl.c
>>> @@ -80,6 +80,7 @@ static struct cmd_struct commands[] = {
>>> { "disable-region", .c_fn = cmd_disable_region },
>>> { "destroy-region", .c_fn = cmd_destroy_region },
>>> { "monitor", .c_fn = cmd_monitor },
>>> + { "inject-error", .c_fn = cmd_inject_error },
>>> };
>>>
>>> int main(int argc, const char **argv)
>>> diff --git a/cxl/inject-error.c b/cxl/inject-error.c
>>> new file mode 100644
>>> index 0000000..c48ea69
>>> --- /dev/null
>>> +++ b/cxl/inject-error.c
>>> @@ -0,0 +1,195 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Copyright (C) 2025 AMD. All rights reserved. */
>>> +#include <util/parse-options.h>
>>> +#include <cxl/libcxl.h>
>>> +#include <cxl/filter.h>
>>> +#include <util/log.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +#include <stdio.h>
>>> +#include <errno.h>
>>> +#include <limits.h>
>>> +
>>> +#define EINJ_TYPES_BUF_SIZE 512
>>> +
>>> +static bool debug;
>>> +
>>> +static struct inject_params {
>>> + const char *type;
>>> + const char *address;
>>> +} inj_param;
>>> +
>>> +static const struct option inject_options[] = {
>>> + OPT_STRING('t', "type", &inj_param.type, "Error type",
>>> + "Error type to inject into <device>"),
>>> + OPT_STRING('a', "address", &inj_param.address, "Address for poison
>>> injection",
>>> + "Device physical address for poison injection in hex or
>>> decimal"),
>>> +#ifdef ENABLE_DEBUG
>>> + OPT_BOOLEAN(0, "debug", &debug, "turn on debug output"),
>>> +#endif
>>> + OPT_END(),
>>> +};
>>> +
>>> +static struct log_ctx iel;
>>> +
>>> +static struct cxl_protocol_error *find_cxl_proto_err(struct cxl_ctx *ctx,
>>> + const char *type)
>>> +{
>>> + struct cxl_protocol_error *perror;
>>> +
>>> + cxl_protocol_error_foreach(ctx, perror) {
>>> + if (strcmp(type, cxl_protocol_error_get_str(perror)) == 0)
>>> + return perror;
>>> + }
>>> +
>>> + log_err(&iel, "Invalid CXL protocol error type: %s\n", type);
>>> + return NULL;
>>> +}
>>> +
>>> +static struct cxl_dport *find_cxl_dport(struct cxl_ctx *ctx, const char
>>> *devname)
>>> +{
>>> + struct cxl_port *port, *top;
>>> + struct cxl_dport *dport;
>>> + struct cxl_bus *bus;
>>> +
>>> + cxl_bus_foreach(ctx, bus) {
>>> + top = cxl_bus_get_port(bus);
>>> +
>>> + cxl_port_foreach_all(top, port)
>>> + cxl_dport_foreach(port, dport)
>>> + if (!strcmp(devname,
>>> + cxl_dport_get_devname(dport)))
>>> + return dport;
>>
>> Would it be worthwhile to create a util_cxl_dport_filter()?
>>
>
> Yeah probably. I'll make one for the next revision.
>
>>> + }
>>> +
>>> + log_err(&iel, "Downstream port \"%s\" not found\n", devname);
>>> + return NULL;
>>> +}
>>> +
>>> +static struct cxl_memdev *find_cxl_memdev(struct cxl_ctx *ctx,
>>> + const char *filter)
>>> +{
>>> + struct cxl_memdev *memdev;
>>> +
>>> + cxl_memdev_foreach(ctx, memdev) {
>>> + if (util_cxl_memdev_filter(memdev, filter, NULL))
>>> + return memdev;
>>> + }
>>> +
>>> + log_err(&iel, "Memdev \"%s\" not found\n", filter);
>>> + return NULL;
>>> +}
>>> +
>>> +static int inject_proto_err(struct cxl_ctx *ctx, const char *devname,
>>> + struct cxl_protocol_error *perror)
>>> +{
>>> + struct cxl_dport *dport;
>>> + int rc;
>>> +
>>> + if (!devname) {
>>> + log_err(&iel, "No downstream port specified for injection\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + dport = find_cxl_dport(ctx, devname);
>>> + if (!dport)
>>> + return -ENODEV;
>>> +
>>> + rc = cxl_dport_protocol_error_inject(dport,
>>> +
>>> cxl_protocol_error_get_num(perror));
>>> + if (rc)
>>> + return rc;
>>> +
>>> + printf("injected %s protocol error.\n",
>>> + cxl_protocol_error_get_str(perror));
>>
>> log_info() maybe?
>
> I think I had it as log_info() before, but I don't think it was making it's
> way to
> the console. I think I wanted the console output because I personally don't
> like running
> silent commands. Not a great reason, so I'm fine with changing it if that's
> the preferred
> way.
>
Alison,
Do you have a preference?
DJ
>>
>>> + return 0;
>>> +}
>>> +
>>> +static int poison_action(struct cxl_ctx *ctx, const char *filter,
>>> + const char *addr)
>>> +{
>>> + struct cxl_memdev *memdev;
>>> + size_t a;
>>
>> Maybe rename 'addr' to 'addr_str' and rename 'a' to 'addr'
>>
>
> Sure.
>
>>> + 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) {
>>> + log_err(&iel, "no address provided\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + a = strtoull(addr, NULL, 0);
>>> + if (a == ULLONG_MAX && errno == ERANGE) {
>>> + log_err(&iel, "invalid address %s", addr);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + rc = cxl_memdev_inject_poison(memdev, a);
>>> +
>>
>> unnecessary blank line> + if (rc)
>
> Will remove!
>
>>> + log_err(&iel, "failed to inject poison at %s:%s: %s\n",
>>> + cxl_memdev_get_devname(memdev), addr, strerror(-rc));
>>> + else
>>> + printf("poison injected at %s:%s\n",
>>> + cxl_memdev_get_devname(memdev), addr);
>>
>> log_info() maybe?
>
> Same thing as above.
>
> Thanks,
> Ben
>
>>
>> DJ
>>
