Re: [RFC V2 PATCH 02/11] cxl/mem: Implement Get Event Records command

[Smita Koralahalli]

Hi Ira,

On 10/10/22 5:41 PM, ira.weiny@xxxxxxxxx wrote:
> From: Ira Weiny <ira.weiny@xxxxxxxxx>
>
>
>   
> +static void cxl_mem_get_records_log(struct cxl_dev_state *cxlds,
> +				    enum cxl_event_log_type type)
> +{
> +	struct cxl_get_event_payload payload;
> +
> +	do {
> +		u8 log_type = type;
> +		int rc;
> +
> +		rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_GET_EVENT_RECORD,
> +				       &log_type, sizeof(log_type),
> +				       &payload, sizeof(payload));
> +		if (rc) {
> +			dev_err(cxlds->dev, "Event log '%s': Failed to query event records : %d",
> +				cxl_event_log_type_str(type), rc);
> +			return;
> +		}
> +
> +		if (le16_to_cpu(payload.record_count) == 1)
> +			trace_generic_event(dev_name(cxlds->dev), type,
> +					    &payload.record);
> +
> +		if (payload.flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> +			trace_overflow(dev_name(cxlds->dev), type, &payload);
> +
> +	} while (payload.flags & CXL_GET_EVENT_FLAG_MORE_RECORDS);
> +}
> +
> +/**
> + * cxl_mem_get_event_records - Get Event Records from the device
> + * @cxlds: The device data for the operation
> + *
> + * Retrieve all event records available on the device and report them as trace
> + * events.
> + *
> + * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
> + */
> +void cxl_mem_get_event_records(struct cxl_dev_state *cxlds)
> +{
> +	enum cxl_event_log_type log_type;
> +
> +	dev_dbg(cxlds->dev, "Reading event logs\n");
> +
> +	for (log_type = CXL_EVENT_TYPE_INFO;
> +	     log_type < CXL_EVENT_TYPE_MAX; log_type++)

Why should we loop through each event log here? What if the event
record doesn't exist in the event log?

I got some Mailbox error messages like this while bootup..
[  346.387010] cxl_pci 0000:7f:00.0: Sending command
[  346.387181] cxl_pci 0000:7f:00.0: Doorbell wait took 0ms
[  346.387197] cxl_pci 0000:7f:00.0: Mailbox operation had an error: cmd 
input was invalid
[  346.387205] cxl_pci 0000:7f:00.0: Event log 'Warning': Failed to 
query event records : -6
..

Can we just read the "Event Status" field from Event Status Register
(Device Status Registers Capability Offset + 00h) 8.2.8.3.1 in CXL Spec,
determine if the records exist and just query those event logs?

Thanks,
Smita

> +		cxl_mem_get_records_log(cxlds, log_type);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, CXL);
> +
>   /**
>    * cxl_mem_get_partition_info - Get partition info
>    * @cxlds: The device data for the operation