diff mbox

[RFC,v3,20/20] x86: Add support to make use of Secure Memory Encryption

Message ID 20161110003838.3280.23327.stgit@tlendack-t1.amdoffice.net (mailing list archive)
State New, archived
Headers show

Commit Message

Tom Lendacky Nov. 10, 2016, 12:38 a.m. UTC 
This patch adds the support to check if SME has been enabled and if the
mem_encrypt=on command line option is set. If both of these conditions
are true, then the encryption mask is set and the kernel is encrypted
"in place."

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/head_64.S          |    1 +
 arch/x86/kernel/mem_encrypt_init.c |   60 +++++++++++++++++++++++++++++++++++-
 arch/x86/mm/mem_encrypt.c          |    2 +
 3 files changed, 62 insertions(+), 1 deletion(-)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Borislav Petkov Nov. 22, 2016, 6:58 p.m. UTC | #1 
On Wed, Nov 09, 2016 at 06:38:38PM -0600, Tom Lendacky wrote:
> This patch adds the support to check if SME has been enabled and if the
> mem_encrypt=on command line option is set. If both of these conditions
> are true, then the encryption mask is set and the kernel is encrypted
> "in place."

Something went wrong here - 19/20 and 20/20 have the same Subject and
commit message.

Care to resend only 19 and 20 with the above fixed?

Thanks.
Borislav Petkov Nov. 26, 2016, 8:47 p.m. UTC | #2 
On Wed, Nov 09, 2016 at 06:38:38PM -0600, Tom Lendacky wrote:
> This patch adds the support to check if SME has been enabled and if the
> mem_encrypt=on command line option is set. If both of these conditions
> are true, then the encryption mask is set and the kernel is encrypted
> "in place."
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/kernel/head_64.S          |    1 +
>  arch/x86/kernel/mem_encrypt_init.c |   60 +++++++++++++++++++++++++++++++++++-
>  arch/x86/mm/mem_encrypt.c          |    2 +
>  3 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index e8a7272..c225433 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -100,6 +100,7 @@ startup_64:
>  	 * to include it in the page table fixups.
>  	 */
>  	push	%rsi
> +	movq	%rsi, %rdi
>  	call	sme_enable
>  	pop	%rsi
>  	movq	%rax, %r12
> diff --git a/arch/x86/kernel/mem_encrypt_init.c b/arch/x86/kernel/mem_encrypt_init.c
> index 7bdd159..c94ceb8 100644
> --- a/arch/x86/kernel/mem_encrypt_init.c
> +++ b/arch/x86/kernel/mem_encrypt_init.c
> @@ -16,9 +16,14 @@
>  #include <linux/mm.h>
>  
>  #include <asm/sections.h>
> +#include <asm/processor-flags.h>
> +#include <asm/msr.h>
> +#include <asm/cmdline.h>
>  
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  
> +static char sme_cmdline_arg[] __initdata = "mem_encrypt=on";

One more thing: just like we're adding an =on switch, we'd need an =off
switch in case something's wrong with the SME code. IOW, if a user
supplies "mem_encrypt=off", we do not encrypt.

Thanks.
Tom Lendacky Nov. 29, 2016, 6:48 p.m. UTC | #3 
On 11/26/2016 2:47 PM, Borislav Petkov wrote:
> On Wed, Nov 09, 2016 at 06:38:38PM -0600, Tom Lendacky wrote:
>> This patch adds the support to check if SME has been enabled and if the
>> mem_encrypt=on command line option is set. If both of these conditions
>> are true, then the encryption mask is set and the kernel is encrypted
>> "in place."
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  arch/x86/kernel/head_64.S          |    1 +
>>  arch/x86/kernel/mem_encrypt_init.c |   60 +++++++++++++++++++++++++++++++++++-
>>  arch/x86/mm/mem_encrypt.c          |    2 +
>>  3 files changed, 62 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>> index e8a7272..c225433 100644
>> --- a/arch/x86/kernel/head_64.S
>> +++ b/arch/x86/kernel/head_64.S
>> @@ -100,6 +100,7 @@ startup_64:
>>  	 * to include it in the page table fixups.
>>  	 */
>>  	push	%rsi
>> +	movq	%rsi, %rdi
>>  	call	sme_enable
>>  	pop	%rsi
>>  	movq	%rax, %r12
>> diff --git a/arch/x86/kernel/mem_encrypt_init.c b/arch/x86/kernel/mem_encrypt_init.c
>> index 7bdd159..c94ceb8 100644
>> --- a/arch/x86/kernel/mem_encrypt_init.c
>> +++ b/arch/x86/kernel/mem_encrypt_init.c
>> @@ -16,9 +16,14 @@
>>  #include <linux/mm.h>
>>  
>>  #include <asm/sections.h>
>> +#include <asm/processor-flags.h>
>> +#include <asm/msr.h>
>> +#include <asm/cmdline.h>
>>  
>>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>>  
>> +static char sme_cmdline_arg[] __initdata = "mem_encrypt=on";
> 
> One more thing: just like we're adding an =on switch, we'd need an =off
> switch in case something's wrong with the SME code. IOW, if a user
> supplies "mem_encrypt=off", we do not encrypt.

Well, we can document "off", but if the exact string "mem_encrypt=on"
isn't specified on the command line then the encryption won't occur.
The cmdline_find_option_bool() function looks for the exact string and
isn't interpreting the value on the right side of the equal sign. So
omitting mem_encrypt=on or using mem_encrypt=off is the same.

