Received-SPF: pass (google.com: domain of linux-kernel+bounces-36281-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 23 Jan 2024 20:38:53 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: "Naik, Avadhut" <avadnaik@amd.com>
Cc: Tony Luck <tony.luck@intel.com>, Avadhut Naik <avadhut.naik@amd.com>,
 linux-trace-kernel@vger.kernel.org, linux-edac@vger.kernel.org,
 bp@alien8.de, x86@kernel.org, linux-kernel@vger.kernel.org,
 yazen.ghannam@amd.com
Subject: Re: [PATCH] tracing: Include PPIN in mce_record tracepoint
Message-ID: <20240123203853.66655e95@rorschach.local.home>
In-Reply-To: <be870e14-eeb9-4dcf-ba43-a72ef66a3d87@amd.com>
References: <20240123235150.3744089-1-avadhut.naik@amd.com>
	<ZbBV4EGrZw6hJ5IE@agluck-desk3>
	<be870e14-eeb9-4dcf-ba43-a72ef66a3d87@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 23 Jan 2024 19:29:52 -0600
"Naik, Avadhut" <avadnaik@amd.com> wrote:

> > But some questions:
> > 
> > 1) Are tracepoints a user visible ABI? Adding a new field in the middle
> > feels like it might be problematic. I asked this question many years
> > ago and Steven Rostedt said there was some tracing library in the works
> > that would make this OK for appplications using that library.
> >   
> 
> I think they can be user visible through the "trace" and "trace_pipe" in
> /sys/kernel/debug/tracing. But you will have to enable the events you want
> to trace through /sys/kernel/debug/tracing/events/<event-name>/enable.
> 
> AFAIK, this (adding field in the middle) shouldn't be problematic as we
> have the tracepoint format available in debugfs. For e.g. with this patch,
> the format is as follows:
> 
> [root avadnaik]# cat /sys/kernel/debug/tracing/events/mce/mce_record/format 
> name: mce_record
> ID: 113
> format:
>         field:unsigned short common_type;       offset:0;       size:2; signed:0;
>         field:unsigned char common_flags;       offset:2;       size:1; signed:0;
>         field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
>         field:int common_pid;   offset:4;       size:4; signed:1;
> 
>         field:u64 mcgcap;       offset:8;       size:8; signed:0;
>         field:u64 mcgstatus;    offset:16;      size:8; signed:0;
>         field:u64 status;       offset:24;      size:8; signed:0;
>         field:u64 addr; offset:32;      size:8; signed:0;
>         field:u64 misc; offset:40;      size:8; signed:0;
>         field:u64 synd; offset:48;      size:8; signed:0;
>         field:u64 ipid; offset:56;      size:8; signed:0;
>         field:u64 ip;   offset:64;      size:8; signed:0;
>         field:u64 tsc;  offset:72;      size:8; signed:0;
>         field:u64 ppin; offset:80;      size:8; signed:0;
>         field:u64 walltime;     offset:88;      size:8; signed:0;
>         field:u32 cpu;  offset:96;      size:4; signed:0;
>         field:u32 cpuid;        offset:100;     size:4; signed:0;
>         field:u32 apicid;       offset:104;     size:4; signed:0;
>         field:u32 socketid;     offset:108;     size:4; signed:0;
>         field:u8 cs;    offset:112;     size:1; signed:0;
>         field:u8 bank;  offset:113;     size:1; signed:0;
>         field:u8 cpuvendor;     offset:114;     size:1; signed:0;
> 
> print fmt: "CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, IPID: %016Lx, ADDR/MISC/SYND: %016Lx/%016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PPIN: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x", REC->cpu, REC->mcgcap, REC->mcgstatus, REC->bank, REC->status, REC->ipid, REC->addr, REC->misc, REC->synd, REC->cs, REC->ip, REC->tsc, REC->ppin, REC->cpuvendor, REC->cpuid, REC->walltime, REC->socketid, REC->apicid
> 
> 
> Just quickly tried with rasdaemon and things seem to be okay.
> 
> Also, not a cent percent sure, but the library you are mentioning of, I think
> its the libtraceevent library and IIUC, it utilizes the above tracepoint format.
> 

Yes, rasdaemon uses libtraceevent (or a copy of it internally) that
reads the format file to find fields. You can safely add fields to the
middle of the event structure and the parsing will be just fine.

-- Steve