linux-kernel - Re: [PATCH RFC] x86/cpu: fix intermittent lockup on poweroff

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:   Wed, 26 Apr 2023 22:00:33 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Tom Lendacky <thomas.lendacky@....com>,
        Tony Battersby <tonyb@...ernetics.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org
Cc:     "H. Peter Anvin" <hpa@...or.com>,
        Mario Limonciello <mario.limonciello@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH RFC] x86/cpu: fix intermittent lockup on poweroff

On Wed, Apr 26 2023 at 12:51, Tom Lendacky wrote:
> On 4/26/23 12:37, Tony Battersby wrote:
>>> +	/*
>>> +	 * native_stop_other_cpus() will write to @stop_cpus_count after
>>> +	 * observing that it went down to zero, which will invalidate the
>>> +	 * cacheline on this CPU.
>>> +	 */
>>> +	atomic_dec(&stop_cpus_count);
>
> This is probably going to pull in a cache line and cause the problem the 
> native_wbinvd() is trying to avoid.

The comment above this atomic_dec() explains why this is _not_ a
problem. Here is the counterpart in native_stop_other_cpus():

>>> @@ -216,6 +219,12 @@ static void native_stop_other_cpus(int w
>>>   	disable_local_APIC();
>>>   	mcheck_cpu_clear(this_cpu_ptr(&cpu_info));
>>>   	local_irq_restore(flags);
>>> +
>>> +	/*
>>> +	 * Ensure that the cache line is invalidated on the other CPUs. See
>>> +	 * comment vs. SME in stop_this_cpu().
>>> +	 */
>>> +	atomic_set(&stop_cpus_count, INT_MAX);

That happens _after_ all the other CPUs did the atomic_dec() as the
control CPU waits for it to become 0.

As this makes the cacheline exclusive on the control CPU the dirty
cacheline on the CPU which did the last atomic_dec() is invalidated.

As the atomic_dec() is obviously serialized via the lock prefix there
can be only one dirty copy on some other CPU at the time when the
control CPU writes to it.

After that the only dirty copy is on the control CPU, no?

Thanks,

        tglx