Re: [PATCH 2/3] x86/cpu/amd: Add feature bit for MSR_AMD64_LS_CFG enumeration

From: Andrew Cooper <Andrew.Cooper3@citrix.com>
To: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Pavel Machek <pavel@ucw.cz>,
	"hdegoede@redhat.com" <hdegoede@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Daniel Sneddon <daniel.sneddon@linux.intel.com>,
	"antonio.gomez.iglesias@linux.intel.com" 
	<antonio.gomez.iglesias@linux.intel.com>
Subject: Re: [PATCH 2/3] x86/cpu/amd: Add feature bit for MSR_AMD64_LS_CFG enumeration
Date: Thu, 10 Nov 2022 00:00:11 +0000	[thread overview]
Message-ID: <bee087ff-d7f7-4f6a-4c43-8c91fdec043a@citrix.com> (raw)
In-Reply-To: <20221109173720.4ovtb2ao3vuuge43@desk>

On 09/11/2022 17:37, Pawan Gupta wrote:
> On Wed, Nov 09, 2022 at 12:45:58AM +0000, Andrew Cooper wrote:
>> On 08/11/2022 23:10, Borislav Petkov wrote:
>>> On Tue, Nov 08, 2022 at 02:51:41PM -0800, Pawan Gupta wrote:
>>>> Looking at bsp_init_amd() this feature bit will only be set on AMD
>>>> families 0x15-0x17. Andrew mentioned that the MSR LS_CFG is present on
>>>> AMD family >= 0x10 && family <= 0x18.
>>> Do you need to save that MSR on those families?
>>>
>>> Or do 0x15-0x18 suffice?
>>>
>>> Yes, 0x18 because that's Hygon and that does its own detection.
>>>
>>> So, do you need to save it on families 0x10-0x14?
>>
>> https://www.amd.com/system/files/documents/software-techniques-for-managing-speculation.pdf 
>>
>> Mitigation G-2.
>>
>> The MSR exists on Fam 10/12/14/15/16/17, and in all cases the
>> LFENCE_DISPATCH bit wants setting if not already set.
>
> Isn't that a different MSR:
>
>   #define MSR_AMD64_LS_CFG                0xc0011020
>
>   #define MSR_F10H_DECFG                  0xc0011029
>   #define MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT     1

Oh yes.  You're absolutely correct.  I did get the two mixed up.

Funnily enough, the lfence dispatch properties are in the Decode/Execute
configuration MSR, while the knob for Speculative Store Bypass is in the
Load/Store configuration MSR.

> Looks like we need to restore this MSR too, and we can use existing
> X86_FEATURE_XMM2 to enumerate it.

In this case, I wouldn't say so.

For lfence dispatch, there are no user options.  The bit needs setting,
and you don't care about preserving the old value.

In fact, AMD retroactively declared bit is architectural, and it's
fixed-1 in Fam19h and later so you can't even accidentally turn off
speculation protections.  (Actually, so a malicious hypervisor can't
turn off speculation protections behind the back of an encrypted VM.)

>
> If SSBD is the only reason to restore MSR_AMD64_LS_CFG then we should be
> able to use X86_FEATURE_LS_CFG_SSBD for enumeration.

Yeah, although it occurs to me that you probably don't want to
save/restore the whole MSR.  It's full of other things which want to
stay in their current configuration.

~Andrew