[v3,1/6] trace, cxl: Introduce a TRACE_EVENT for CXL poison records
Commit Message
From: Alison Schofield <alison.schofield@intel.com>
CXL devices may support the retrieval of a device poison list.
Introduce a trace event that the CXL subsystem can use to log
the poison error records.
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
drivers/cxl/cxlmem.h | 14 +++++++
include/trace/events/cxl.h | 80 ++++++++++++++++++++++++++++++++++++++
2 files changed, 94 insertions(+)
create mode 100644 include/trace/events/cxl.h
Comments
On Thu, 10 Nov 2022 19:12:39 -0800
alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> CXL devices may support the retrieval of a device poison list.
> Introduce a trace event that the CXL subsystem can use to log
> the poison error records.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Only thing I wondered a bit about in here is the philosophy of whether it is
useful to include the pcidev and region given they can both be established
fairly easily via other paths.. Meh. Seems reasonable to me to have the
in here so.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/cxl/cxlmem.h | 14 +++++++
> include/trace/events/cxl.h | 80 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
> create mode 100644 include/trace/events/cxl.h
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..669868cc1553 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -347,6 +347,20 @@ struct cxl_mbox_set_partition_info {
>
> #define CXL_SET_PARTITION_IMMEDIATE_FLAG BIT(0)
>
> +/* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
> +
> +/* Get Poison List: Payload out flags */
> +#define CXL_POISON_FLAG_MORE BIT(0)
> +#define CXL_POISON_FLAG_OVERFLOW BIT(1)
> +#define CXL_POISON_FLAG_SCANNING BIT(2)
> +
> +/* Get Poison List: Poison Source */
> +#define CXL_POISON_SOURCE_UNKNOWN 0
> +#define CXL_POISON_SOURCE_EXTERNAL 1
> +#define CXL_POISON_SOURCE_INTERNAL 2
> +#define CXL_POISON_SOURCE_INJECTED 3
> +#define CXL_POISON_SOURCE_VENDOR 7
> +
> /**
> * struct cxl_mem_command - Driver representation of a memory device command
> * @info: Command information as it exists for the UAPI
> diff --git a/include/trace/events/cxl.h b/include/trace/events/cxl.h
> new file mode 100644
> index 000000000000..03428125573f
> --- /dev/null
> +++ b/include/trace/events/cxl.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM cxl
> +
> +#if !defined(_CXL_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _CXL_TRACE_H
> +
> +#include <linux/tracepoint.h>
> +#include <cxlmem.h>
> +
> +#define __show_poison_source(source) \
> + __print_symbolic(source, \
> + { CXL_POISON_SOURCE_UNKNOWN, "Unknown" }, \
> + { CXL_POISON_SOURCE_EXTERNAL, "External" }, \
> + { CXL_POISON_SOURCE_INTERNAL, "Internal" }, \
> + { CXL_POISON_SOURCE_INJECTED, "Injected" }, \
> + { CXL_POISON_SOURCE_VENDOR, "Vendor" })
> +
> +#define show_poison_source(source) \
> + (((source > CXL_POISON_SOURCE_INJECTED) && \
> + (source != CXL_POISON_SOURCE_VENDOR)) ? "Reserved" \
> + : __show_poison_source(source))
> +
> +#define show_poison_flags(flags) \
> + __print_flags(flags, "|", \
> + { CXL_POISON_FLAG_MORE, "More" }, \
> + { CXL_POISON_FLAG_OVERFLOW, "Overflow" }, \
> + { CXL_POISON_FLAG_SCANNING, "Scanning" })
> +
> +TRACE_EVENT(cxl_poison,
> +
> + TP_PROTO(const char *memdev, const char *pcidev, const char *region,
> + const uuid_t *uuid, u64 dpa, u32 length, u8 source,
> + u8 flags, u64 overflow_t),
> +
> + TP_ARGS(memdev, pcidev, region, uuid, dpa, length, source,
> + flags, overflow_t),
> +
> + TP_STRUCT__entry(
> + __string(memdev, memdev)
> + __string(pcidev, pcidev)
> + __string(region, region ? region : "")
> + __array(char, uuid, 16)
> + __field(u64, dpa)
> + __field(u32, length)
> + __field(u8, source)
> + __field(u8, flags)
> + __field(u64, overflow_t)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(memdev, memdev);
> + __assign_str(pcidev, pcidev);
> + __assign_str(region, region ? region : "");
> + if (uuid)
> + memcpy(__entry->uuid, uuid, 16);
> + __entry->dpa = dpa;
> + __entry->length = length;
> + __entry->source = source;
> + __entry->flags = flags;
> + __entry->overflow_t = overflow_t;
> + ),
> +
> + TP_printk("memdev=%s pcidev=%s region=%s region_uuid=%pU dpa=0x%llx length=0x%x source=%s flags=%s overflow_time=%llu",
> + __get_str(memdev),
> + __get_str(pcidev),
> + __get_str(region),
> + __entry->uuid,
> + __entry->dpa,
> + __entry->length,
> + show_poison_source(__entry->source),
> + show_poison_flags(__entry->flags),
> + __entry->overflow_t)
> +);
> +#endif /* _CXL_TRACE_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE cxl
> +#include <trace/define_trace.h>
alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> CXL devices may support the retrieval of a device poison list.
> Introduce a trace event that the CXL subsystem can use to log
> the poison error records.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> drivers/cxl/cxlmem.h | 14 +++++++
> include/trace/events/cxl.h | 80 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
> create mode 100644 include/trace/events/cxl.h
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..669868cc1553 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -347,6 +347,20 @@ struct cxl_mbox_set_partition_info {
>
> #define CXL_SET_PARTITION_IMMEDIATE_FLAG BIT(0)
>
> +/* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
> +
> +/* Get Poison List: Payload out flags */
> +#define CXL_POISON_FLAG_MORE BIT(0)
> +#define CXL_POISON_FLAG_OVERFLOW BIT(1)
> +#define CXL_POISON_FLAG_SCANNING BIT(2)
> +
> +/* Get Poison List: Poison Source */
> +#define CXL_POISON_SOURCE_UNKNOWN 0
> +#define CXL_POISON_SOURCE_EXTERNAL 1
> +#define CXL_POISON_SOURCE_INTERNAL 2
> +#define CXL_POISON_SOURCE_INJECTED 3
> +#define CXL_POISON_SOURCE_VENDOR 7
> +
> /**
> * struct cxl_mem_command - Driver representation of a memory device command
> * @info: Command information as it exists for the UAPI
> diff --git a/include/trace/events/cxl.h b/include/trace/events/cxl.h
> new file mode 100644
> index 000000000000..03428125573f
> --- /dev/null
> +++ b/include/trace/events/cxl.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM cxl
> +
> +#if !defined(_CXL_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _CXL_TRACE_H
> +
> +#include <linux/tracepoint.h>
> +#include <cxlmem.h>
> +
> +#define __show_poison_source(source) \
> + __print_symbolic(source, \
> + { CXL_POISON_SOURCE_UNKNOWN, "Unknown" }, \
> + { CXL_POISON_SOURCE_EXTERNAL, "External" }, \
> + { CXL_POISON_SOURCE_INTERNAL, "Internal" }, \
> + { CXL_POISON_SOURCE_INJECTED, "Injected" }, \
> + { CXL_POISON_SOURCE_VENDOR, "Vendor" })
> +
> +#define show_poison_source(source) \
> + (((source > CXL_POISON_SOURCE_INJECTED) && \
> + (source != CXL_POISON_SOURCE_VENDOR)) ? "Reserved" \
> + : __show_poison_source(source))
> +
> +#define show_poison_flags(flags) \
> + __print_flags(flags, "|", \
> + { CXL_POISON_FLAG_MORE, "More" }, \
> + { CXL_POISON_FLAG_OVERFLOW, "Overflow" }, \
> + { CXL_POISON_FLAG_SCANNING, "Scanning" })
> +
> +TRACE_EVENT(cxl_poison,
> +
> + TP_PROTO(const char *memdev, const char *pcidev, const char *region,
Same feedback as on the Events patch, make these arguments a 'struct
device *', 'struct pci_dev *', and 'struct cxl_region *' to maximize
type-safety. Otherwise you could do something like pass "region0",
"mem1", and "0000:64:09.0" in the wrong order and the compiler cannot
help prevent that.
> + const uuid_t *uuid, u64 dpa, u32 length, u8 source,
> + u8 flags, u64 overflow_t),
> +
> + TP_ARGS(memdev, pcidev, region, uuid, dpa, length, source,
> + flags, overflow_t),
> +
> + TP_STRUCT__entry(
> + __string(memdev, memdev)
> + __string(pcidev, pcidev)
> + __string(region, region ? region : "")
> + __array(char, uuid, 16)
> + __field(u64, dpa)
> + __field(u32, length)
> + __field(u8, source)
> + __field(u8, flags)
> + __field(u64, overflow_t)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(memdev, memdev);
> + __assign_str(pcidev, pcidev);
> + __assign_str(region, region ? region : "");
> + if (uuid)
> + memcpy(__entry->uuid, uuid, 16);
> + __entry->dpa = dpa;
> + __entry->length = length;
> + __entry->source = source;
> + __entry->flags = flags;
> + __entry->overflow_t = overflow_t;
> + ),
> +
> + TP_printk("memdev=%s pcidev=%s region=%s region_uuid=%pU dpa=0x%llx length=0x%x source=%s flags=%s overflow_time=%llu",
> + __get_str(memdev),
> + __get_str(pcidev),
> + __get_str(region),
> + __entry->uuid,
> + __entry->dpa,
> + __entry->length,
> + show_poison_source(__entry->source),
> + show_poison_flags(__entry->flags),
> + __entry->overflow_t)
> +);
> +#endif /* _CXL_TRACE_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE cxl
> +#include <trace/define_trace.h>
> --
> 2.37.3
>
@@ -347,6 +347,20 @@ struct cxl_mbox_set_partition_info {
#define CXL_SET_PARTITION_IMMEDIATE_FLAG BIT(0)
+/* Get Poison List CXL 3.0 Spec 8.2.9.8.4.1 */
+
+/* Get Poison List: Payload out flags */
+#define CXL_POISON_FLAG_MORE BIT(0)
+#define CXL_POISON_FLAG_OVERFLOW BIT(1)
+#define CXL_POISON_FLAG_SCANNING BIT(2)
+
+/* Get Poison List: Poison Source */
+#define CXL_POISON_SOURCE_UNKNOWN 0
+#define CXL_POISON_SOURCE_EXTERNAL 1
+#define CXL_POISON_SOURCE_INTERNAL 2
+#define CXL_POISON_SOURCE_INJECTED 3
+#define CXL_POISON_SOURCE_VENDOR 7
+
/**
* struct cxl_mem_command - Driver representation of a memory device command
* @info: Command information as it exists for the UAPI
new file mode 100644
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cxl
+
+#if !defined(_CXL_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _CXL_TRACE_H
+
+#include <linux/tracepoint.h>
+#include <cxlmem.h>
+
+#define __show_poison_source(source) \
+ __print_symbolic(source, \
+ { CXL_POISON_SOURCE_UNKNOWN, "Unknown" }, \
+ { CXL_POISON_SOURCE_EXTERNAL, "External" }, \
+ { CXL_POISON_SOURCE_INTERNAL, "Internal" }, \
+ { CXL_POISON_SOURCE_INJECTED, "Injected" }, \
+ { CXL_POISON_SOURCE_VENDOR, "Vendor" })
+
+#define show_poison_source(source) \
+ (((source > CXL_POISON_SOURCE_INJECTED) && \
+ (source != CXL_POISON_SOURCE_VENDOR)) ? "Reserved" \
+ : __show_poison_source(source))
+
+#define show_poison_flags(flags) \
+ __print_flags(flags, "|", \
+ { CXL_POISON_FLAG_MORE, "More" }, \
+ { CXL_POISON_FLAG_OVERFLOW, "Overflow" }, \
+ { CXL_POISON_FLAG_SCANNING, "Scanning" })
+
+TRACE_EVENT(cxl_poison,
+
+ TP_PROTO(const char *memdev, const char *pcidev, const char *region,
+ const uuid_t *uuid, u64 dpa, u32 length, u8 source,
+ u8 flags, u64 overflow_t),
+
+ TP_ARGS(memdev, pcidev, region, uuid, dpa, length, source,
+ flags, overflow_t),
+
+ TP_STRUCT__entry(
+ __string(memdev, memdev)
+ __string(pcidev, pcidev)
+ __string(region, region ? region : "")
+ __array(char, uuid, 16)
+ __field(u64, dpa)
+ __field(u32, length)
+ __field(u8, source)
+ __field(u8, flags)
+ __field(u64, overflow_t)
+ ),
+
+ TP_fast_assign(
+ __assign_str(memdev, memdev);
+ __assign_str(pcidev, pcidev);
+ __assign_str(region, region ? region : "");
+ if (uuid)
+ memcpy(__entry->uuid, uuid, 16);
+ __entry->dpa = dpa;
+ __entry->length = length;
+ __entry->source = source;
+ __entry->flags = flags;
+ __entry->overflow_t = overflow_t;
+ ),
+
+ TP_printk("memdev=%s pcidev=%s region=%s region_uuid=%pU dpa=0x%llx length=0x%x source=%s flags=%s overflow_time=%llu",
+ __get_str(memdev),
+ __get_str(pcidev),
+ __get_str(region),
+ __entry->uuid,
+ __entry->dpa,
+ __entry->length,
+ show_poison_source(__entry->source),
+ show_poison_flags(__entry->flags),
+ __entry->overflow_t)
+);
+#endif /* _CXL_TRACE_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE cxl
+#include <trace/define_trace.h>