linux-kernel - [PATCH v4 2/4] x86/mce, EDAC/mce_amd: Add support for new MCA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9189a857-8c7b-42ac-958b-89a3d6a7ded5@amd.com>
Date: Wed, 21 Aug 2024 13:55:00 -0500
From: Avadhut Naik <avadnaik@....com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: x86@...nel.org, linux-trace-kernel@...r.kernel.org,
 linux-edac@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org, bp@...en8.de, tony.luck@...el.com,
 rafael@...nel.org, tglx@...utronix.de, mingo@...hat.com, lenb@...nel.org,
 mchehab@...nel.org, james.morse@....com, airlied@...il.com,
 yazen.ghannam@....com, john.allen@....com,
 Avadhut Naik <avadhut.naik@....com>
Subject: [PATCH v4 2/4] x86/mce, EDAC/mce_amd: Add support for new
 MCA_SYND{1,2} registers



On 8/21/24 12:40, Steven Rostedt wrote:
> On Thu, 15 Aug 2024 16:16:33 -0500
> Avadhut Naik <avadhut.naik@....com> wrote:
> 
>> diff --git a/include/trace/events/mce.h b/include/trace/events/mce.h
>> index 65aba1afcd07..1e7d5696b3ba 100644
>> --- a/include/trace/events/mce.h
>> +++ b/include/trace/events/mce.h
>> @@ -43,6 +43,8 @@ TRACE_EVENT(mce_record,
>>  		__field(	u8,		bank		)
>>  		__field(	u8,		cpuvendor	)
>>  		__field(	u32,		microcode	)
>> +		__field(	u8,		len	)
> 
> You don't need to save the length. It's already saved in the
> __dynamic_array meta data.
> 
That helps! Will remove the len field.

>> +		__dynamic_array(u8, v_data, sizeof(err->vendor))
>>  	),
>>  
>>  	TP_fast_assign(
>> @@ -65,9 +67,11 @@ TRACE_EVENT(mce_record,
>>  		__entry->bank		= err->m.bank;
>>  		__entry->cpuvendor	= err->m.cpuvendor;
>>  		__entry->microcode	= err->m.microcode;
>> +		__entry->len		= sizeof(err->vendor);
>> +		memcpy(__get_dynamic_array(v_data), &err->vendor, sizeof(err->vendor));
>>  	),
>>  
>> -	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, IPID: %016Lx, ADDR: %016Lx, MISC: %016Lx, SYND: %016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PPIN: %llx, vendor: %u, CPUID: %x, time: %llu, socket: %u, APIC: %x, microcode: %x",
>> +	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016llx, IPID: %016llx, ADDR: %016llx, MISC: %016llx, SYND: %016llx, RIP: %02x:<%016llx>, TSC: %llx, PPIN: %llx, vendor: %u, CPUID: %x, time: %llu, socket: %u, APIC: %x, microcode: %x, vendor data: %s",
>>  		__entry->cpu,
>>  		__entry->mcgcap, __entry->mcgstatus,
>>  		__entry->bank, __entry->status,
>> @@ -83,7 +87,8 @@ TRACE_EVENT(mce_record,
>>  		__entry->walltime,
>>  		__entry->socketid,
>>  		__entry->apicid,
>> -		__entry->microcode)
>> +		__entry->microcode,
>> +		__print_array(__get_dynamic_array(v_data), __entry->len / 8, 8))
> 
> You can replace the __entry->len with:
> 
> 		__print_array(__get_dynamic_array(v_data), __get_dynamic_array_len(v_data) / 8, 8))
> 
> Hmm, I should add a helper function that would do the same with just:
> 
> 		__print_dynamic_array(vdata, 8);
> 
> Where it will have __print_dynamic_array(array-name, el-size)
> 
> -- Steve
> 
Thanks for the patch! Will incorporate it this set.

I did, however, have another question. This was actually raised in an earlier
version of this set by Boris and Tony. Not very sure about the answer though.

Is there a way through which we can make the tracepoints smarter i.e. have
conditional checks in them?

To provide some more context, as of now, the v_data dynamic array above will
contain vendor-specific data only on AMD systems. On Intel systems, IIUC,
its contents will be 0x0.

Consequently, the tracepoint output on Intel systems will have some thing like
"0x0,0x0,0x0" at the end.

Is there some way through which we can work around this? Can we perform a
vendor check inside the tracepoint so that the above dynamic array is logged
only on AMD systems and not on Intel systems?

-- 
Thanks,
Avadhut Naik