linux-kernel - RE: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader

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, 29 Apr 2015 00:24:40 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	Borislav Petkov <bp@...en8.de>
CC:	linux-edac <linux-edac@...r.kernel.org>,
	Jiri Kosina <jkosina@...e.cz>, Borislav Petkov <bp@...e.de>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"Len Brown" <lenb@...nel.org>, "Luck, Tony" <tony.luck@...el.com>,
	Tomasz Nowicki <tomasz.nowicki@...aro.org>,
	"Chen, Gong" <gong.chen@...ux.intel.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader

Hi,

> From: Borislav Petkov [mailto:bp@...en8.de]
> Sent: Tuesday, April 28, 2015 9:59 PM
> Subject: Re: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader
> 
> On Tue, Apr 28, 2015 at 01:38:41PM +0000, Zheng, Lv wrote:
> > > -	raw_spin_lock(&ghes_nmi_lock);
> > > +	if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
> > > +		return ret;
> > > +
> >
> > if (atomic_cmpxchg(&ghes_in_nmi, 0, 1))
> > 	return ret;
> 
> Ok, now I understand what you mean.
> 
> We absolutely want to use atomic_add_unless() because we get to save us
> the expensive
> 
> 	LOCK; CMPXCHG
> 
> if the value was already 1. Which is exactly what this patch is trying
> to avoid - a thundering herd of cores CMPXCHGing a global variable.

IMO, on most architectures, the "cmp" part should work just like what you've done with "if".
And on some architectures, if the "xchg" doesn't happen, the "cmp" part even won't cause a pipe line hazard.

Thanks and best regards
-Lv


> 
> I.e.,
> 
> 	movl	ghes_in_nmi(%rip), %ecx	# MEM[(const int *)&ghes_in_nmi], c
> 	cmpl	$1, %ecx	#, c
> 	je	.L311	#,				<--- exit here if ghes_in_nmi == 1.
> 	leal	1(%rcx), %edx	#, D.37163
> 	movl	%ecx, %eax	# c, c
> #APP
> # 177 "./arch/x86/include/asm/atomic.h" 1
> 	.pushsection .smp_locks,"a"
> .balign 4
> .long 671f - .
> .popsection
> 671:
> 	lock; cmpxchgl %edx,ghes_in_nmi(%rip)	# D.37163, MEM[(volatile u32 *)&ghes_in_nmi]
> # 0 "" 2
> #NO_APP
> 
> --
> Regards/Gruss,
>     Boris.
> 
> ECO tip #101: Trim your mails when you reply.
> --