linux-kernel - Re: [PATCH 12/22] x86/fpu: Only write PKRU if it is different from current

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]

Message-ID: <20190207112748.kzjd3cagqgecamp7@linutronix.de>
Date:   Thu, 7 Feb 2019 12:27:49 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        kvm@...r.kernel.org, "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...riel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 12/22] x86/fpu: Only write PKRU if it is different from
 current

On 2019-01-23 10:09:24 [-0800], Dave Hansen wrote:
> On 1/9/19 3:47 AM, Sebastian Andrzej Siewior wrote:
> > +static inline void __write_pkru(u32 pkru)
> > +{
> > +	/*
> > +	 * Writting PKRU is expensive. Only write the PKRU value if it is
> > +	 * different from the current one.
> > +	 */
> 
> I'd say:
> 
> 	WRPKRU is relatively expensive compared to RDPKRU.
> 	Avoid WRPKRU when it would not change the value.
> 
> In the grand scheme of things, WRPKRU is cheap.  It's certainly not an
> "expensive instruction" compared to things like WBINVD.

Okay.

> > +	if (pkru == __read_pkru())
> > +		return;
> > +	__write_pkru_insn(pkru);
> > +}
> 
> Is there a case where we need __write_pkru_insn() directly?  Why not
> just put the inline assembly in here?

There is no user of __write_pkru_insn(). I had one in the past I think.
Let me merge it for now.

Sebastian