On Thu, 10 Nov 2022 19:20:04 -0800
[email protected] wrote:

> From: Alison Schofield <[email protected]>
> 
> CXL devices maintain a list of locations that are poisoned or result
> in poison if the addresses are accessed by the host.
> 
> Per the spec (CXL 3.0 8.2.9.8.4.1), the device returns this Poison
> list as a set of  Media Error Records that include the source of the
> error, the starting device physical address and length.
> 
> Trigger the retrieval of the poison list by writing to the device
> sysfs attribute: trigger_poison_list.
> 
> Retrieval is offered by memdev or by region:
> int cxl_memdev_trigger_poison_list(struct cxl_memdev *memdev);
> int cxl_region_trigger_poison_list(struct cxl_region *region);
> 
> This interface triggers the retrieval of the poison list from the
> devices and logs the error records as kernel trace events named
> 'cxl_poison'.
> 
> Signed-off-by: Alison Schofield <[email protected]>
Trivial comment inline + I haven't been tracking closely development
of this tool closely so hopefully this will get other eyes on it who
are more familiar.  With that in mind:

Reviewed-by: Jonathan Cameron <[email protected]>

> ---
>  cxl/lib/libcxl.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |  6 ++++++
>  cxl/libcxl.h       |  2 ++
>  3 files changed, 52 insertions(+)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index e8c5d4444dd0..1a8a8eb0ffcb 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1331,6 +1331,50 @@ CXL_EXPORT int cxl_memdev_disable_invalidate(struct 
> cxl_memdev *memdev)
>       return 0;
>  }
>  
> +CXL_EXPORT int cxl_memdev_trigger_poison_list(struct cxl_memdev *memdev)
> +{
> +     struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +     char *path = memdev->dev_buf;
> +     int len = memdev->buf_len, rc;
> +
> +     if (snprintf(path, len, "%s/trigger_poison_list", memdev->dev_path) >=
> +         len) {

Ugly line break choice to break mid argument..
        if (snprintf(path, len, "%s/trigger_poison_list",
                     memdev->dev_path) >= len) {
would be better.

> +             err(ctx, "%s: buffer too small\n",
> +                 cxl_memdev_get_devname(memdev));
> +             return -ENXIO;
> +     }
> +     rc = sysfs_write_attr(ctx, path, "1\n");
> +     if (rc < 0) {
> +             fprintf(stderr,
> +                     "%s: Failed write sysfs attr trigger_poison_list\n",
> +                     cxl_memdev_get_devname(memdev));
> +             return rc;
> +     }
> +     return 0;
> +}
> +
> +CXL_EXPORT int cxl_region_trigger_poison_list(struct cxl_region *region)
> +{
> +     struct cxl_ctx *ctx = cxl_region_get_ctx(region);
> +     char *path = region->dev_buf;
> +     int len = region->buf_len, rc;
> +
> +     if (snprintf(path, len, "%s/trigger_poison_list", region->dev_path) >=
> +         len) {
as above.

> +             err(ctx, "%s: buffer too small\n",
> +                 cxl_region_get_devname(region));
> +             return -ENXIO;
> +     }
> +     rc = sysfs_write_attr(ctx, path, "1\n");
> +     if (rc < 0) {
> +             fprintf(stderr,
> +                     "%s: Failed write sysfs attr trigger_poison_list\n",
> +                     cxl_region_get_devname(region));
> +             return rc;
> +     }
> +     return 0;
> +}
> +
>  CXL_EXPORT int cxl_memdev_enable(struct cxl_memdev *memdev)
>  {
>       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 8bb91e05638b..ecf98e6c7af2 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -217,3 +217,9 @@ global:
>       cxl_decoder_get_max_available_extent;
>       cxl_decoder_get_region;
>  } LIBCXL_2;
> +
> +LIBCXL_4 {
> +global:
> +     cxl_memdev_trigger_poison_list;
> +     cxl_region_trigger_poison_list;
> +} LIBCXL_3;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 9fe4e99263dd..5ebdf0879325 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -375,6 +375,8 @@ enum cxl_setpartition_mode {
>  
>  int cxl_cmd_partition_set_mode(struct cxl_cmd *cmd,
>               enum cxl_setpartition_mode mode);
> +int cxl_memdev_trigger_poison_list(struct cxl_memdev *memdev);
> +int cxl_region_trigger_poison_list(struct cxl_region *region);
>  
>  #ifdef __cplusplus
>  } /* extern "C" */


Reply via email to