Thanks,
Tom

> 
> Thanks.
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov Nov. 29, 2016, 7:56 p.m. UTC | #4 
On Tue, Nov 29, 2016 at 12:48:17PM -0600, Tom Lendacky wrote:
> > One more thing: just like we're adding an =on switch, we'd need an =off
> > switch in case something's wrong with the SME code. IOW, if a user
> > supplies "mem_encrypt=off", we do not encrypt.
> 
> Well, we can document "off", but if the exact string "mem_encrypt=on"
> isn't specified on the command line then the encryption won't occur.

So you have this:

+       /*
+        * Fixups have not been to applied phys_base yet, so we must obtain
+        * the address to the SME command line option in the following way.
+        */
+       asm ("lea sme_cmdline_arg(%%rip), %0"
+            : "=r" (cmdline_arg)
+            : "p" (sme_cmdline_arg));
+       cmdline_ptr = bp->hdr.cmd_line_ptr | ((u64)bp->ext_cmd_line_ptr << 32);
+       if (cmdline_find_option_bool((char *)cmdline_ptr, cmdline_arg))
+               sme_me_mask = 1UL << (ebx & 0x3f);

If I parse this right, we will enable SME *only* if mem_encrypt=on is
explicitly supplied on the command line.

Which means, users will have to *know* about that cmdline switch first.
Which then means, we have to go and tell them. Do you see where I'm
going with this?

I know we talked about this already but I still think we should enable
it by default and people who don't want it will use the =off switch. We
can also do something like CONFIG_AMD_SME_ENABLED_BY_DEFAULT which we
can be selected during build for the different setups.

Hmmm.
diff mbox

Patch

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index e8a7272..c225433 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -100,6 +100,7 @@  startup_64:
 	 * to include it in the page table fixups.
 	 */
 	push	%rsi
+	movq	%rsi, %rdi
 	call	sme_enable
 	pop	%rsi
 	movq	%rax, %r12
diff --git a/arch/x86/kernel/mem_encrypt_init.c b/arch/x86/kernel/mem_encrypt_init.c
index 7bdd159..c94ceb8 100644
--- a/arch/x86/kernel/mem_encrypt_init.c
+++ b/arch/x86/kernel/mem_encrypt_init.c
@@ -16,9 +16,14 @@ 
 #include <linux/mm.h>
 
 #include <asm/sections.h>
+#include <asm/processor-flags.h>
+#include <asm/msr.h>
+#include <asm/cmdline.h>
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 
+static char sme_cmdline_arg[] __initdata = "mem_encrypt=on";
+
 extern void sme_encrypt_execute(unsigned long, unsigned long, unsigned long,
 				void *, pgd_t *);
 
@@ -219,7 +224,60 @@  unsigned long __init sme_get_me_mask(void)
 	return sme_me_mask;
 }
 
-unsigned long __init sme_enable(void)
+unsigned long __init sme_enable(void *boot_data)
 {
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+	struct boot_params *bp = boot_data;
+	unsigned int eax, ebx, ecx, edx;
+	u64 msr;
+	unsigned long cmdline_ptr;
+	void *cmdline_arg;
+
+	/* Check for an AMD processor */
+	eax = 0;
+	ecx = 0;
+	native_cpuid(&eax, &ebx, &ecx, &edx);
+	if ((ebx != 0x68747541) || (edx != 0x69746e65) || (ecx != 0x444d4163))
+		goto out;
+
+	/* Check for the SME support leaf */
+	eax = 0x80000000;
+	ecx = 0;
+	native_cpuid(&eax, &ebx, &ecx, &edx);
+	if (eax < 0x8000001f)
+		goto out;
+
+	/*
+	 * Check for the SME feature:
+	 *   CPUID Fn8000_001F[EAX] - Bit 0
+	 *     Secure Memory Encryption support
+	 *   CPUID Fn8000_001F[EBX] - Bits 5:0
+	 *     Pagetable bit position used to indicate encryption
+	 */
+	eax = 0x8000001f;
+	ecx = 0;
+	native_cpuid(&eax, &ebx, &ecx, &edx);
+	if (!(eax & 1))
+		goto out;
+
+	/* Check if SME is enabled */
+	msr = native_read_msr(MSR_K8_SYSCFG);
+	if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
+		goto out;
+
+	/*
+	 * Fixups have not been to applied phys_base yet, so we must obtain
+	 * the address to the SME command line option in the following way.
+	 */
+	asm ("lea sme_cmdline_arg(%%rip), %0"
+	     : "=r" (cmdline_arg)
+	     : "p" (sme_cmdline_arg));
+	cmdline_ptr = bp->hdr.cmd_line_ptr | ((u64)bp->ext_cmd_line_ptr << 32);
+	if (cmdline_find_option_bool((char *)cmdline_ptr, cmdline_arg))
+		sme_me_mask = 1UL << (ebx & 0x3f);
+
+out:
+#endif	/* CONFIG_AMD_MEM_ENCRYPT */
+
 	return sme_me_mask;
 }
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index e351003..d0bc3f5 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -251,6 +251,8 @@  void __init mem_encrypt_init(void)
 
 	/* Make SWIOTLB use an unencrypted DMA area */
 	swiotlb_clear_encryption();
+
+	pr_info("AMD Secure Memory Encryption active\n");
 }
 
 void swiotlb_set_mem_unenc(void *vaddr, unsigned long size)