From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0054.outbound.protection.outlook.com [104.47.34.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3xH0xz3tT5zDqmj for ; Wed, 26 Jul 2017 00:29:57 +1000 (AEST) Subject: Re: [RFC Part1 PATCH v3 02/17] x86/CPU/AMD: Add the Secure Encrypted Virtualization CPU feature To: Borislav Petkov , Brijesh Singh Cc: linux-kernel@vger.kernel.org, x86@kernel.org, linux-efi@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , Andy Lutomirski , Tony Luck , Piotr Luc , Fenghua Yu , Lu Baolu , Reza Arbab , David Howells , Matt Fleming , "Kirill A . Shutemov" , Laura Abbott , Ard Biesheuvel , Andrew Morton , Eric Biederman , Benjamin Herrenschmidt , Paul Mackerras , Konrad Rzeszutek Wilk , Jonathan Corbet , Dave Airlie , Kees Cook , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Arnd Bergmann , Tejun Heo , Christoph Lameter References: <20170724190757.11278-1-brijesh.singh@amd.com> <20170724190757.11278-3-brijesh.singh@amd.com> <20170725102657.GD21822@nazgul.tnic> From: Tom Lendacky Message-ID: <7236d267-ebcb-8b45-b8d3-5955903e395f@amd.com> Date: Tue, 25 Jul 2017 09:29:40 -0500 MIME-Version: 1.0 In-Reply-To: <20170725102657.GD21822@nazgul.tnic> Content-Type: text/plain; charset=utf-8; format=flowed List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 7/25/2017 5:26 AM, Borislav Petkov wrote: > On Mon, Jul 24, 2017 at 02:07:42PM -0500, Brijesh Singh wrote: >> From: Tom Lendacky >> >> Update the CPU features to include identifying and reporting on the >> Secure Encrypted Virtualization (SEV) feature. SME is identified by >> CPUID 0x8000001f, but requires BIOS support to enable it (set bit 23 of >> MSR_K8_SYSCFG and set bit 0 of MSR_K7_HWCR). Only show the SEV feature >> as available if reported by CPUID and enabled by BIOS. >> >> Signed-off-by: Tom Lendacky >> Signed-off-by: Brijesh Singh >> --- >> arch/x86/include/asm/cpufeatures.h | 1 + >> arch/x86/include/asm/msr-index.h | 2 ++ >> arch/x86/kernel/cpu/amd.c | 30 +++++++++++++++++++++++++----- >> arch/x86/kernel/cpu/scattered.c | 1 + >> 4 files changed, 29 insertions(+), 5 deletions(-) > > ... > >> @@ -637,6 +642,21 @@ static void early_init_amd(struct cpuinfo_x86 *c) >> clear_cpu_cap(c, X86_FEATURE_SME); >> } >> } >> + >> + if (cpu_has(c, X86_FEATURE_SEV)) { >> + if (IS_ENABLED(CONFIG_X86_32)) { >> + clear_cpu_cap(c, X86_FEATURE_SEV); >> + } else { >> + u64 syscfg, hwcr; >> + >> + /* Check if SEV is enabled */ >> + rdmsrl(MSR_K8_SYSCFG, syscfg); >> + rdmsrl(MSR_K7_HWCR, hwcr); >> + if (!(syscfg & MSR_K8_SYSCFG_MEM_ENCRYPT) || >> + !(hwcr & MSR_K7_HWCR_SMMLOCK)) >> + clear_cpu_cap(c, X86_FEATURE_SEV); >> + } >> + } > > Let's simplify this and read the MSRs only once. Diff ontop. Please > check if I'm missing a case: Yup, we can do something like that. I believe the only change that would be needed to your patch would be to move the IS_ENABLED() check to after the physical address space reduction check. Thanks, Tom > > --- > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index c413f04bdd41..79af07731ab1 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -546,6 +546,48 @@ static void bsp_init_amd(struct cpuinfo_x86 *c) > } > } > > +static void early_detect_mem_enc(struct cpuinfo_x86 *c) > +{ > + u64 syscfg, hwcr; > + > + /* > + * BIOS support is required for SME and SEV. > + * For SME: If BIOS has enabled SME then adjust x86_phys_bits by > + * the SME physical address space reduction value. > + * If BIOS has not enabled SME then don't advertise the > + * SME feature (set in scattered.c). > + * For SEV: If BIOS has not enabled SEV then don't advertise the > + * SEV feature (set in scattered.c). > + * > + * In all cases, since support for SME and SEV requires long mode, > + * don't advertise the feature under CONFIG_X86_32. > + */ > + if (cpu_has(c, X86_FEATURE_SME) || > + cpu_has(c, X86_FEATURE_SEV)) { > + > + if (IS_ENABLED(CONFIG_X86_32)) > + goto clear; > + > + /* Check if SME is enabled */ > + rdmsrl(MSR_K8_SYSCFG, syscfg); > + if (!(syscfg & MSR_K8_SYSCFG_MEM_ENCRYPT)) > + goto clear; > + > + c->x86_phys_bits -= (cpuid_ebx(0x8000001f) >> 6) & 0x3f; > + > + /* Check if SEV is enabled */ > + rdmsrl(MSR_K7_HWCR, hwcr); > + if (!(hwcr & MSR_K7_HWCR_SMMLOCK)) > + goto clear_sev; > + > + return; > +clear: > + clear_cpu_cap(c, X86_FEATURE_SME); > +clear_sev: > + clear_cpu_cap(c, X86_FEATURE_SEV); > + } > +} > + > static void early_init_amd(struct cpuinfo_x86 *c) > { > u32 dummy; > @@ -617,46 +659,8 @@ static void early_init_amd(struct cpuinfo_x86 *c) > if (cpu_has_amd_erratum(c, amd_erratum_400)) > set_cpu_bug(c, X86_BUG_AMD_E400); > > - /* > - * BIOS support is required for SME and SEV. > - * For SME: If BIOS has enabled SME then adjust x86_phys_bits by > - * the SME physical address space reduction value. > - * If BIOS has not enabled SME then don't advertise the > - * SME feature (set in scattered.c). > - * For SEV: If BIOS has not enabled SEV then don't advertise the > - * SEV feature (set in scattered.c). > - * > - * In all cases, since support for SME and SEV requires long mode, > - * don't advertise the feature under CONFIG_X86_32. > - */ > - if (cpu_has(c, X86_FEATURE_SME)) { > - u64 msr; > - > - /* Check if SME is enabled */ > - rdmsrl(MSR_K8_SYSCFG, msr); > - if (msr & MSR_K8_SYSCFG_MEM_ENCRYPT) { > - c->x86_phys_bits -= (cpuid_ebx(0x8000001f) >> 6) & 0x3f; > - if (IS_ENABLED(CONFIG_X86_32)) > - clear_cpu_cap(c, X86_FEATURE_SME); > - } else { > - clear_cpu_cap(c, X86_FEATURE_SME); > - } > - } > + early_detect_mem_enc(c); > > - if (cpu_has(c, X86_FEATURE_SEV)) { > - if (IS_ENABLED(CONFIG_X86_32)) { > - clear_cpu_cap(c, X86_FEATURE_SEV); > - } else { > - u64 syscfg, hwcr; > - > - /* Check if SEV is enabled */ > - rdmsrl(MSR_K8_SYSCFG, syscfg); > - rdmsrl(MSR_K7_HWCR, hwcr); > - if (!(syscfg & MSR_K8_SYSCFG_MEM_ENCRYPT) || > - !(hwcr & MSR_K7_HWCR_SMMLOCK)) > - clear_cpu_cap(c, X86_FEATURE_SEV); > - } > - } > } > > static void init_amd_k8(struct cpuinfo_x86 *c) >