Re: [PATCH v1 4/4] trace: platform/x86/intel/ifs: Add SBAF trace support

[Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>]

On 6/27/24 6:56 AM, Steven Rostedt wrote:
> On Thu, 27 Jun 2024 02:35:16 +0000
> Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
>> From: Jithu Joseph <jithu.joseph@intel.com>
>>
>> Add tracing support for the SBAF IFS tests, which may be useful for
>> debugging systems that fail these tests. Log details like test content
>> batch number, SBAF bundle ID, program index and the exact errors or
>> warnings encountered by each HT thread during the test.
>>
>> Reviewed-by: Ashok Raj <ashok.raj@intel.com>
>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>  include/trace/events/intel_ifs.h         | 27 ++++++++++++++++++++++++
>>  drivers/platform/x86/intel/ifs/runtest.c |  1 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/include/trace/events/intel_ifs.h b/include/trace/events/intel_ifs.h
>> index 0d88ebf2c980..9c7413de432b 100644
>> --- a/include/trace/events/intel_ifs.h
>> +++ b/include/trace/events/intel_ifs.h
>> @@ -35,6 +35,33 @@ TRACE_EVENT(ifs_status,
>>  		__entry->status)
>>  );
>>  
>> +TRACE_EVENT(ifs_sbaf,
>> +
>> +	TP_PROTO(int batch, union ifs_sbaf activate, union ifs_sbaf_status status),
>> +
>> +	TP_ARGS(batch, activate, status),
>> +
>> +	TP_STRUCT__entry(
>> +		__field(	int,	batch	)
>> +		__field(	u64,	status	)
> Please put the 64 bit status field before the 32 bit batch field,
> otherwise this will likely create a 4 byte hole between the two fields.
> Space on the ring buffer is expensive real-estate.

Agree. I will fix this in next version.

>
> -- Steve
>
>> +		__field(	u16,	bundle	)
>> +		__field(	u16,	pgm	)
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__entry->batch	= batch;
>> +		__entry->bundle	= activate.bundle_idx;
>> +		__entry->pgm	= activate.pgm_idx;
>> +		__entry->status	= status.data;
>> +	),
>> +
>> +	TP_printk("batch: 0x%.2x, bundle_idx: 0x%.4x, pgm_idx: 0x%.4x, status: 0x%.16llx",
>> +		__entry->batch,
>> +		__entry->bundle,
>> +		__entry->pgm,
>> +		__entry->status)
>> +);
>> +
>>  #endif /* _TRACE_IFS_H */
>>  
>>  /* This part must be outside protection */
>> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
>> index bdb31b2f45b4..69ee0eb72025 100644
>> --- a/drivers/platform/x86/intel/ifs/runtest.c
>> +++ b/drivers/platform/x86/intel/ifs/runtest.c
>> @@ -530,6 +530,7 @@ static int dosbaf(void *data)
>>  	 */
>>  	wrmsrl(MSR_ACTIVATE_SBAF, run_params->activate->data);
>>  	rdmsrl(MSR_SBAF_STATUS, status.data);
>> +	trace_ifs_sbaf(ifsd->cur_batch, *run_params->activate, status);
>>  
>>  	/* Pass back the result of the test */
>>  	if (cpu == first)

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer