Message-ID: <76c527e3-b376-18df-bc93-f5a75e029d76@intel.com>
Date:   Wed, 15 Feb 2023 13:18:20 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2 4/7] platform/x86/intel/ifs: Implement Array BIST test
Content-Language: en-US
To:     "Joseph, Jithu" <jithu.joseph@intel.com>, hdegoede@redhat.com,
        markgross@kernel.org
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
        gregkh@linuxfoundation.org, rostedt@goodmis.org,
        ashok.raj@intel.com, tony.luck@intel.com,
        linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        patches@lists.linux.dev, ravi.v.shankar@intel.com,
        thiago.macieira@intel.com, athenas.jimenez.gonzalez@intel.com,
        sohil.mehta@intel.com
References: <20230131234302.3997223-1-jithu.joseph@intel.com>
 <20230214234426.344960-1-jithu.joseph@intel.com>
 <20230214234426.344960-5-jithu.joseph@intel.com>
 <a24c65f8-978d-8968-7874-6b83e14b01ba@intel.com>
 <d9d18954-8434-4c85-88b8-8e1d99cd6a4b@intel.com>
 <dd501d6a-e7e9-be82-24fb-33b18fb6192b@intel.com>
 <b67d2125-4075-4eac-4cad-8e315b866144@intel.com>
 <5d57f42a-1bfc-5e6a-bb49-556ecb7402af@intel.com>
From:   Dave Hansen <dave.hansen@intel.com>
In-Reply-To: <5d57f42a-1bfc-5e6a-bb49-556ecb7402af@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/15/23 13:13, Joseph, Jithu wrote:
> 
> On 2/15/2023 12:26 PM, Dave Hansen wrote:
>> On 2/15/23 12:22, Joseph, Jithu wrote:
>>> 		trace_ifs_array(cpu, *((u64 *)&before), *((u64 *)&command));
>> Uhh, you control the types in the tracepoint.  Just make them compatible
>> so you don't need casts.
> will change it to:
> trace_ifs_array(cpu, before.array_bitmask, before.array_bank,  *((u64 *)&command));
> 
> i.e will pass compatible types for array_list and array_bank. And for the last argument, we need to dump the whole 64 bits within "command"
> into trace output . Since the suggested change replaced the union with a struct, it is simplest to cast it to u64 needed by traceoutput.
> So I would prefer to keep the cast for the last argument alone.

<sigh>

Your trace even can literally be:

+	TP_STRUCT__entry(
+		__field(struct ifs_foo,	before	)
+		__field(struct ifs_foo,	after	)
+		__field(	int,	cpu	)
+	),

and then you can just use structure assignment or a memcpy.  *That* is
what I mean by compatible types.

But, also, I'm not sure these tracepoints even make any sense.  You're
passing raw MSR contents back and forth.  Why not just use the MSR
tracepoints?  They'll give you the same data.