| Message ID | 20170418211727.10190.18774.stgit@tlendack-t1.amdoffice.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 18, 2017 at 04:17:27PM -0500, Tom Lendacky wrote: > Add support for Secure Memory Encryption (SME). This initial support > provides a Kconfig entry to build the SME support into the kernel and > defines the memory encryption mask that will be used in subsequent > patches to mark pages as encrypted. ... > diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h > new file mode 100644 > index 0000000..d5c4a2b > --- /dev/null > +++ b/arch/x86/include/asm/mem_encrypt.h > @@ -0,0 +1,42 @@ > +/* > + * AMD Memory Encryption Support > + * > + * Copyright (C) 2016 Advanced Micro Devices, Inc. > + * > + * Author: Tom Lendacky <thomas.lendacky@amd.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + These ifdeffery closing #endif markers look strange: > +#ifndef __X86_MEM_ENCRYPT_H__ > +#define __X86_MEM_ENCRYPT_H__ > + > +#ifndef __ASSEMBLY__ > + > +#ifdef CONFIG_AMD_MEM_ENCRYPT > + > +extern unsigned long sme_me_mask; > + > +static inline bool sme_active(void) > +{ > + return !!sme_me_mask; > +} > + > +#else /* !CONFIG_AMD_MEM_ENCRYPT */ > + > +#ifndef sme_me_mask > +#define sme_me_mask 0UL > + > +static inline bool sme_active(void) > +{ > + return false; > +} > +#endif this endif is the sme_me_mask closing one and it has sme_active() in it. Shouldn't it be: #ifndef sme_me_mask #define sme_me_mask 0UL #endif and have sme_active below it, in the !CONFIG_AMD_MEM_ENCRYPT branch? The same thing is in include/linux/mem_encrypt.h
On 4/27/2017 10:46 AM, Borislav Petkov wrote: > On Tue, Apr 18, 2017 at 04:17:27PM -0500, Tom Lendacky wrote: >> Add support for Secure Memory Encryption (SME). This initial support >> provides a Kconfig entry to build the SME support into the kernel and >> defines the memory encryption mask that will be used in subsequent >> patches to mark pages as encrypted. > > ... > >> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h >> new file mode 100644 >> index 0000000..d5c4a2b >> --- /dev/null >> +++ b/arch/x86/include/asm/mem_encrypt.h >> @@ -0,0 +1,42 @@ >> +/* >> + * AMD Memory Encryption Support >> + * >> + * Copyright (C) 2016 Advanced Micro Devices, Inc. >> + * >> + * Author: Tom Lendacky <thomas.lendacky@amd.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + > > These ifdeffery closing #endif markers look strange: > >> +#ifndef __X86_MEM_ENCRYPT_H__ >> +#define __X86_MEM_ENCRYPT_H__ >> + >> +#ifndef __ASSEMBLY__ >> + >> +#ifdef CONFIG_AMD_MEM_ENCRYPT >> + >> +extern unsigned long sme_me_mask; >> + >> +static inline bool sme_active(void) >> +{ >> + return !!sme_me_mask; >> +} >> + >> +#else /* !CONFIG_AMD_MEM_ENCRYPT */ >> + >> +#ifndef sme_me_mask >> +#define sme_me_mask 0UL >> + >> +static inline bool sme_active(void) >> +{ >> + return false; >> +} >> +#endif > > this endif is the sme_me_mask closing one and it has sme_active() in it. > Shouldn't it be: > > #ifndef sme_me_mask > #define sme_me_mask 0UL > #endif > > and have sme_active below it, in the !CONFIG_AMD_MEM_ENCRYPT branch? > > The same thing is in include/linux/mem_encrypt.h I did this so that an the include order wouldn't cause issues (including asm/mem_encrypt.h followed by later by a linux/mem_encrypt.h include). I can make this a bit clearer by having separate #defines for each thing, e.g.: #ifndef sme_me_mask #define sme_me_mask 0UL #endif #ifndef sme_active #define sme_active sme_active static inline ... #endif Is that better/clearer? Thanks, Tom >
On Thu, May 04, 2017 at 09:24:11AM -0500, Tom Lendacky wrote: > I did this so that an the include order wouldn't cause issues (including > asm/mem_encrypt.h followed by later by a linux/mem_encrypt.h include). > I can make this a bit clearer by having separate #defines for each > thing, e.g.: > > #ifndef sme_me_mask > #define sme_me_mask 0UL > #endif > > #ifndef sme_active > #define sme_active sme_active > static inline ... > #endif > > Is that better/clearer? I guess but where do we have to include both the asm/ and the linux/ version? IOW, can we avoid these issues altogether by partitioning symbol declarations differently among the headers?
On 5/4/2017 9:36 AM, Borislav Petkov wrote: > On Thu, May 04, 2017 at 09:24:11AM -0500, Tom Lendacky wrote: >> I did this so that an the include order wouldn't cause issues (including >> asm/mem_encrypt.h followed by later by a linux/mem_encrypt.h include). >> I can make this a bit clearer by having separate #defines for each >> thing, e.g.: >> >> #ifndef sme_me_mask >> #define sme_me_mask 0UL >> #endif >> >> #ifndef sme_active >> #define sme_active sme_active >> static inline ... >> #endif >> >> Is that better/clearer? > > I guess but where do we have to include both the asm/ and the linux/ > version? It's more of the sequence of various includes. For example, init/do_mounts.c includes <linux/module.h> that eventually gets down to <asm/pgtable_types.h> and then <asm/mem_encrypt.h>. However, a bit further down <linux/nfs_fs.h> is included which eventually gets down to <linux/dma-mapping.h> and then <linux/mem_encyrpt.h>. > > IOW, can we avoid these issues altogether by partitioning symbol > declarations differently among the headers? It's most problematic when CONFIG_AMD_MEM_ENCRYPT is not defined since we never include an asm/ version from the linux/ path. I could create a mem_encrypt.h in include/asm-generic/ that contains the info that is in the !CONFIG_AMD_MEM_ENCRYPT path of the linux/ version. Let me look into that. Thanks, Tom >
On Tue, May 16, 2017 at 02:28:42PM -0500, Tom Lendacky wrote: > It's most problematic when CONFIG_AMD_MEM_ENCRYPT is not defined since > we never include an asm/ version from the linux/ path. I could create > a mem_encrypt.h in include/asm-generic/ that contains the info that > is in the !CONFIG_AMD_MEM_ENCRYPT path of the linux/ version. Let me > look into that. So we need to keep asm/ and linux/ apart. The linux/ stuff is generic, global, more or less. The asm/ is arch-specific. So they shouldn't be overlapping wrt definitions, IMHO. So asm-generic is the proper approach here because then you won't need the ifndef fun. Thanks.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 4e153e9..cf0cbe8 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1407,6 +1407,28 @@ config X86_DIRECT_GBPAGES supports them), so don't confuse the user by printing that we have them enabled. +config AMD_MEM_ENCRYPT + bool "AMD Secure Memory Encryption (SME) support" + depends on X86_64 && CPU_SUP_AMD + ---help--- + Say yes to enable support for the encryption of system memory. + This requires an AMD processor that supports Secure Memory + Encryption (SME). + +config AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT + bool "Activate AMD Secure Memory Encryption (SME) by default" + default y + depends on AMD_MEM_ENCRYPT + ---help--- + Say yes to have system memory encrypted by default if running on + an AMD processor that supports Secure Memory Encryption (SME). + + If set to Y, then the encryption of system memory can be + deactivated with the mem_encrypt=off command line option. + + If set to N, then the encryption of system memory can be + activated with the mem_encrypt=on command line option. + # Common NUMA Features config NUMA bool "Numa Memory Allocation and Scheduler Support" diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h new file mode 100644 index 0000000..d5c4a2b --- /dev/null +++ b/arch/x86/include/asm/mem_encrypt.h @@ -0,0 +1,42 @@ +/* + * AMD Memory Encryption Support + * + * Copyright (C) 2016 Advanced Micro Devices, Inc. + * + * Author: Tom Lendacky <thomas.lendacky@amd.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __X86_MEM_ENCRYPT_H__ +#define __X86_MEM_ENCRYPT_H__ + +#ifndef __ASSEMBLY__ + +#ifdef CONFIG_AMD_MEM_ENCRYPT + +extern unsigned long sme_me_mask; + +static inline bool sme_active(void) +{ + return !!sme_me_mask; +} + +#else /* !CONFIG_AMD_MEM_ENCRYPT */ + +#ifndef sme_me_mask +#define sme_me_mask 0UL + +static inline bool sme_active(void) +{ + return false; +} +#endif + +#endif /* CONFIG_AMD_MEM_ENCRYPT */ + +#endif /* __ASSEMBLY__ */ + +#endif /* __X86_MEM_ENCRYPT_H__ */ diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile index 0fbdcb6..a94a7b6 100644 --- a/arch/x86/mm/Makefile +++ b/arch/x86/mm/Makefile @@ -39,3 +39,4 @@ obj-$(CONFIG_X86_INTEL_MPX) += mpx.o obj-$(CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS) += pkeys.o obj-$(CONFIG_RANDOMIZE_MEMORY) += kaslr.o +obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt.o diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c new file mode 100644 index 0000000..b99d469 --- /dev/null +++ b/arch/x86/mm/mem_encrypt.c @@ -0,0 +1,21 @@ +/* + * AMD Memory Encryption Support + * + * Copyright (C) 2016 Advanced Micro Devices, Inc. + * + * Author: Tom Lendacky <thomas.lendacky@amd.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/linkage.h> + +/* + * Since SME related variables are set early in the boot process they must + * reside in the .data section so as not to be zeroed out when the .bss + * section is later cleared. + */ +unsigned long sme_me_mask __section(.data) = 0; +EXPORT_SYMBOL_GPL(sme_me_mask); diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h new file mode 100644 index 0000000..14a7b9f --- /dev/null +++ b/include/linux/mem_encrypt.h @@ -0,0 +1,37 @@ +/* + * AMD Memory Encryption Support + * + * Copyright (C) 2016 Advanced Micro Devices, Inc. + * + * Author: Tom Lendacky <thomas.lendacky@amd.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __MEM_ENCRYPT_H__ +#define __MEM_ENCRYPT_H__ + +#ifndef __ASSEMBLY__ + +#ifdef CONFIG_AMD_MEM_ENCRYPT + +#include <asm/mem_encrypt.h> + +#else /* !CONFIG_AMD_MEM_ENCRYPT */ + +#ifndef sme_me_mask +#define sme_me_mask 0UL + +static inline bool sme_active(void) +{ + return false; +} +#endif + +#endif /* CONFIG_AMD_MEM_ENCRYPT */ + +#endif /* __ASSEMBLY__ */ + +#endif /* __MEM_ENCRYPT_H__ */
Add support for Secure Memory Encryption (SME). This initial support provides a Kconfig entry to build the SME support into the kernel and defines the memory encryption mask that will be used in subsequent patches to mark pages as encrypted. Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> --- arch/x86/Kconfig | 22 +++++++++++++++++++ arch/x86/include/asm/mem_encrypt.h | 42 ++++++++++++++++++++++++++++++++++++ arch/x86/mm/Makefile | 1 + arch/x86/mm/mem_encrypt.c | 21 ++++++++++++++++++ include/linux/mem_encrypt.h | 37 ++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+) create mode 100644 arch/x86/include/asm/mem_encrypt.h create mode 100644 arch/x86/mm/mem_encrypt.c create mode 100644 include/linux/mem_encrypt.h