RE: [PATCH 4/4] cxl/events: Trace Memory Sparing Event Record

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 16 July 2025 14:17
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-cxl@vger.kernel.org; dan.j.williams@intel.com; dave.jiang@intel.com;
>alison.schofield@intel.com; dave@stgolabs.net; vishal.l.verma@intel.com;
>ira.weiny@intel.com; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>
>Subject: Re: [PATCH 4/4] cxl/events: Trace Memory Sparing Event Record
>
>On Wed, 16 Jul 2025 11:49:45 +0100
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL rev 3.2 section 8.2.10.2.1.4 Table 8-60 defines the Memory Sparing
>> Event Record.
>>
>> Determine if the event read is memory sparing record and if so trace
>> the record.
>>
>> Memory device shall produce a memory sparing event record 1. After
>> completion of a PPR maintenance operation if the memory sparing event
>> record enable bit is set (Field: sPPR/hPPR Operation Mode in Table
>> 8-128/Table 8-131).
>> 2. In response to a query request by the host (see section
>> 8.2.10.7.1.4) to determine the availability of sparing resources.
>> The device shall report the resource availability by producing the
>> Memory Sparing Event Record (see Table 8-60) in which the channel,
>> rank, nibble mask, bank group, bank, row, column, sub-channel fields
>> are a copy of the values specified in the request. If the controller
>> does not support reporting whether a resource is available, and a
>> perform maintenance operation for memory sparing is issued with query
>> resources set to 1, the controller shall return invalid input.
>>
>> Example trace log for produce memory sparing event record on
>> completion of a soft PPR operation,
>> cxl_memory_sparing: memdev=mem1 host=0000:0f:00.0 serial=3
>> log=Informational : time=55045163029
>> uuid=e71f3a40-2d29-4092-8a39-4d1c966c7c65 len=128 flags='0x1' handle=1
>> related_handle=0 maint_op_class=2 maint_op_sub_class=1
>> ld_id=0 head_id=0 : flags='' result=0
>> validity_flags='CHANNEL|RANK|NIBBLE|BANK GROUP|BANK|ROW|COLUMN'
>> spare resource avail=1 channel=2 rank=5 nibble_mask=a59c bank_group=2
>> bank=4 row=13 column=23 sub_channel=0
>> comp_id=00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> comp_id_pldm_valid_flags='' pldm_entity_id=0x00 pldm_resource_id=0x00
>>
>> Note: For memory sparing event record, fields 'maintenance operation
>> class' and 'maintenance operation subclass' are defined twice, first
>> in the common event record (Table 8-55) and second in the memory
>> sparing event record (Table 8-60). Thus those in the sparing event
>> record coded as reserved, to be removed when the spec is updated.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>Only comment formatting related.
>
>Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
>> ---
>>  drivers/cxl/core/mbox.c  |   6 +++
>>  drivers/cxl/core/trace.h | 100
>+++++++++++++++++++++++++++++++++++++++
>>  drivers/cxl/cxlmem.h     |   8 ++++
>>  include/cxl/event.h      |  33 +++++++++++++
>>  4 files changed, 147 insertions(+)
>>
>
>> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h index
>> c3cd871942c5..2c291fb1857c 100644
>> --- a/drivers/cxl/core/trace.h
>> +++ b/drivers/cxl/core/trace.h
>> @@ -888,6 +888,106 @@ TRACE_EVENT(cxl_memory_module,
>>  	)
>>  );
>>
>> +#define CXL_MSER_QUERY_RESOURCE_FLAG			BIT(0)
>> +#define CXL_MSER_HARD_SPARING_FLAG			BIT(1)
>> +#define CXL_MSER_DEV_INITED_FLAG			BIT(2)
>> +#define show_mem_sparing_flags(flags)	__print_flags(flags, "|",
>	\
>> +	{ CXL_MSER_QUERY_RESOURCE_FLAG,		"Query Resources" },
>		\
>> +	{ CXL_MSER_HARD_SPARING_FLAG,		"Hard Sparing" },
>		\
>> +	{ CXL_MSER_DEV_INITED_FLAG,	"Device Initiated Sparing"
>	}	\
>
>Spacing before the } is inconsistent for this last line.  Copy whatever we have in
>the file already and if it is inconsistent (which it is) pick most common option.

Thanks Jonathan.
I will correct in v2.
>
>> +)
>

Thanks
Shiju