Received-SPF: pass (google.com: domain of linux-kernel+bounces-39270-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <a5ddf76b-26bb-49ed-b202-0bde606a3bea@intel.com>
Date: Thu, 25 Jan 2024 12:58:16 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/2] tracing: Include PPIN in mce_record tracepoint
Content-Language: en-US
To: Avadhut Naik <avadhut.naik@amd.com>, <linux-trace-kernel@vger.kernel.org>,
	<linux-edac@vger.kernel.org>
CC: <rostedt@goodmis.org>, <tony.luck@intel.com>, <bp@alien8.de>,
	<x86@kernel.org>, <linux-kernel@vger.kernel.org>, <yazen.ghannam@amd.com>,
	<avadnaik@amd.com>
References: <20240125184857.851355-1-avadhut.naik@amd.com>
 <20240125184857.851355-2-avadhut.naik@amd.com>
From: Sohil Mehta <sohil.mehta@intel.com>
In-Reply-To: <20240125184857.851355-2-avadhut.naik@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aUErdGNLZzNrdzBXVmVTZnVpQVloVDFSK0hOWHNUVTZIUEVIR2hmWWxUcHRn?=
 =?utf-8?B?bzZZajJjYkJlR3hpcDVVd1RXdFJHUWRYWE1QbEZHMmpERU1SR3pOVm5ZbUNY?=
 =?utf-8?B?bm5GVmwra3UvS3ptUGZpZkQwS0ZKTmJUeXFkNWhOQlMxZ1BHNFhsREdDMG9J?=
 =?utf-8?B?VXNaNTJtZ1JwT2ZDQU9tZTN1V3IyWGFQd1JzWXZIUnNCWnc0VmRnM1NrN2Nr?=
 =?utf-8?B?TnA3Z2w3ZTlYQ01Gd2plR0VvUktuTVo4elQxelVyeGpqMU51V3lNQlJ6ellp?=
 =?utf-8?B?SXZIb21ZYityOHNPSlRvenRuQVlpWldOeXlDTTRBVStLbjNYZ1dKRWNPVWw3?=
 =?utf-8?B?VWFNbnNNT0Zubmw3RlFJYWNRbmF5MTlORG9SRndyWkRqblBLaG1KUFNkYUtB?=
 =?utf-8?B?bVpmY0RkOHJNYmppOGFvRS9rbDgralJsd3JBdExjUDJMdksxVE9lc2U0TlV0?=
 =?utf-8?B?VFBMSEM2dGtFUTBmVU43T3p3SXdQOWMyZkZwQnBIRCtiNGExdjhha1Iwd1Ew?=
 =?utf-8?B?QjdQa0ExTis3djMzYjJ3Q1BNbU4wcGZ1Q1hoV09vc2hXaUMzV2ZQNllJbGJq?=
 =?utf-8?B?R1ROZ1doSzBzcVJQRGlBOWZxU2RyVjMxakpVUURQdkFtdEhDU1h6SlZtZXZG?=
 =?utf-8?B?YzBOR24ySzJTaC9Oam83Ujh5RXpHRjNHWThhZHVpV3lKQ1J3b0ZoWGtWR1lu?=
 =?utf-8?B?NWdQakdUYkJ2NE9DUXdTTTVPcG9yYkUzbGNaaE02bmczTWt2andBRG9TRlVR?=
 =?utf-8?B?VlF6K2FCUzBJbXBxeUI5NDNkLzBlTUpOM1N3N3E5VGw1eVZZWHZscVdHQ3B1?=
 =?utf-8?B?c1BxcXQzUitGWkhtZmhMenplUXdZbXd4TlBLNE5SaG5PZWFrTjNIK0RkdVF5?=
 =?utf-8?B?aGhYY084dTJaNmt6MnMvUlIreGpuNUhQM3Jad01RRDREcVEzRnRXeHBVajZU?=
 =?utf-8?B?T0xhSmFLN0pyazBKYURocWU1cmJ5WUVTZHV0UXFKVHRta0s4MWlZRG0rZWls?=
 =?utf-8?B?eHgvMTVUNzRyUktFZDVjQzBXdGhZeGllcFF3NzJ0RGNFdEhWLzQwSkptdTlz?=
 =?utf-8?B?bnNjajdaYkI3WTFCdHRqYlJsS1oxODVpZFVmMUJUTkJVdEJ6dCt0cmhkMXhz?=
 =?utf-8?B?OEluNXYwcFJ4dVNYOGcrWUJFVXovVXVzdTJjK0kxbDA2ZUZZemFiVWY5VEoz?=
 =?utf-8?B?UGIxdXQrakFDckN6MWZ6WFVKcEhNb1BhVlRFczljTWExMmtFRnBSTXBhVi9N?=
 =?utf-8?B?bWsvYjVPSHpSZzhkd1pQRlRqSDExaHhSVDNyN1hVYmlUM3lUU0h6K1c1QnBv?=
 =?utf-8?B?YmhsSnVkbUZLdDYxU0RFOHNVL0w0a1hURkg3LzF5NktDQWx5dS9mR0tBTk81?=
 =?utf-8?B?dURzdWJmWnZvL2lCNUgxdEVsRlp0L0twamN1ajBkTTNSTzVHUUNpZks4REVp?=
 =?utf-8?B?WVYzR0UrbnlsTDl0QkFHSy9Sd1ZjQlpCSzdWZnVtaCtJdHdYSkpwVHJ4NVRs?=
 =?utf-8?B?QUlQZkozYjd2OWxDWWhhTVRzc1VwdWNMeDBjdkVPMWxVeVdyN1ZnQ2ZMYStT?=
 =?utf-8?B?c3Z1ekZZSXVkUnQySTNhMHlnTk4vZktCUllQR1RXbEFrNUllVWFMSGR0QmxG?=
 =?utf-8?B?SFJ5cXJlYUxMem1qZ1NwL203a1VwcmpPbnJuTWc2OHB1MnRGZW1zNm9qR211?=
 =?utf-8?B?T0h4YkppOWNuTXFLVUpBbjFJWi92OWFiNURQUVVPSzBFU0k5SzhNVEZ1QXc2?=
 =?utf-8?B?T1JKUm45NmJ2WWd3K2NyNGVMNy96WHMrN21ySDl3UEJZKy8yby9YQXJkaHcr?=
 =?utf-8?B?Ni9hdGdUb1BhbHhtenVOdDQ1ZGZUZG1wb01FUndhMWxjZzFYK3hlQWRVb1Ns?=
 =?utf-8?B?a2s5MVQrc3ZpcWt4VDk3SVJjSyszRldudnBFMVIzRkszdzJKcmloWHgraHRn?=
 =?utf-8?B?OFlzcGJtQzFDaE9GK0sxS3A0MlhBaXdzZnJreWN0TFlLRFV6dmp5TmtmcWVt?=
 =?utf-8?B?Vk45V2hkZDQyM0hnYU5SdUpXK29ibVBtcDRQQXI1YzNKVW4yeHYxdHArVHlW?=
 =?utf-8?B?RDBxM1FiMTN4V1JYQS8rSnB1aVp5cWNLamZBMFBXaTRuWWZRQ20ySitaL3hz?=
 =?utf-8?Q?Ncm6ekKq2mhZ4FsPnzZsSlmB1?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 66ce78f4-e72c-4a2b-e02d-08dc1de85b5c
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3320.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jan 2024 20:58:18.4094
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: h2jqln/JaZP0k5KSi/xaM5xSu6BgLkpMOOfsP71qSXSJ4loTeL9PCqoPaor8fSv9WVM16/CL0JG9Vg69IhRIIw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6798
X-OriginatorOrg: intel.com

