| Message ID | 034c7f5ac243ee7b40ba1a8cc3f9b10b1e380674.1663025154.git.pawan.kumar.gupta@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Check enumeration before MSR save/restore | expand |
On Mon, Sep 12, 2022 at 04:40:47PM -0700, Pawan Gupta wrote: > Currently there is no easy way to enumerate MSR_AMD64_LS_CFG. As this > MSR is supported on AMD CPU families 10h to 18h, set a new feature bit > on these CPU families. The new bit can be used to detect the MSR > support. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > --- > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/kernel/cpu/amd.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h > index dd173733e40d..90bdb1d98531 100644 > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -305,6 +305,7 @@ > #define X86_FEATURE_USE_IBPB_FW (11*32+16) /* "" Use IBPB during runtime firmware calls */ > #define X86_FEATURE_RSB_VMEXIT_LITE (11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */ > #define X86_FEATURE_MSR_TSX_CTRL (11*32+18) /* "" MSR IA32_TSX_CTRL */ > +#define X86_FEATURE_MSR_LS_CFG (11*32+19) /* "" MSR AMD64_LS_CFG */ We already have that one: #define X86_FEATURE_LS_CFG_SSBD ( 7*32+24) /* "" AMD SSBD implementation via LS_CFG MSR */
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?
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 Looks like we need to restore this MSR too, and we can use existing X86_FEATURE_XMM2 to enumerate it. 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.
On Wed, Nov 09, 2022 at 09:37:20AM -0800, Pawan Gupta wrote: > Looks like we need to restore this MSR too, Why do we? Is that MSR read-only too or what is the reason for that one? > and we can use existing X86_FEATURE_XMM2 to enumerate it. Or 86_FEATURE_LFENCE_RDTSC. > 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. Yes, MSR_AMD64_LS_CFG is used in SSBD mitigations. For everything <= 0x12: /* AMD Family 0xf - 0x12 */ VULNWL_AMD(0x0f, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), VULNWL_AMD(0x10, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), VULNWL_AMD(0x11, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), VULNWL_AMD(0x12, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), On F14h it says here: [ 0.278930] Speculative Store Bypass: Vulnerable simply because it is not present there.
On Wed, Nov 09, 2022 at 07:34:55PM +0100, Borislav Petkov wrote: >On Wed, Nov 09, 2022 at 09:37:20AM -0800, Pawan Gupta wrote: >> Looks like we need to restore this MSR too, > >Why do we? Is that MSR read-only too or what is the reason for that one? Please ignore, it doesn't concern this series. >Yes, MSR_AMD64_LS_CFG is used in SSBD mitigations. For everything <= 0x12: > > /* AMD Family 0xf - 0x12 */ > VULNWL_AMD(0x0f, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), > VULNWL_AMD(0x10, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), > VULNWL_AMD(0x11, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), > VULNWL_AMD(0x12, NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO), > >On F14h it says here: > >[ 0.278930] Speculative Store Bypass: Vulnerable > >simply because it is not present there. So looks like MSR_AMD64_LS_CFG should only be restored when X86_FEATURE_LS_CFG_SSBD is present. I will make this change in v2.
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
On Thu, Nov 10, 2022 at 12:00:11AM +0000, Andrew Cooper wrote: >On 09/11/2022 17:37, Pawan Gupta wrote: >> 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. Upstream currently saves and restores the whole MSR_AMD64_LS_CFG at suspend/resume. Is it that the configuration saved at suspend(other than SSBD) will not be relevant at resume for this MSR? Is there any harm if keep the current behavior?
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index dd173733e40d..90bdb1d98531 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -305,6 +305,7 @@ #define X86_FEATURE_USE_IBPB_FW (11*32+16) /* "" Use IBPB during runtime firmware calls */ #define X86_FEATURE_RSB_VMEXIT_LITE (11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */ #define X86_FEATURE_MSR_TSX_CTRL (11*32+18) /* "" MSR IA32_TSX_CTRL */ +#define X86_FEATURE_MSR_LS_CFG (11*32+19) /* "" MSR AMD64_LS_CFG */ /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */ #define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */ diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 48276c0e479d..88f59d630826 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -521,6 +521,9 @@ static void bsp_init_amd(struct cpuinfo_x86 *c) __max_die_per_package = nodes_per_socket = ((value >> 3) & 7) + 1; } + if (c->x86 >= 0x10 && c->x86 <= 0x18) + setup_force_cpu_cap(X86_FEATURE_MSR_LS_CFG); + if (!boot_cpu_has(X86_FEATURE_AMD_SSBD) && !boot_cpu_has(X86_FEATURE_VIRT_SSBD) && c->x86 >= 0x15 && c->x86 <= 0x17) {
Currently there is no easy way to enumerate MSR_AMD64_LS_CFG. As this MSR is supported on AMD CPU families 10h to 18h, set a new feature bit on these CPU families. The new bit can be used to detect the MSR support. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> --- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/kernel/cpu/amd.c | 3 +++ 2 files changed, 4 insertions(+)