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

[Steven Rostedt <rostedt@goodmis.org>]

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)