linux-kernel - Re: [PATCH v2 07/16] x86/mce/amd: Simplify DFR handler setup

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] [thread-next>] [day] [month] [year] [list]

Date: Mon, 29 Apr 2024 10:25:02 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: yazen.ghannam@....com, robert.richter@....com,
 linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
 tony.luck@...el.com, x86@...nel.org, Avadhut.Naik@....com, John.Allen@....com
Subject: Re: [PATCH v2 07/16] x86/mce/amd: Simplify DFR handler setup

On 4/29/2024 10:12 AM, Borislav Petkov wrote:
> On Mon, Apr 29, 2024 at 09:56:56AM -0400, Yazen Ghannam wrote:
>> Right, I mean we should do things the simpler way unless there's a real issue
>> to address.
> 
> You need to pay attention to past issues before you go, simplify it and
> break it again.
> 

I completely agree. I haven't seen evidence of an issue yet for the DFR case
though. Which is why I thought it'd be safe to do some clean up.

>> I'm not opposed to this, but I don't understand what is at risk.
>>
>> Is it that the function pointer may not be written atomically? So even if we
>> write it again with the same value, a concurrent interrupt on another core may
>> see a partially updated (corrupt) pointer?
> 
> Yes, it won't happen, they say as it is guaranteed by the
> architecture. But I've heard those "promises".
> 
>> intel_init_cmci() does not do this check. So is it more at risk, or is the AMD
>> code just more cautious?
>>
>> Again I'm not against the current code. I just think we should simplify it, if
>> possible.
> 
> So in looking at the INTR_CFG MSR, I think we should do a function which
> does MCA init stuff only on the BSP exactly for things like that.
> 
> There you can set the interrupt handler pointer, the INTR_CFG MSR and so
> on. And we don't have such function and I've needed a function like that
> in the past.
> 
> And just for the general goal of not doing ugly code which should run
> only once but is run per-CPU just because our infrastructure doesn't
> allow it.
> 
> Wanna give that a try?
> 
> Thx.
> 

Yep, "MCA init cleanup" is another thing on my TODO list.

The BSP still completely finishes init before the APs, correct? I recall some
effort to make CPU init more parallel, but I haven't been following it.

Thanks,
Yazen