diff mbox series

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

Message ID 20240627023516.3783454-5-sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series Add SBAF test to IFS | expand

Commit Message

Kuppuswamy Sathyanarayanan June 27, 2024, 2:35 a.m. UTC 
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(+)

Comments

Steven Rostedt June 27, 2024, 1:56 p.m. UTC | #1 
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.

-- 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)
Kuppuswamy Sathyanarayanan June 27, 2024, 2:04 p.m. UTC | #2 
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)
diff mbox series

Patch

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	)
+		__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)