| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <80579c46-3d29-f33b-9102-770f0c1c64ac@amd.com>
Date: Wed, 1 Dec 2021 13:56:02 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org
Cc: Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
"H . Peter Anvin" <hpa@...or.com>, Tony Luck <tony.luck@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Andi Kleen <ak@...ux.intel.com>,
Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 4/4] x86: Move common memory encryption code to
mem_encrypt.c
On 11/15/21 6:45 PM, Kuppuswamy Sathyanarayanan wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
>
> SEV and TDX both protect guest memory from the host access. They both
> use guest physical address bits to communicate to the hardware which
> pages receive protection or not. SEV and TDX both assume that all I/O
> (real devices and virtio) must be performed to pages *without*
> protection.
>
> To add this support, AMD SEV code forces force_dma_unencrypted() to
> decrypt DMA pages when DMA pages were allocated for I/O. It also uses
> swiotlb_update_mem_attributes() to update decryption bits in SWIOTLB
> DMA buffers.
>
> Since TDX also uses a similar memory sharing design, all the above
> mentioned changes can be reused. So move force_dma_unencrypted(),
> SWIOTLB update code and virtio changes out of mem_encrypt_amd.c to
> mem_encrypt.c.
>
> Introduce a new config option X86_MEM_ENCRYPT that can be selected
> by platforms which uses x86 memory encryption features (needed in both
> AMD SEV and Intel TDX guest platforms).
>
> Since the code is moved from mem_encrypt_amdc.c, inherit the same make
> flags.
>
> This is preparation for enabling TDX memory encryption support and it
> has no functional changes.
It seems strange that some AMD specific function/code is in the common
file, especially print_amd_mem_encrypt_feature_info(). I'm not sure if it
would be better to wait and move that and mem_encrypt_init() when you add
the TDX support, since you'll have to update mem_encrypt_init() to change
the check anyway. Just my opinion, though.
Thanks,
Tom
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reviewed-by: Andi Kleen <ak@...ux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> ---
> arch/x86/Kconfig | 10 +++--
> arch/x86/mm/Makefile | 5 +++
> arch/x86/mm/mem_encrypt.c | 84 +++++++++++++++++++++++++++++++++++
> arch/x86/mm/mem_encrypt_amd.c | 69 ----------------------------
> 4 files changed, 96 insertions(+), 72 deletions(-)
> create mode 100644 arch/x86/mm/mem_encrypt.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 95dd1ee01546..793e9b42ace0 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1523,16 +1523,20 @@ config X86_CPA_STATISTICS
> helps to determine the effectiveness of preserving large and huge
> page mappings when mapping protections are changed.
>
> +config X86_MEM_ENCRYPT
> + select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> + select DYNAMIC_PHYSICAL_MASK
> + select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
> + def_bool n
> +
> config AMD_MEM_ENCRYPT
> bool "AMD Secure Memory Encryption (SME) support"
> depends on X86_64 && CPU_SUP_AMD
> select DMA_COHERENT_POOL
> - select DYNAMIC_PHYSICAL_MASK
> select ARCH_USE_MEMREMAP_PROT
> - select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> select INSTRUCTION_DECODER
> - select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
> select ARCH_HAS_CC_PLATFORM
> + select X86_MEM_ENCRYPT
> help
> Say yes to enable support for the encryption of system memory.
> This requires an AMD processor that supports Secure Memory
> diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
> index c9c480641153..fe3d3061fc11 100644
> --- a/arch/x86/mm/Makefile
> +++ b/arch/x86/mm/Makefile
> @@ -1,9 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0
> # Kernel does not boot with instrumentation of tlb.c and mem_encrypt*.c
> KCOV_INSTRUMENT_tlb.o := n
> +KCOV_INSTRUMENT_mem_encrypt.o := n
> KCOV_INSTRUMENT_mem_encrypt_amd.o := n
> KCOV_INSTRUMENT_mem_encrypt_identity.o := n
>
> +KASAN_SANITIZE_mem_encrypt.o := n
> KASAN_SANITIZE_mem_encrypt_amd.o := n
> KASAN_SANITIZE_mem_encrypt_identity.o := n
>
> @@ -12,6 +14,7 @@ KASAN_SANITIZE_mem_encrypt_identity.o := n
> KCSAN_SANITIZE := n
>
> ifdef CONFIG_FUNCTION_TRACER
> +CFLAGS_REMOVE_mem_encrypt.o = -pg
> CFLAGS_REMOVE_mem_encrypt_amd.o = -pg
> CFLAGS_REMOVE_mem_encrypt_identity.o = -pg
> endif
> @@ -52,6 +55,8 @@ obj-$(CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS) += pkeys.o
> obj-$(CONFIG_RANDOMIZE_MEMORY) += kaslr.o
> obj-$(CONFIG_PAGE_TABLE_ISOLATION) += pti.o
>
> +obj-$(CONFIG_X86_MEM_ENCRYPT) += mem_encrypt.o
> obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_amd.o
> +
> obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_identity.o
> obj-$(CONFIG_AMD_MEM_ENCRYPT) += mem_encrypt_boot.o
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> new file mode 100644
> index 000000000000..5ae4b3d7d549
> --- /dev/null
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Memory Encryption Support Common Code
> + *
> + * Copyright (C) 2016 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lendacky@....com>
> + */
> +
> +#include <linux/dma-direct.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/swiotlb.h>
> +#include <linux/cc_platform.h>
> +#include <linux/mem_encrypt.h>
> +#include <linux/virtio_config.h>
> +
> +/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> +bool force_dma_unencrypted(struct device *dev)
> +{
> + /*
> + * For SEV, all DMA must be to unencrypted addresses.
> + */
> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> + return true;
> +
> + /*
> + * For SME, all DMA must be to unencrypted addresses if the
> + * device does not support DMA to addresses that include the
> + * encryption mask.
> + */
> + if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> + u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask));
> + u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask,
> + dev->bus_dma_limit);
> +
> + if (dma_dev_mask <= dma_enc_mask)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void print_amd_mem_encrypt_feature_info(void)
> +{
> + pr_info("AMD Memory Encryption Features active:");
> +
> + /* Secure Memory Encryption */
> + if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> + /*
> + * SME is mutually exclusive with any of the SEV
> + * features below.
> + */
> + pr_cont(" SME\n");
> + return;
> + }
> +
> + /* Secure Encrypted Virtualization */
> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> + pr_cont(" SEV");
> +
> + /* Encrypted Register State */
> + if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> + pr_cont(" SEV-ES");
> +
> + pr_cont("\n");
> +}
> +
> +/* Architecture __weak replacement functions */
> +void __init mem_encrypt_init(void)
> +{
> + if (!sme_me_mask)
> + return;
> +
> + /* Call into SWIOTLB to update the SWIOTLB DMA buffers */
> + swiotlb_update_mem_attributes();
> +
> + print_amd_mem_encrypt_feature_info();
> +}
> +
> +int arch_has_restricted_virtio_memory_access(void)
> +{
> + return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
> +}
> +EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index b520021a7e7b..2b2d018ea345 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -413,32 +413,6 @@ void __init early_set_mem_enc_dec_hypercall(unsigned long vaddr, int npages, boo
> notify_range_enc_status_changed(vaddr, npages, enc);
> }
>
> -/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> -bool force_dma_unencrypted(struct device *dev)
> -{
> - /*
> - * For SEV, all DMA must be to unencrypted addresses.
> - */
> - if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> - return true;
> -
> - /*
> - * For SME, all DMA must be to unencrypted addresses if the
> - * device does not support DMA to addresses that include the
> - * encryption mask.
> - */
> - if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> - u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask));
> - u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask,
> - dev->bus_dma_limit);
> -
> - if (dma_dev_mask <= dma_enc_mask)
> - return true;
> - }
> -
> - return false;
> -}
> -
> void __init mem_encrypt_free_decrypted_mem(void)
> {
> unsigned long vaddr, vaddr_end, npages;
> @@ -462,46 +436,3 @@ void __init mem_encrypt_free_decrypted_mem(void)
>
> free_init_pages("unused decrypted", vaddr, vaddr_end);
> }
> -
> -static void print_mem_encrypt_feature_info(void)
> -{
> - pr_info("AMD Memory Encryption Features active:");
> -
> - /* Secure Memory Encryption */
> - if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> - /*
> - * SME is mutually exclusive with any of the SEV
> - * features below.
> - */
> - pr_cont(" SME\n");
> - return;
> - }
> -
> - /* Secure Encrypted Virtualization */
> - if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> - pr_cont(" SEV");
> -
> - /* Encrypted Register State */
> - if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> - pr_cont(" SEV-ES");
> -
> - pr_cont("\n");
> -}
> -
> -/* Architecture __weak replacement functions */
> -void __init mem_encrypt_init(void)
> -{
> - if (!sme_me_mask)
> - return;
> -
> - /* Call into SWIOTLB to update the SWIOTLB DMA buffers */
> - swiotlb_update_mem_attributes();
> -
> - print_mem_encrypt_feature_info();
> -}
> -
> -int arch_has_restricted_virtio_memory_access(void)
> -{
> - return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
> -}
> -EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
>
Powered by blists - more mailing lists