On 1/25/2024 10:48 AM, Avadhut Naik wrote:
> Machine Check Error information from struct mce is exported to userspace
> through the mce_record tracepoint.
> 
> Currently, however, the PPIN (Protected Processor Inventory Number) field
> of struct mce is not exported through the tracepoint.
> 
> Export PPIN through the tracepoint as it may provide useful information
> for debug and analysis.
> 
> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
> ---

The patch looks fine to me expect for a nit below.

With that fixed, please feel free to add:
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>

>  include/trace/events/mce.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/mce.h b/include/trace/events/mce.h
> index 1391ada0da3b..657b93ec8176 100644
> --- a/include/trace/events/mce.h
> +++ b/include/trace/events/mce.h
> @@ -25,6 +25,7 @@ TRACE_EVENT(mce_record,
>  		__field(	u64,		ipid		)
>  		__field(	u64,		ip		)
>  		__field(	u64,		tsc		)
> +		__field(	u64,		ppin	)

The tabs are not aligned with the rest of the structure.

>  		__field(	u64,		walltime	)
>  		__field(	u32,		cpu		)
>  		__field(	u32,		cpuid		)
> @@ -45,6 +46,7 @@ TRACE_EVENT(mce_record,
>  		__entry->ipid		= m->ipid;
>  		__entry->ip		= m->ip;
>  		__entry->tsc		= m->tsc;
> +		__entry->ppin		= m->ppin;
>  		__entry->walltime	= m->time;
>  		__entry->cpu		= m->extcpu;
>  		__entry->cpuid		= m->cpuid;
> @@ -55,7 +57,7 @@ TRACE_EVENT(mce_record,
>  		__entry->cpuvendor	= m->cpuvendor;
>  	),
>  
> -	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, IPID: %016Lx, ADDR/MISC/SYND: %016Lx/%016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x",
> +	TP_printk("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",
>  		__entry->cpu,
>  		__entry->mcgcap, __entry->mcgstatus,
>  		__entry->bank, __entry->status,
> @@ -63,6 +65,7 @@ TRACE_EVENT(mce_record,
>  		__entry->addr, __entry->misc, __entry->synd,
>  		__entry->cs, __entry->ip,
>  		__entry->tsc,
> +		__entry->ppin,
>  		__entry->cpuvendor, __entry->cpuid,
>  		__entry->walltime,
>  		__entry->socketid,