| Message ID | 20230220045709.94027-3-junhyeok.im@samsung.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support for inject poison | expand |
On Mon, Feb 20, 2023 at 01:57:08PM +0900, Junhyeok Im wrote: > Add new command to cli tool, to inject poison into dpa(-a) on the > memory device. > > DPA written in sysfs attribute(inject_poison) is converted by > kstrtou64 with 0 base by 'inject_poison_store' of CXL driver, so if > it begins with 0x the number will be parsed as a hexadecimal > (case insensitive), if it otherwise begins with 0, it will be parsed > as an octal number. Otherwise it will be parsed as a decimal. > > Since the validity verification of the dpa would be done in > 'cxl_validate_poison_dpa' of CXL driver, no additional logic > is added. I'm not sure that zero validity checks on that DPA is best here. That validity checks in the driver always returns -EINVAL, and the driver emits a dev_dbg message offering more details. So, for a user that is not running a debug kernel, figuring out which validity check they failed is not so trivial. The driver fails the validity check for these reasons: device has no dpa resource dpa in resource dpa is not 64-byte aligned dpa mapped in region I lean towards the cxl command checking all of these, but I'm not clear of the precedence here, so let's see what others say. > > Also since it is expected no use case of injecting poison into the > same address for multiple devices, this command targets only one > memdev, like write-labels command. > > usage: cxl inject-poison <memdev> -a <dpa> [<options>] maybe -m memdev (to be like others) > > -v, --verbose turn on debug > -S, --serial use serial numbers to id memdevs > -a, --address <dpa> DPA to inject poison Let's open up the naming discussion again. This isn't solely about your patch here. With the cxl list --media-errors patch, Jonathan and I had a discussion about whether that should be --poison or --media-errors and stuck with --media-errors to align with the CXL spec and be like ndctl. Our prior thoughts: https://lore.kernel.org/nvdimm/20221121105711.0000770c@Huawei.com/ Note that this is all pending work, nothing has been merged. I started using 'poison' for the sysfs attributes: trigger_poison_list, inject_poison, clear_posion. But, in the CXL tool, I went with 'media-errors', cxl list --media-errors. Following that pattern, s/inject-poison/inject-media-error I'd like to revisit that, because now it seems like less of a match as we grow to include inject_poison and clear_poison. > > Link to corresponding kernel patch: > https://patchwork.kernel.org/project/cxl/patch/97a0b128d0d0df56cea1a1a4ead65a40b9cf008e.1674101475.git.alison.schofield@intel.com/ > > Signed-off-by: Junhyeok Im <junhyeok.im@samsung.com> > --- > cxl/builtin.h | 1 + > cxl/cxl.c | 1 + > cxl/memdev.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++--- > 3 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/cxl/builtin.h b/cxl/builtin.h > index 34c5cfb..ddc4da9 100644 > --- a/cxl/builtin.h > +++ b/cxl/builtin.h > @@ -23,4 +23,5 @@ 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_monitor(int argc, const char **argv, struct cxl_ctx *ctx); > +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx); > #endif /* _CXL_BUILTIN_H_ */ > diff --git a/cxl/cxl.c b/cxl/cxl.c > index 3be7026..aa8d090 100644 > --- a/cxl/cxl.c > +++ b/cxl/cxl.c > @@ -77,6 +77,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-poison", .c_fn = cmd_inject_poison }, > }; > > int main(int argc, const char **argv) > diff --git a/cxl/memdev.c b/cxl/memdev.c > index 0b3ad02..7a10f79 100644 > --- a/cxl/memdev.c > +++ b/cxl/memdev.c > @@ -34,6 +34,7 @@ static struct parameters { > const char *type; > const char *size; > const char *decoder_filter; > + const char *poison_address; > } param; > > static struct log_ctx ml; > @@ -85,6 +86,10 @@ OPT_STRING('t', "type", ¶m.type, "type", \ > OPT_BOOLEAN('f', "force", ¶m.force, \ > "Attempt 'expected to fail' operations") > > +#define INJECT_POISON_OPTIONS() \ > +OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ > + "DPA to inject poison") > + git clang-format doesn't like the above. -#define INJECT_POISON_OPTIONS() \ -OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ - "DPA to inject poison") +#define INJECT_POISON_OPTIONS() \ + OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ + "DPA to inject poison") > static const struct option read_options[] = { > BASE_OPTIONS(), > LABEL_OPTIONS(), > @@ -135,6 +140,12 @@ static const struct option free_dpa_options[] = { > OPT_END(), > }; > > +static const struct option inject_poison_options[] = { > + BASE_OPTIONS(), > + INJECT_POISON_OPTIONS(), > + OPT_END(), > +}; > + > enum reserve_dpa_mode { > DPA_ALLOC, > DPA_FREE, > @@ -351,6 +362,24 @@ static int action_free_dpa(struct cxl_memdev *memdev, > return __reserve_dpa(memdev, DPA_FREE, actx); > } > > +static int action_inject_poison(struct cxl_memdev *memdev, > + struct action_context *actx) > +{ > + int rc; > + > + if (!param.poison_address) { > + log_err(&ml, "%s: set dpa to inject poison.\n", > + cxl_memdev_get_devname(memdev)); > + return -EINVAL; > + } > + rc = cxl_memdev_inject_poison(memdev, param.poison_address); > + if (rc < 0) { > + log_err(&ml, "%s: inject poison failed: %s\n", > + cxl_memdev_get_devname(memdev), strerror(-rc)); > + } > + return rc; > +} > + > static int action_disable(struct cxl_memdev *memdev, struct action_context *actx) > { > if (!cxl_memdev_is_enabled(memdev)) > @@ -755,7 +784,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, > continue; > found = true; > > - if (action == action_write) { > + if ((action == action_write) || > + (action == action_inject_poison)) { > single = memdev; > rc = 0; > } else > @@ -771,9 +801,15 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, > } > rc = err; > > - if (action == action_write) { > + if ((action == action_write) || (action == action_inject_poison)) { > if (count > 1) { > - error("write-labels only supports writing a single memdev\n"); > + if (action == action_write) { > + error("write-labels only supports writing " > + "a single memdev\n"); > + } else { > + error("inject-poison only supports injection " > + "of poison into a single memdev\n"); > + } > usage_with_options(u, options); > return -EINVAL; > } else if (single) { > @@ -893,3 +929,14 @@ int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx) > > return count >= 0 ? 0 : EXIT_FAILURE; > } > + > +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx) > +{ > + int count = memdev_action( > + argc, argv, ctx, action_inject_poison, inject_poison_options, > + "cxl inject-poison <memdev> -a <dpa> [<options>]"); > + log_info(&ml, "inject-poison %d mem%s\n", count >= 0 ? count : 0, > + count > 1 ? "s" : ""); > + > + return count >= 0 ? 0 : EXIT_FAILURE; > +} > -- > 2.34.1 >
On Mon, Feb 20, 2023 at 01:57:08PM +0900, Junhyeok Im wrote: > Add new command to cli tool, to inject poison into dpa(-a) on the > memory device. > > DPA written in sysfs attribute(inject_poison) is converted by > kstrtou64 with 0 base by 'inject_poison_store' of CXL driver, so if > it begins with 0x the number will be parsed as a hexadecimal > (case insensitive), if it otherwise begins with 0, it will be parsed > as an octal number. Otherwise it will be parsed as a decimal. > > Since the validity verification of the dpa would be done in > 'cxl_validate_poison_dpa' of CXL driver, no additional logic > is added. > > Also since it is expected no use case of injecting poison into the > same address for multiple devices, this command targets only one > memdev, like write-labels command. > > usage: cxl inject-poison <memdev> -a <dpa> [<options>] > > -v, --verbose turn on debug > -S, --serial use serial numbers to id memdevs > -a, --address <dpa> DPA to inject poison > Upon successful completion of the command, display the --media-errors list for that device. <snip>
On Sun, Feb 26, 2023 at 07:21:44PM -0800, Alison Schofield wrote: > On Mon, Feb 20, 2023 at 01:57:08PM +0900, Junhyeok Im wrote: > > Add new command to cli tool, to inject poison into dpa(-a) on the > > memory device. > > > > DPA written in sysfs attribute(inject_poison) is converted by > > kstrtou64 with 0 base by 'inject_poison_store' of CXL driver, so if > > it begins with 0x the number will be parsed as a hexadecimal > > (case insensitive), if it otherwise begins with 0, it will be parsed > > as an octal number. Otherwise it will be parsed as a decimal. > > > > Since the validity verification of the dpa would be done in > > 'cxl_validate_poison_dpa' of CXL driver, no additional logic > > is added. > > I'm not sure that zero validity checks on that DPA is best here. > That validity checks in the driver always returns -EINVAL, and > the driver emits a dev_dbg message offering more details. So, for > a user that is not running a debug kernel, figuring out which > validity check they failed is not so trivial. > > The driver fails the validity check for these reasons: > > device has no dpa resource > dpa in resource > dpa is not 64-byte aligned > dpa mapped in region > > I lean towards the cxl command checking all of these, but I'm not > clear of the precedence here, so let's see what others say. > Thank you for sharing your thoughts. Then I will also wait for others opinion for now. > > > > Also since it is expected no use case of injecting poison into the > > same address for multiple devices, this command targets only one > > memdev, like write-labels command. > > > > usage: cxl inject-poison <memdev> -a <dpa> [<options>] > > maybe -m memdev (to be like others) AFAIK, memdev is to be specified without "-m" except for 'list' cmd, but I think the reason you said is to distinguish memdev from region(e.g."-r"). (I also found in your previous patch for GET_POISON_LIST that memdev and region were separated.) If so, codes and documentation should be modified like: cxl inject-poison -m <memdev> -a <dpa> [<options>] Thanks. > > > > -v, --verbose turn on debug > > -S, --serial use serial numbers to id memdevs > > -a, --address <dpa> DPA to inject poison > > Let's open up the naming discussion again. This isn't solely about > your patch here. With the cxl list --media-errors patch, Jonathan and I > had a discussion about whether that should be --poison or --media-errors > and stuck with --media-errors to align with the CXL spec and be like ndctl. > > Our prior thoughts: > https://lore.kernel.org/nvdimm/20221121105711.0000770c@Huawei.com/ > > Note that this is all pending work, nothing has been merged. I started > using 'poison' for the sysfs attributes: trigger_poison_list, inject_poison, > clear_posion. > > But, in the CXL tool, I went with 'media-errors', cxl list --media-errors. > > Following that pattern, s/inject-poison/inject-media-error > > I'd like to revisit that, because now it seems like less of a match as > we grow to include inject_poison and clear_poison. I agree that naming should be changed and unified if needed. Personally I do not think it's awkward to use the name 'poison' seperately from general 'errors' to indicate that it's inserted artificially. But still a concern is that spec calls them "Media Error Records", like your previous comments above. > > > > > Link to corresponding kernel patch: > > https://patchwork.kernel.org/project/cxl/patch/97a0b128d0d0df56cea1a1a4ead65a40b9cf008e.1674101475.git.alison.schofield@intel.com/ > > > > Signed-off-by: Junhyeok Im <junhyeok.im@samsung.com> > > --- > > cxl/builtin.h | 1 + > > cxl/cxl.c | 1 + > > cxl/memdev.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++--- > > 3 files changed, 52 insertions(+), 3 deletions(-) > > > > diff --git a/cxl/builtin.h b/cxl/builtin.h > > index 34c5cfb..ddc4da9 100644 > > --- a/cxl/builtin.h > > +++ b/cxl/builtin.h > > @@ -23,4 +23,5 @@ 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_monitor(int argc, const char **argv, struct cxl_ctx *ctx); > > +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx); > > #endif /* _CXL_BUILTIN_H_ */ > > diff --git a/cxl/cxl.c b/cxl/cxl.c > > index 3be7026..aa8d090 100644 > > --- a/cxl/cxl.c > > +++ b/cxl/cxl.c > > @@ -77,6 +77,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-poison", .c_fn = cmd_inject_poison }, > > }; > > > > int main(int argc, const char **argv) > > diff --git a/cxl/memdev.c b/cxl/memdev.c > > index 0b3ad02..7a10f79 100644 > > --- a/cxl/memdev.c > > +++ b/cxl/memdev.c > > @@ -34,6 +34,7 @@ static struct parameters { > > const char *type; > > const char *size; > > const char *decoder_filter; > > + const char *poison_address; > > } param; > > > > static struct log_ctx ml; > > @@ -85,6 +86,10 @@ OPT_STRING('t', "type", ¶m.type, "type", \ > > OPT_BOOLEAN('f', "force", ¶m.force, \ > > "Attempt 'expected to fail' operations") > > > > +#define INJECT_POISON_OPTIONS() \ > > +OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ > > + "DPA to inject poison") > > + > > git clang-format doesn't like the above. > > -#define INJECT_POISON_OPTIONS() \ > -OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ > - "DPA to inject poison") > +#define INJECT_POISON_OPTIONS() \ > + OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ > + "DPA to inject poison") > > > Thank you for check and you're right, clang-format wants a different style of macro definition. Before defining them, I referred to the other macros in memdev.c (e.g. DPA_OPTIONS()) and found they were also not indented. Do you think that we have to modify them all? > > static const struct option read_options[] = { > > BASE_OPTIONS(), > > LABEL_OPTIONS(), > > @@ -135,6 +140,12 @@ static const struct option free_dpa_options[] = { > > OPT_END(), > > }; > > > > +static const struct option inject_poison_options[] = { > > + BASE_OPTIONS(), > > + INJECT_POISON_OPTIONS(), > > + OPT_END(), > > +}; > > + > > enum reserve_dpa_mode { > > DPA_ALLOC, > > DPA_FREE, > > @@ -351,6 +362,24 @@ static int action_free_dpa(struct cxl_memdev *memdev, > > return __reserve_dpa(memdev, DPA_FREE, actx); > > } > > > > +static int action_inject_poison(struct cxl_memdev *memdev, > > + struct action_context *actx) > > +{ > > + int rc; > > + > > + if (!param.poison_address) { > > + log_err(&ml, "%s: set dpa to inject poison.\n", > > + cxl_memdev_get_devname(memdev)); > > + return -EINVAL; > > + } > > + rc = cxl_memdev_inject_poison(memdev, param.poison_address); > > + if (rc < 0) { > > + log_err(&ml, "%s: inject poison failed: %s\n", > > + cxl_memdev_get_devname(memdev), strerror(-rc)); > > + } > > + return rc; > > +} > > + > > static int action_disable(struct cxl_memdev *memdev, struct action_context *actx) > > { > > if (!cxl_memdev_is_enabled(memdev)) > > @@ -755,7 +784,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, > > continue; > > found = true; > > > > - if (action == action_write) { > > + if ((action == action_write) || > > + (action == action_inject_poison)) { > > single = memdev; > > rc = 0; > > } else > > @@ -771,9 +801,15 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, > > } > > rc = err; > > > > - if (action == action_write) { > > + if ((action == action_write) || (action == action_inject_poison)) { > > if (count > 1) { > > - error("write-labels only supports writing a single memdev\n"); > > + if (action == action_write) { > > + error("write-labels only supports writing " > > + "a single memdev\n"); > > + } else { > > + error("inject-poison only supports injection " > > + "of poison into a single memdev\n"); > > + } > > usage_with_options(u, options); > > return -EINVAL; > > } else if (single) { > > @@ -893,3 +929,14 @@ int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx) > > > > return count >= 0 ? 0 : EXIT_FAILURE; > > } > > + > > +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx) > > +{ > > + int count = memdev_action( > > + argc, argv, ctx, action_inject_poison, inject_poison_options, > > + "cxl inject-poison <memdev> -a <dpa> [<options>]"); > > + log_info(&ml, "inject-poison %d mem%s\n", count >= 0 ? count : 0, > > + count > 1 ? "s" : ""); > > + > > + return count >= 0 ? 0 : EXIT_FAILURE; > > +} > > -- > > 2.34.1 > >
On Sun, Feb 26, 2023 at 07:25:50PM -0800, Alison Schofield wrote: > On Mon, Feb 20, 2023 at 01:57:08PM +0900, Junhyeok Im wrote: > > Add new command to cli tool, to inject poison into dpa(-a) on the > > memory device. > > > > DPA written in sysfs attribute(inject_poison) is converted by > > kstrtou64 with 0 base by 'inject_poison_store' of CXL driver, so if > > it begins with 0x the number will be parsed as a hexadecimal > > (case insensitive), if it otherwise begins with 0, it will be parsed > > as an octal number. Otherwise it will be parsed as a decimal. > > > > Since the validity verification of the dpa would be done in > > 'cxl_validate_poison_dpa' of CXL driver, no additional logic > > is added. > > > > Also since it is expected no use case of injecting poison into the > > same address for multiple devices, this command targets only one > > memdev, like write-labels command. > > > > usage: cxl inject-poison <memdev> -a <dpa> [<options>] > > > > -v, --verbose turn on debug > > -S, --serial use serial numbers to id memdevs > > -a, --address <dpa> DPA to inject poison > > > > Upon successful completion of the command, display the --media-errors > list for that device. > > <snip> > So, your comment is that the media-errors list (like $ cxl list --media-erros) should be printed after command completion. I agree in terms of the completeness of the command operation. Thank you for your good opinion. I'll make up for it.
On Tue, 2023-02-28 at 18:43 +0900, Junhyeok Im wrote: > On Sun, Feb 26, 2023 at 07:21:44PM -0800, Alison Schofield wrote: > > > > <..> > > > Also since it is expected no use case of injecting poison into the > > > same address for multiple devices, this command targets only one > > > memdev, like write-labels command. > > > > > > usage: cxl inject-poison <memdev> -a <dpa> [<options>] > > > > maybe -m memdev (to be like others) > > AFAIK, memdev is to be specified without "-m" except for 'list' cmd, > but I think the reason you said is to distinguish memdev from region(e.g."-r"). > (I also found in your previous patch for GET_POISON_LIST that > memdev and region were separated.) > If so, codes and documentation should be modified like: > cxl inject-poison -m <memdev> -a <dpa> [<options>] Just wanted to chime in on this quickly before having gone through the rest of the series - The convention has been if a command can only have one type of 'target', then no need for a -m, or -r etc. e.g. cxl enable-region region0 But if it can (potentially) have different types of targets, then an option can be used to determine the target type. e.g. cxl-create-region was originally intending to allow both memdev and endpoint targets, hence the -m option to say that the non-option arguments should be treated as memdevs. The other place where you'd have options like -m="mem0,mem1" etc is for filtering or restricting an operation. cxl-list uses the different options to filter results, but also other commands: e.g. cxl disable-region --bus=cxl_test all will disable all regions under cxl_test, but leave alone regions under another bus.
diff --git a/cxl/builtin.h b/cxl/builtin.h index 34c5cfb..ddc4da9 100644 --- a/cxl/builtin.h +++ b/cxl/builtin.h @@ -23,4 +23,5 @@ 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_monitor(int argc, const char **argv, struct cxl_ctx *ctx); +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx); #endif /* _CXL_BUILTIN_H_ */ diff --git a/cxl/cxl.c b/cxl/cxl.c index 3be7026..aa8d090 100644 --- a/cxl/cxl.c +++ b/cxl/cxl.c @@ -77,6 +77,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-poison", .c_fn = cmd_inject_poison }, }; int main(int argc, const char **argv) diff --git a/cxl/memdev.c b/cxl/memdev.c index 0b3ad02..7a10f79 100644 --- a/cxl/memdev.c +++ b/cxl/memdev.c @@ -34,6 +34,7 @@ static struct parameters { const char *type; const char *size; const char *decoder_filter; + const char *poison_address; } param; static struct log_ctx ml; @@ -85,6 +86,10 @@ OPT_STRING('t', "type", ¶m.type, "type", \ OPT_BOOLEAN('f', "force", ¶m.force, \ "Attempt 'expected to fail' operations") +#define INJECT_POISON_OPTIONS() \ +OPT_STRING('a', "address", ¶m.poison_address, "dpa", \ + "DPA to inject poison") + static const struct option read_options[] = { BASE_OPTIONS(), LABEL_OPTIONS(), @@ -135,6 +140,12 @@ static const struct option free_dpa_options[] = { OPT_END(), }; +static const struct option inject_poison_options[] = { + BASE_OPTIONS(), + INJECT_POISON_OPTIONS(), + OPT_END(), +}; + enum reserve_dpa_mode { DPA_ALLOC, DPA_FREE, @@ -351,6 +362,24 @@ static int action_free_dpa(struct cxl_memdev *memdev, return __reserve_dpa(memdev, DPA_FREE, actx); } +static int action_inject_poison(struct cxl_memdev *memdev, + struct action_context *actx) +{ + int rc; + + if (!param.poison_address) { + log_err(&ml, "%s: set dpa to inject poison.\n", + cxl_memdev_get_devname(memdev)); + return -EINVAL; + } + rc = cxl_memdev_inject_poison(memdev, param.poison_address); + if (rc < 0) { + log_err(&ml, "%s: inject poison failed: %s\n", + cxl_memdev_get_devname(memdev), strerror(-rc)); + } + return rc; +} + static int action_disable(struct cxl_memdev *memdev, struct action_context *actx) { if (!cxl_memdev_is_enabled(memdev)) @@ -755,7 +784,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, continue; found = true; - if (action == action_write) { + if ((action == action_write) || + (action == action_inject_poison)) { single = memdev; rc = 0; } else @@ -771,9 +801,15 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx, } rc = err; - if (action == action_write) { + if ((action == action_write) || (action == action_inject_poison)) { if (count > 1) { - error("write-labels only supports writing a single memdev\n"); + if (action == action_write) { + error("write-labels only supports writing " + "a single memdev\n"); + } else { + error("inject-poison only supports injection " + "of poison into a single memdev\n"); + } usage_with_options(u, options); return -EINVAL; } else if (single) { @@ -893,3 +929,14 @@ int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx) return count >= 0 ? 0 : EXIT_FAILURE; } + +int cmd_inject_poison(int argc, const char **argv, struct cxl_ctx *ctx) +{ + int count = memdev_action( + argc, argv, ctx, action_inject_poison, inject_poison_options, + "cxl inject-poison <memdev> -a <dpa> [<options>]"); + log_info(&ml, "inject-poison %d mem%s\n", count >= 0 ? count : 0, + count > 1 ? "s" : ""); + + return count >= 0 ? 0 : EXIT_FAILURE; +}
Add new command to cli tool, to inject poison into dpa(-a) on the memory device. DPA written in sysfs attribute(inject_poison) is converted by kstrtou64 with 0 base by 'inject_poison_store' of CXL driver, so if it begins with 0x the number will be parsed as a hexadecimal (case insensitive), if it otherwise begins with 0, it will be parsed as an octal number. Otherwise it will be parsed as a decimal. Since the validity verification of the dpa would be done in 'cxl_validate_poison_dpa' of CXL driver, no additional logic is added. Also since it is expected no use case of injecting poison into the same address for multiple devices, this command targets only one memdev, like write-labels command. usage: cxl inject-poison <memdev> -a <dpa> [<options>] -v, --verbose turn on debug -S, --serial use serial numbers to id memdevs -a, --address <dpa> DPA to inject poison Link to corresponding kernel patch: https://patchwork.kernel.org/project/cxl/patch/97a0b128d0d0df56cea1a1a4ead65a40b9cf008e.1674101475.git.alison.schofield@intel.com/ Signed-off-by: Junhyeok Im <junhyeok.im@samsung.com> --- cxl/builtin.h | 1 + cxl/cxl.c | 1 + cxl/memdev.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 3 deletions(-)