Re: [PATCH V2 2/4] rasdaemon: Add support for the CXL poison events

[Ira Weiny <ira.weiny@intel.com>]

shiju.jose@ wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Add support to log and record the CXL poison events.
> 
> The corresponding Kernel patches here:
> https://lore.kernel.org/linux-cxl/de11785ff05844299b40b100f8e0f56c7eef7f08.1674070170.git.alison.schofield@intel.com/
> 
> Presently RFC draft version for logging, could be extended for the policy
> based recovery action for the frequent poison events depending on the above
> kernel patches.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

[snip]

> +
> +int ras_cxl_poison_event_handler(struct trace_seq *s,
> +				 struct tep_record *record,
> +				 struct tep_event *event, void *context)
> +{
> +	int len;
> +	unsigned long long val;
> +	struct ras_events *ras = context;
> +	time_t now;
> +	struct tm *tm;
> +	struct ras_cxl_poison_event ev;
> +
> +	now = record->ts/user_hz + ras->uptime_diff;
> +	tm = localtime(&now);
> +	if (tm)
> +		strftime(ev.timestamp, sizeof(ev.timestamp),
> +			 "%Y-%m-%d %H:%M:%S %z", tm);
> +	else
> +		strncpy(ev.timestamp, "1970-01-01 00:00:00 +0000", sizeof(ev.timestamp));
> +	trace_seq_printf(s, "%s ", ev.timestamp);
> +
> +	ev.memdev = tep_get_field_raw(s, event, "memdev",
> +				      record, &len, 1);
> +	if (!ev.memdev)
> +		return -1;
> +	trace_seq_printf(s, "memdev:%s ", ev.memdev);
> +
> +	ev.pcidev = tep_get_field_raw(s, event, "pcidev",
> +				      record, &len, 1);
> +	if (!ev.pcidev)
> +		return -1;
> +	trace_seq_printf(s, "pcidev:%s ", ev.pcidev);
> +
> +	ev.region = tep_get_field_raw(s, event, "region",
> +				      record, &len, 1);
> +	if (!ev.region)
> +		return -1;
> +	trace_seq_printf(s, "region:%s ", ev.region);
> +
> +	ev.uuid = tep_get_field_raw(s, event, "uuid",
> +				    record, &len, 1);
> +	if (!ev.uuid)
> +		return -1;
> +	trace_seq_printf(s, "region_uuid:%s ", ev.uuid);
> +
> +	if (tep_get_field_val(s, event, "hpa", record, &val, 1) < 0)
> +		return -1;
> +	ev.hpa = val;
> +	trace_seq_printf(s, "poison list: hpa:0x%llx ", (unsigned long long)ev.hpa);
> +
> +	if (tep_get_field_val(s, event, "dpa", record, &val, 1) < 0)
> +		return -1;
> +	ev.dpa = val;
> +	trace_seq_printf(s, "dpa:0x%llx ", (unsigned long long)ev.dpa);
> +
> +	if (tep_get_field_val(s, event, "length", record, &val, 1) < 0)
> +		return -1;
> +	ev.length = val;
> +	trace_seq_printf(s, "length:%d ", ev.length);
> +
> +	if (tep_get_field_val(s,  event, "source", record, &val, 1) < 0)
> +		return -1;
> +
> +	switch (val) {
> +	case CXL_POISON_SOURCE_UNKNOWN:
> +		ev.source = "Unknown";
> +		break;
> +	case CXL_POISON_SOURCE_EXTERNAL:
> +		ev.source = "External";
> +		break;
> +	case CXL_POISON_SOURCE_INTERNAL:
> +		ev.source = "Internal";
> +		break;
> +	case CXL_POISON_SOURCE_INJECTED:
> +		ev.source = "Injected";
> +		break;
> +	case CXL_POISON_SOURCE_VENDOR:
> +		ev.source = "Vendor";
> +		break;
> +	default:
> +		ev.source = "Invalid";
> +	}
> +	trace_seq_printf(s, "source:%s ", ev.source);
> +
> +	if (tep_get_field_val(s,  event, "flags", record, &val, 1) < 0)
> +		return -1;
> +	ev.flags = val;
> +	trace_seq_printf(s, "flags:%d ", ev.flags);
> +
> +	if (ev.flags & CXL_POISON_FLAG_OVERFLOW) {
> +		if (tep_get_field_val(s,  event, "overflow_t", record, &val, 1) < 0)
> +			return -1;
> +		if (val) {
> +			/* CXL Specification 3.0
> +			 * Overflow timestamp - The number of unsigned nanoseconds
> +			 * that have elapsed since midnight, 01-Jan-1970 UTC
> +			 */
> +			time_t ovf_ts_secs = val / 1000000000ULL;
> +
> +			tm = localtime(&ovf_ts_secs);
> +			if (tm) {
> +				strftime(ev.overflow_ts, sizeof(ev.overflow_ts),
> +					 "%Y-%m-%d %H:%M:%S %z", tm);
> +			}
> +		}
> +		if (!val || !tm)
> +			strncpy(ev.overflow_ts, "1970-01-01 00:00:00 +0000",
> +				sizeof(ev.overflow_ts));
> +	} else
> +		strncpy(ev.overflow_ts, "1970-01-01 00:00:00 +0000", sizeof(ev.overflow_ts));
> +	trace_seq_printf(s, "overflow timestamp:%s ", ev.overflow_ts);
> +	trace_seq_printf(s, "\n");
> +
> +	/* Insert data into the SGBD */
> +#ifdef HAVE_SQLITE3
> +	ras_store_cxl_poison_event(ras, &ev);
> +#endif

I know nothing about the rasdaemon build system but it seems like this
needs a ifdef HAVE_CXL around it?

[snip]

> --- a/ras-record.c
> +++ b/ras-record.c
> @@ -559,6 +559,67 @@ int ras_store_mf_event(struct ras_events *ras, struct ras_mf_event *ev)
>  }
>  #endif
>  
> +#ifdef HAVE_CXL
> +/*
> + * Table and functions to handle cxl:cxl_poison
> + */
> +static const struct db_fields cxl_poison_event_fields[] = {
> +	{ .name = "id",                   .type = "INTEGER PRIMARY KEY" },
> +	{ .name = "timestamp",            .type = "TEXT" },
> +	{ .name = "memdev",               .type = "TEXT" },
> +	{ .name = "pcidev",               .type = "TEXT" },
> +	{ .name = "region",               .type = "TEXT" },
> +	{ .name = "region_uuid",          .type = "TEXT" },
> +	{ .name = "hpa",                  .type = "INTEGER" },
> +	{ .name = "dpa",                  .type = "INTEGER" },
> +	{ .name = "length",               .type = "INTEGER" },
> +	{ .name = "source",               .type = "TEXT" },
> +	{ .name = "flags",                .type = "INTEGER" },
> +	{ .name = "overflow_ts",          .type = "TEXT" },
> +};
> +
> +static const struct db_table_descriptor cxl_poison_event_tab = {
> +	.name = "cxl_poison_event",
> +	.fields = cxl_poison_event_fields,
> +	.num_fields = ARRAY_SIZE(cxl_poison_event_fields),
> +};
> +
> +int ras_store_cxl_poison_event(struct ras_events *ras, struct ras_cxl_poison_event *ev)

