Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()

[Brian Gerst]

On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky  wrote:
> Currently there is a check if the address being mapped is in the ISA
> range (is_ISA_range()), and if it is, then phys_to_virt() is used to
> perform the mapping. When SME is active, the default is to add pagetable
> mappings with the encryption bit set unless specifically overridden. The
> resulting pagetable mapping from phys_to_virt() will result in a mapping
> that has the encryption bit set. With SME, the use of ioremap() is
> intended to generate pagetable mappings that do not have the encryption
> bit set through the use of the PAGE_KERNEL_IO protection value.
>
> Rather than special case the SME scenario, remove the ISA range check and
> usage of phys_to_virt() and have ISA range mappings continue through the
> remaining ioremap() path.
>
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/mm/ioremap.c |7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 4c1b5fd..bfc3e2d 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -106,12 +107,6 @@ static void __iomem *__ioremap_caller(resource_size_t 
> phys_addr,
> }
>
> /*
> -* Don't remap the low PCI/ISA area, it's always mapped..
> -*/
> -   if (is_ISA_range(phys_addr, last_addr))
> -   return (__force void __iomem *)phys_to_virt(phys_addr);
> -
> -   /*
>  * Don't allow anybody to remap normal RAM that we're using..
>  */
> pfn  = phys_addr >> PAGE_SHIFT;
>

Removing this also affects 32-bit, which is more likely to access
legacy devices in this range.  Put in a check for SME instead
(provided you follow my recommendations to not set the SME feature bit
on 32-bit even when the processor supports it).

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v9 04/38] x86/CPU/AMD: Add the Secure Memory Encryption CPU feature

[Brian Gerst]

On Fri, Jul 7, 2017 at 9:38 AM, Tom Lendacky  wrote:
> Update the CPU features to include identifying and reporting on the
> Secure Memory Encryption (SME) feature.  SME is identified by CPUID
> 0x801f, 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 
> Signed-off-by: Tom Lendacky 
> ---
>  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.

>  #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_MEM10xc001001a
>  #define MSR_K8_TOP_MEM20xc001001d
>  #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_MASK0x1800
> 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.

>  }
>
>  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, 0x8007, 0 },
> { X86_FEATURE_CPB,  CPUID_EDX,  9, 0x8007, 0 },
> { X86_FEATURE_PROC_FEEDBACK,CPUID_EDX, 11, 0x8007, 0 },
> +   { X86_FEATURE_SME,  CPUID_EAX,  0, 0x801f, 0 },

This should also be conditional.  We don't want to set this feature on
32-bit, even if the processor has support.

> { 0, 0, 0, 0, 0 }
>  };

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v9 00/38] x86: Secure Memory Encryption (AMD)

[Ingo Molnar]

* Tom Lendacky  wrote:

> This patch series provides support for AMD's new Secure Memory Encryption 
> (SME)
> feature.

I'm wondering, what's the typical performance hit to DRAM access latency when 
SME 
is enabled?

On that same note, if the performance hit is noticeable I'd expect SME to not 
be 
enabled in native kernels typically - but still it looks like a useful hardware 
feature. Since it's controlled at the page table level, have you considered 
allowing SME-activated vmas via mmap(), even on kernels that are otherwise not 
using encrypted DRAM?

One would think that putting encryption keys into such encrypted RAM regions 
would 
generally improve robustness against various physical space attacks that want 
to 
extract keys but don't have full control of the CPU.

Thanks,

Ingo

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec