Re: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-28 Thread Borislav Petkov 
On Thu, Mar 28, 2024 at 01:17:43AM -0500, Naik, Avadhut wrote:
> SOCKET -> Socket
> PROCESSOR -> Processor
> MICROCODE -> Microcode

SOCKET -> socket
PROCESSOR -> processor
MICROCODE -> microcode

And yeah, the acronyms need to obviously stay in all caps.

Thx.

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-27 Thread Naik, Avadhut 



On 3/27/2024 17:31, Sohil Mehta wrote:
> On 3/27/2024 1:54 PM, Avadhut Naik wrote:
> 
>> -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",
>> +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, MICROCODE 
>> REVISION: %x",
> 
> Nit: s/MICROCODE REVISION/MICROCODE/g
> 
> You could probably get rid of the word REVISION in the interest of
> brevity similar to __print_mce().
> 
>   pr_emerg(HW_ERR "PROCESSOR %u:%x TIME %llu SOCKET %u APIC %x microcode
> %x\n",
>   m->cpuvendor, m->cpuid, m->time, m->socketid, m->apicid,
>   m->microcode);
> 
>
Okay. Will remove "REVISION".
> -Sohil

-- 
Thanks,
Avadhut Naik

Re: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-27 Thread Sohil Mehta 


> 
> You *definitely* want to do that - good catch.
> 
> And TBH, all the screaming words aren't helping either... :)
> 

:) I thought the same as well. But, I felt inconsistently screaming
words might be worse. Maybe just update all the words that are not
acronyms (such as Processor, Time, Socket, etc.)

Re: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-27 Thread Borislav Petkov 
On Wed, Mar 27, 2024 at 03:31:01PM -0700, Sohil Mehta wrote:
> On 3/27/2024 1:54 PM, Avadhut Naik wrote:
> 
> > -   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",
> > +   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, MICROCODE 
> > REVISION: %x",
> 
> Nit: s/MICROCODE REVISION/MICROCODE/g
> 
> You could probably get rid of the word REVISION in the interest of
> brevity similar to __print_mce().

You *definitely* want to do that - good catch.

And TBH, all the screaming words aren't helping either... :)

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-27 Thread Sohil Mehta 
On 3/27/2024 1:54 PM, Avadhut Naik wrote:

> - 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",
> + 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, MICROCODE REVISION: 
> %x",

Nit: s/MICROCODE REVISION/MICROCODE/g

You could probably get rid of the word REVISION in the interest of
brevity similar to __print_mce().

pr_emerg(HW_ERR "PROCESSOR %u:%x TIME %llu SOCKET %u APIC %x microcode
%x\n",
m->cpuvendor, m->cpuid, m->time, m->socketid, m->apicid,
m->microcode);


-Sohil

RE: [PATCH v4 2/2] tracing: Include Microcode Revision in mce_record tracepoint

2024-03-27 Thread Luck, Tony 
> Export microcode version through the tracepoint to prevent ambiguity over
> the active version on the system when the MCE was received.
>
> Signed-off-by: Avadhut Naik 
> Reviewed-by: Sohil Mehta 
> Reviewed-by: Steven Rostedt (Google) 

Reviewed-by: Tony Luck