On Mon, Jul 24, 2017 at 02:07:54PM -0500, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lenda...@amd.com>
> 
> Early in the boot process, add checks to determine if the kernel is
> running with Secure Encrypted Virtualization (SEV) active.
> 
> Checking for SEV requires checking that the kernel is running under a
> hypervisor (CPUID 0x00000001, bit 31), that the SEV feature is available
> (CPUID 0x8000001f, bit 1) and then check a non-interceptable SEV MSR
> (0xc0010131, bit 0).
> 
> This check is required so that during early compressed kernel booting the
> pagetables (both the boot pagetables and KASLR pagetables (if enabled) are
> updated to include the encryption mask so that when the kernel is
> decompressed into encrypted memory.

                                    , it can boot properly.

:)

> After the kernel is decompressed and continues booting the same logic is
> used to check if SEV is active and set a flag indicating so.  This allows
> us to distinguish between SME and SEV, each of which have unique
> differences in how certain things are handled: e.g. DMA (always bounce
> buffered with SEV) or EFI tables (always access decrypted with SME).
> 
> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
>  arch/x86/boot/compressed/Makefile      |   2 +
>  arch/x86/boot/compressed/head_64.S     |  16 +++++
>  arch/x86/boot/compressed/mem_encrypt.S | 103 
> +++++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.h        |   2 +
>  arch/x86/boot/compressed/pagetable.c   |   8 ++-
>  arch/x86/include/asm/mem_encrypt.h     |   3 +
>  arch/x86/include/asm/msr-index.h       |   3 +
>  arch/x86/include/uapi/asm/kvm_para.h   |   1 -
>  arch/x86/mm/mem_encrypt.c              |  42 +++++++++++---
>  9 files changed, 169 insertions(+), 11 deletions(-)
>  create mode 100644 arch/x86/boot/compressed/mem_encrypt.S
> 
> diff --git a/arch/x86/boot/compressed/Makefile 
> b/arch/x86/boot/compressed/Makefile
> index 2c860ad..d2fe901 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -72,6 +72,8 @@ vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o 
> $(obj)/misc.o \
>       $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
>       $(obj)/piggy.o $(obj)/cpuflags.o
>  
> +vmlinux-objs-$(CONFIG_X86_64) += $(obj)/mem_encrypt.o

There's a

ifdef CONFIG_X86_64

a couple of lines below. Just put it there.

...

> +++ b/arch/x86/boot/compressed/mem_encrypt.S
> @@ -0,0 +1,103 @@
> +/*
> + * AMD Memory Encryption Support
> + *
> + * Copyright (C) 2017 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lenda...@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>
> +
> +#include <asm/processor-flags.h>
> +#include <asm/msr.h>
> +#include <asm/asm-offsets.h>
> +
> +     .text
> +     .code32
> +ENTRY(get_sev_encryption_bit)
> +     xor     %eax, %eax
> +
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +     push    %ebx
> +     push    %ecx
> +     push    %edx
> +
> +     /* Check if running under a hypervisor */
> +     movl    $1, %eax
> +     cpuid
> +     bt      $31, %ecx               /* Check the hypervisor bit */
> +     jnc     .Lno_sev
> +
> +     movl    $0x80000000, %eax       /* CPUID to check the highest leaf */
> +     cpuid
> +     cmpl    $0x8000001f, %eax       /* See if 0x8000001f is available */
> +     jb      .Lno_sev
> +
> +     /*
> +      * Check for the SEV feature:
> +      *   CPUID Fn8000_001F[EAX] - Bit 1
> +      *   CPUID Fn8000_001F[EBX] - Bits 5:0
> +      *     Pagetable bit position used to indicate encryption
> +      */
> +     movl    $0x8000001f, %eax
> +     cpuid
> +     bt      $1, %eax                /* Check if SEV is available */
> +     jnc     .Lno_sev
> +
> +     movl    $MSR_F17H_SEV, %ecx     /* Read the SEV MSR */
> +     rdmsr
> +     bt      $MSR_F17H_SEV_ENABLED_BIT, %eax /* Check if SEV is active */
> +     jnc     .Lno_sev
> +
> +     /*
> +      * Get memory encryption information:
> +      */

The side-comment is enough. This one above can go.

> +     movl    %ebx, %eax
> +     andl    $0x3f, %eax             /* Return the encryption bit location */
> +     jmp     .Lsev_exit
> +
> +.Lno_sev:
> +     xor     %eax, %eax
> +
> +.Lsev_exit:
> +     pop     %edx
> +     pop     %ecx
> +     pop     %ebx
> +
> +#endif       /* CONFIG_AMD_MEM_ENCRYPT */
> +
> +     ret
> +ENDPROC(get_sev_encryption_bit)
> +
> +     .code64
> +ENTRY(get_sev_encryption_mask)
> +     xor     %rax, %rax
> +
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +     push    %rbp
> +     push    %rdx
> +
> +     movq    %rsp, %rbp              /* Save current stack pointer */
> +
> +     call    get_sev_encryption_bit  /* Get the encryption bit position */

So we get to call get_sev_encryption_bit() here again and noodle through
the CPUID discovery and MSR access. We should instead cache that info
and return the encryption bit directly on a second call. (And initialize
it to -1 to denote that get_sev_encryption_bit() hasn't run yet).

...

> diff --git a/arch/x86/include/asm/mem_encrypt.h 
> b/arch/x86/include/asm/mem_encrypt.h
> index 9274ec7..9cb6472 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -19,6 +19,9 @@
>  
>  #include <asm/bootparam.h>
>  
> +#define AMD_SME_FEATURE_BIT  BIT(0)
> +#define AMD_SEV_FEATURE_BIT  BIT(1)

s/_FEATURE//

AMD_SME_BIT and AMD_SEV_BIT is good enough :)

And frankly, if you're going to use them only below in sme_enable() - I
need to check more thoroughly the remaining patches - but if you only
are going to use them there, just define them inside the function so
that they're close.

> +
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  
>  extern unsigned long sme_me_mask;
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index e399d68..530020f 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -326,6 +326,9 @@
>  
>  /* Fam 17h MSRs */
>  #define MSR_F17H_IRPERF                      0xc00000e9
> +#define MSR_F17H_SEV                 0xc0010131

If that MSR is going to be used later on - and I don't see why not - you
probably should make it an arch one: MSR_AMD64_SEV. Even if it isn't yet
officially. :-)

> +#define MSR_F17H_SEV_ENABLED_BIT     0
> +#define MSR_F17H_SEV_ENABLED         BIT_ULL(MSR_F17H_SEV_ENABLED_BIT)
>  
>  /* Fam 16h MSRs */
>  #define MSR_F16H_L2I_PERF_CTL                0xc0010230
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
> b/arch/x86/include/uapi/asm/kvm_para.h
> index a965e5b..c202ba3 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -109,5 +109,4 @@ struct kvm_vcpu_pv_apf_data {
>  #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
>  #define KVM_PV_EOI_DISABLED 0x0
>  
> -
>  #endif /* _UAPI_ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 5e5d460..ed8780e 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -288,7 +288,9 @@ void __init mem_encrypt_init(void)
>       if (sev_active())
>               dma_ops = &sme_dma_ops;
>  
> -     pr_info("AMD Secure Memory Encryption (SME) active\n");
> +     pr_info("AMD %s active\n",
> +             sev_active() ? "Secure Encrypted Virtualization (SEV)"
> +                          : "Secure Memory Encryption (SME)");
>  }
>  
>  void swiotlb_set_mem_attributes(void *vaddr, unsigned long size)
> @@ -616,12 +618,23 @@ void __init __nostackprotector sme_enable(struct 
> boot_params *bp)
>  {
>       const char *cmdline_ptr, *cmdline_arg, *cmdline_on, *cmdline_off;
>       unsigned int eax, ebx, ecx, edx;
> +     unsigned long feature_mask;
>       bool active_by_default;
>       unsigned long me_mask;
>       char buffer[16];
>       u64 msr;
>  
> -     /* Check for the SME support leaf */
> +     /*
> +      * Set the feature mask (SME or SEV) based on whether we are
> +      * running under a hypervisor.
> +      */
> +     eax = 1;
> +     ecx = 0;
> +     native_cpuid(&eax, &ebx, &ecx, &edx);
> +     feature_mask = (ecx & BIT(31)) ? AMD_SEV_FEATURE_BIT
> +                                    : AMD_SME_FEATURE_BIT;

Set that feature mask before using it...

> +
> +     /* Check for the SME/SEV support leaf */

... because if that check exits due to no SME leaf, you're uselessly
doing all the above.

>       eax = 0x80000000;
>       ecx = 0;
>       native_cpuid(&eax, &ebx, &ecx, &edx);
> @@ -629,24 +642,39 @@ void __init __nostackprotector sme_enable(struct 
> boot_params *bp)
>               return;
>  
>       /*
> -      * Check for the SME feature:
> +      * Check for the SME/SEV feature:
>        *   CPUID Fn8000_001F[EAX] - Bit 0
>        *     Secure Memory Encryption support
> +      *   CPUID Fn8000_001F[EAX] - Bit 1

No need to repeat the CPUID leaf here - only Bit 1:

         *   CPUID Fn8000_001F[EAX]
         * - Bit 0:  Secure Memory Encryption support
         * - Bit 1:  Secure Encrypted Virtualization support


> +      *     Secure Encrypted Virtualization 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))
> +     if (!(eax & feature_mask))
>               return;
>  
>       me_mask = 1UL << (ebx & 0x3f);
>  
> -     /* Check if SME is enabled */
> -     msr = __rdmsr(MSR_K8_SYSCFG);
> -     if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
> +     /* Check if memory encryption is enabled */
> +     if (feature_mask == AMD_SME_FEATURE_BIT) {
> +             /* For SME, check the SYSCFG MSR */
> +             msr = __rdmsr(MSR_K8_SYSCFG);
> +             if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
> +                     return;
> +     } else {
> +             /* For SEV, check the SEV MSR */
> +             msr = __rdmsr(MSR_F17H_SEV);
> +             if (!(msr & MSR_F17H_SEV_ENABLED))
> +                     return;
> +
> +             /* SEV state cannot be controlled by a command line option */
> +             sme_me_mask = me_mask;
> +             sev_enabled = 1;
>               return;
> +     }

Nice. Two birds with one stone is always better. :)


-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to