Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 11 Jan 2023 23:00:37 -0800
From:   Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
To:     Borislav Petkov <bp@alien8.de>
Cc:     Breno Leitao <leitao@debian.org>, tglx@linutronix.de,
        mingo@redhat.com, dave.hansen@linux.intel.com, hpa@zytor.com,
        jpoimboe@kernel.org, peterz@infradead.org, x86@kernel.org,
        cascardo@canonical.com, leit@meta.com, kexec@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits
Message-ID: <20230112070037.q6cg23tn57onmxfi@desk>
References: <20221128153148.1129350-1-leitao@debian.org>
 <Y76wtwWN1+XPbnQx@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y76wtwWN1+XPbnQx@zn.tnic>
Precedence: bulk

On Wed, Jan 11, 2023 at 01:51:03PM +0100, Borislav Petkov wrote:
> On Mon, Nov 28, 2022 at 07:31:48AM -0800, Breno Leitao wrote:
> > Currently x86_spec_ctrl_base is read at boot time, and speculative bits
> > are set if configs are enable, such as MSR[SPEC_CTRL_IBRS] is enabled if
> > CONFIG_CPU_IBRS_ENTRY is configured. These MSR bits are not cleared if
> > the mitigations are disabled.
> > 
> > This is a problem when kexec-ing a kernel that has the mitigation
> > disabled, from a kernel that has the mitigation enabled. In this case,
> > the MSR bits are carried forward and not cleared at the boot of the new
> > kernel. This might have some performance degradation that is hard to
> > find.
> > 
> > This problem does not happen if the machine is (hard) rebooted, because
> > the bit will be cleared by default.
> > 
> > Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> >  arch/x86/include/asm/msr-index.h |  4 ++++
> >  arch/x86/kernel/cpu/bugs.c       | 10 +++++++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index 4a2af82553e4..22986a8f18bc 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -54,6 +54,10 @@
> >  #define SPEC_CTRL_RRSBA_DIS_S_SHIFT	6	   /* Disable RRSBA behavior */
> >  #define SPEC_CTRL_RRSBA_DIS_S		BIT(SPEC_CTRL_RRSBA_DIS_S_SHIFT)
> >  
> > +/* A mask for bits which the kernel toggles when controlling mitigations */
> > +#define SPEC_CTRL_MITIGATIONS_MASK	(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD \
> > +							| SPEC_CTRL_RRSBA_DIS_S)
> 
> SPEC_CTRL_RRSBA_DIS_S is a disable bit and I presume it needs to stay enabled.

The mitigation is enabled when this bit is set. When set, it prevents RET
target to be predicted from alternate predictors (BTB). This should stay
0, unless enabled by a mitigation mode.

> Only when spec_ctrl_disable_kernel_rrsba() runs. And I'd say perf-wise it
> doesn't cost that much...

I guess this doesn't matter now, because this patch is resetting it by
default that keeps the mitigation disabled with no perf impact.