linux-kernel - RE: [PATCH V2] x86/speculation: Support Enhanced IBRS on future CPUs

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: <alpine.DEB.2.21.1807311018040.1725@nanos.tec.linutronix.de>
Date:   Tue, 31 Jul 2018 10:24:03 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@...el.com>
cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "Chen, Tim C" <tim.c.chen@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        Ingo Molnar <mingo@...nel.org>
Subject: RE: [PATCH V2] x86/speculation: Support Enhanced IBRS on future
 CPUs

On Tue, 31 Jul 2018, Prakhya, Sai Praneeth wrote:
> > > +#define X86_FEATURE_IBRS_ENHANCED		( 7*32+29) /*
> > "ibrs_enhanced" Use Enhanced IBRS in kernel */
> > 
> > That "ibrs_enhanced" part is not needed.
> 
> Just wanted to confirm with you before removing it, 
> Presently, on platforms that support features like arch_capabilities, stibp, ibrs and ibpb 
> /proc/cpuinfo does show them. Do you think we should really skip showing 
> Enhanced IBRS capability?

The feature strings are automatically generated from the define. The
comment can be used to supress them by an empty "" string or to modify them
by a "override" string at the beginning of the comment.

> > > +	/*
> > > +	 * As we don't use IBRS in kernel, nobody should have set
> > > +	 * SPEC_CTRL_IBRS until now. Shout loud if somebody did enable
> > > +	 * SPEC_CTRL_IBRS before us.
> > > +	 */
> > 
> > This comment does not make sense. What prevents the BIOS/bootloader or the
> > hypervisor from setting it?
> >
> 
> Makes sense. Removed it from V3.
> 
> > > +	WARN_ON_ONCE(x86_spec_ctrl_base & SPEC_CTRL_IBRS);

Please remove the warnon as well.

> > > +	/* Ensure SPEC_CTRL_IBRS is set after VMEXIT from a guest */
> > > +	x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
> > 
> > And what exactly writes the MSR?
> >
> 
> While booting, x86_spec_ctrl_setup_ap() does that and after VMEXIT 
> x86_spec_ctrl_restore_host().
> 
> As x86_spec_ctrl_setup_ap() does wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base), 
> I thought writing here would be redundant.

x86_spec_ctrl_setup_ap() is only called on the AP but not on the BP. So the
boot processor will not have it set, unless something else writes the
MSR. So you really want to have an explicit write there.

Thanks,

	tglx