Because I believe this is not defined if (!HAVE_CXL and HAVE_SQLITE3)

[snip]

>  
>  #ifdef HAVE_SQLITE3
>  
> @@ -155,6 +170,9 @@ struct sqlite3_priv {
>  #ifdef HAVE_MEMORY_FAILURE
>  	sqlite3_stmt	*stmt_mf_event;
>  #endif
> +#ifdef HAVE_CXL
> +	sqlite3_stmt	*stmt_cxl_poison_event;
> +#endif
>  };
>  
>  struct db_fields {
> @@ -182,6 +200,7 @@ int ras_store_arm_record(struct ras_events *ras, struct ras_arm_event *ev);
>  int ras_store_devlink_event(struct ras_events *ras, struct devlink_event *ev);
>  int ras_store_diskerror_event(struct ras_events *ras, struct diskerror_event *ev);
>  int ras_store_mf_event(struct ras_events *ras, struct ras_mf_event *ev);
> +int ras_store_cxl_poison_event(struct ras_events *ras, struct ras_cxl_poison_event *ev);
>  
>  #else
>  static inline int ras_mc_event_opendb(unsigned cpu, struct ras_events *ras) { return 0; };
> @@ -195,6 +214,7 @@ static inline int ras_store_arm_record(struct ras_events *ras, struct ras_arm_ev
>  static inline int ras_store_devlink_event(struct ras_events *ras, struct devlink_event *ev) { return 0; };
>  static inline int ras_store_diskerror_event(struct ras_events *ras, struct diskerror_event *ev) { return 0; };
>  static inline int ras_store_mf_event(struct ras_events *ras, struct ras_mf_event *ev) { return 0; };
> +static inline int ras_store_cxl_poison_event(struct ras_events *ras, struct ras_cxl_poison_event *ev) { return 0; };

But I could be missing something.

Ira

[snip]