[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f5657d4a-aa15-9602-bb36-1a3cfe7fbcc1@amd.com>
Date: Mon, 10 Jul 2017 14:41:51 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Brian Gerst <brgerst@...il.com>
Cc: linux-arch <linux-arch@...r.kernel.org>, linux-efi@...r.kernel.org,
kvm@...r.kernel.org, linux-doc@...r.kernel.org,
the arch/x86 maintainers <x86@...nel.org>,
kexec@...ts.infradead.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
kasan-dev@...glegroups.com, xen-devel@...ts.xen.org,
Linux-MM <linux-mm@...ck.org>, iommu@...ts.linux-foundation.org,
Brijesh Singh <brijesh.singh@....com>,
Toshimitsu Kani <toshi.kani@....com>,
Radim Krčmář <rkrcmar@...hat.com>,
Matt Fleming <matt@...eblueprint.co.uk>,
Alexander Potapenko <glider@...gle.com>,
"H. Peter Anvin" <hpa@...or.com>,
Larry Woodman <lwoodman@...hat.com>,
Jonathan Corbet <corbet@....net>,
Joerg Roedel <joro@...tes.org>,
"Michael S. Tsirkin" <mst@...hat.com>,
Ingo Molnar <mingo@...hat.com>,
Andrey Ryabinin <aryabinin@...tuozzo.com>,
Dave Young <dyoung@...hat.com>, Rik van Riel <riel@...hat.com>,
Arnd Bergmann <arnd@...db.de>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Borislav Petkov <bp@...en8.de>,
Andy Lutomirski <luto@...nel.org>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Juergen Gross <jgross@...e.com>,
Thomas Gleixner <tglx@...utronix.de>,
Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v9 04/38] x86/CPU/AMD: Add the Secure Memory Encryption
CPU feature
On 7/8/2017 7:50 AM, Brian Gerst wrote:
> On Fri, Jul 7, 2017 at 9:38 AM, Tom Lendacky <thomas.lendacky@....com> wrote:
>> Update the CPU features to include identifying and reporting on the
>> Secure Memory Encryption (SME) feature. SME is identified by CPUID
>> 0x8000001f, but requires BIOS support to enable it (set bit 23 of
>> MSR_K8_SYSCFG). Only show the SME feature as available if reported by
>> CPUID and enabled by BIOS.
>>
>> Reviewed-by: Borislav Petkov <bp@...e.de>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>> arch/x86/include/asm/cpufeatures.h | 1 +
>> arch/x86/include/asm/msr-index.h | 2 ++
>> arch/x86/kernel/cpu/amd.c | 13 +++++++++++++
>> arch/x86/kernel/cpu/scattered.c | 1 +
>> 4 files changed, 17 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 2701e5f..2b692df 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -196,6 +196,7 @@
>>
>> #define X86_FEATURE_HW_PSTATE ( 7*32+ 8) /* AMD HW-PState */
>> #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
>> +#define X86_FEATURE_SME ( 7*32+10) /* AMD Secure Memory Encryption */
>
> Given that this feature is available only in long mode, this should be
> added to disabled-features.h as disabled for 32-bit builds.
I can add that. If the series needs a re-spin then I'll include this
change in the series, otherwise I can send a follow-on patch to handle
the feature for 32-bit builds if that works.
>
>> #define X86_FEATURE_INTEL_PPIN ( 7*32+14) /* Intel Processor Inventory Number */
>> #define X86_FEATURE_INTEL_PT ( 7*32+15) /* Intel Processor Trace */
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index 18b1623..460ac01 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -352,6 +352,8 @@
>> #define MSR_K8_TOP_MEM1 0xc001001a
>> #define MSR_K8_TOP_MEM2 0xc001001d
>> #define MSR_K8_SYSCFG 0xc0010010
>> +#define MSR_K8_SYSCFG_MEM_ENCRYPT_BIT 23
>> +#define MSR_K8_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_K8_SYSCFG_MEM_ENCRYPT_BIT)
>> #define MSR_K8_INT_PENDING_MSG 0xc0010055
>> /* C1E active bits in int pending message */
>> #define K8_INTP_C1E_ACTIVE_MASK 0x18000000
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index bb5abe8..c47ceee 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -611,6 +611,19 @@ 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. If BIOS has not enabled SME
>> + * then don't advertise the feature (set in scattered.c)
>> + */
>> + 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))
>> + clear_cpu_cap(c, X86_FEATURE_SME);
>> + }
>
> This should be conditional on CONFIG_X86_64.
If I make the scattered feature support conditional on CONFIG_X86_64
(based on comment below) then cpu_has() will always be false unless
CONFIG_X86_64 is enabled. So this won't need to be wrapped by the
#ifdef.
>
>> }
>>
>> static void init_amd_k8(struct cpuinfo_x86 *c)
>> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
>> index 23c2350..05459ad 100644
>> --- a/arch/x86/kernel/cpu/scattered.c
>> +++ b/arch/x86/kernel/cpu/scattered.c
>> @@ -31,6 +31,7 @@ struct cpuid_bit {
>> { X86_FEATURE_HW_PSTATE, CPUID_EDX, 7, 0x80000007, 0 },
>> { X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 },
>> { X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 },
>> + { X86_FEATURE_SME, CPUID_EAX, 0, 0x8000001f, 0 },
>
> This should also be conditional. We don't want to set this feature on
> 32-bit, even if the processor has support.
Can do. See comment above about re-spin vs. follow-on patch.
Thanks,
Tom
>
>> { 0, 0, 0, 0, 0 }
>> };
>
> --
> Brian Gerst
>
Powered by blists - more mailing lists