Re: [PATCH v3 10/11] trace: platform/x86/intel/ifs: Add trace point to track Intel IFS operations

Steven Rostedt
2022-04-21T12:41:58Z

On Thu, 21 Apr 2022 04:26:39 +0000
"Luck, Tony" <tony.luck@intel.com> wrote:

> >> +TRACE_EVENT(ifs_status,
> >> +
> >> + TP_PROTO(union ifs_scan activate, union ifs_status status),
> >
> > Really, you want to pass the structure in by value, so that we have two
> > copies? One to get to this function and then one to write to the ring
> > buffer?
>
> These "structures" are just bitfield helpers for a u64 that is passed into
> WRMSR (in the case of activate) and received back from RDMSR in
> the case of status.
>
> So this is really just a pair of u64 arguments, with the compiler handling
> the bit field extractions into the ring buffer.

I was just wondering if passing by reference would be better, but as you
stated, they are just two u64 arguments.

>
> Here are the definitions:
>
> union ifs_scan {
> u64 data;
> struct {
> u32 start :8;
> u32 stop :8;
> u32 rsvd :16;
> u32 delay :31;
> u32 sigmce :1;
> };
> };
>
> union ifs_status {
> u64 data;
> struct {
> u32 chunk_num :8;
> u32 chunk_stop_index :8;
> u32 rsvd1 :16;
> u32 error_code :8;
> u32 rsvd2 :22;
> u32 control_error :1;
> u32 signature_error :1;
> };
> };
>
> Would it be better to do the bit extractions of the start/stop fields first?

No, I'm just paranoid about passing structures / unions in by value and not
reference. If you are fine with this, then I'm OK too.

-- Steve