From: Borislav Petkov <bp@alien8.de>
To: Tom Lendacky <thomas.lendacky@amd.com>
Cc: linux-arch@vger.kernel.org, linux-efi@vger.kernel.org,
kvm@vger.kernel.org, linux-doc@vger.kernel.org, x86@kernel.org,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
kasan-dev@googlegroups.com, linux-mm@kvack.org,
iommu@lists.linux-foundation.org,
"Rik van Riel" <riel@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>,
"Toshimitsu Kani" <toshi.kani@hpe.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Jonathan Corbet" <corbet@lwn.net>,
"Matt Fleming" <matt@codeblueprint.co.uk>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Joerg Roedel" <joro@8bytes.org>,
"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Larry Woodman" <lwoodman@redhat.com>,
"Brijesh Singh" <brijesh.singh@amd.com>,
"Ingo Molnar" <mingo@redhat.com>,
"Andy Lutomirski" <luto@kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>,
"Andrey Ryabinin" <aryabinin@virtuozzo.com>,
"Alexander Potapenko" <glider@google.com>,
"Dave Young" <dyoung@redhat.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Dmitry Vyukov" <dvyukov@google.com>
Subject: Re: [PATCH v5 17/32] x86/mm: Add support to access boot related data in the clear
Date: Mon, 15 May 2017 20:35:17 +0200 [thread overview]
Message-ID: <20170515183517.mb4k2gp2qobbuvtm@pd.tnic> (raw)
In-Reply-To: <20170418211921.10190.1537.stgit@tlendack-t1.amdoffice.net>
On Tue, Apr 18, 2017 at 04:19:21PM -0500, Tom Lendacky wrote:
> Boot data (such as EFI related data) is not encrypted when the system is
> booted because UEFI/BIOS does not run with SME active. In order to access
> this data properly it needs to be mapped decrypted.
>
> The early_memremap() support is updated to provide an arch specific
"Update early_memremap() to provide... "
> routine to modify the pagetable protection attributes before they are
> applied to the new mapping. This is used to remove the encryption mask
> for boot related data.
>
> The memremap() support is updated to provide an arch specific routine
Ditto. Passive tone always reads harder than an active tone,
"doer"-sentence.
> to determine if RAM remapping is allowed. RAM remapping will cause an
> encrypted mapping to be generated. By preventing RAM remapping,
> ioremap_cache() will be used instead, which will provide a decrypted
> mapping of the boot related data.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/include/asm/io.h | 4 +
> arch/x86/mm/ioremap.c | 182 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/io.h | 2
> kernel/memremap.c | 20 ++++-
> mm/early_ioremap.c | 18 ++++
> 5 files changed, 219 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index 7afb0e2..75f2858 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -381,4 +381,8 @@ extern int __must_check arch_phys_wc_add(unsigned long base,
> #define arch_io_reserve_memtype_wc arch_io_reserve_memtype_wc
> #endif
>
> +extern bool arch_memremap_do_ram_remap(resource_size_t offset, size_t size,
> + unsigned long flags);
> +#define arch_memremap_do_ram_remap arch_memremap_do_ram_remap
> +
> #endif /* _ASM_X86_IO_H */
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 9bfcb1f..bce0604 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -13,6 +13,7 @@
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
> #include <linux/mmiotrace.h>
> +#include <linux/efi.h>
>
> #include <asm/cacheflush.h>
> #include <asm/e820/api.h>
> @@ -21,6 +22,7 @@
> #include <asm/tlbflush.h>
> #include <asm/pgalloc.h>
> #include <asm/pat.h>
> +#include <asm/setup.h>
>
> #include "physaddr.h"
>
> @@ -419,6 +421,186 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
> iounmap((void __iomem *)((unsigned long)addr & PAGE_MASK));
> }
>
> +/*
> + * Examine the physical address to determine if it is an area of memory
> + * that should be mapped decrypted. If the memory is not part of the
> + * kernel usable area it was accessed and created decrypted, so these
> + * areas should be mapped decrypted.
> + */
> +static bool memremap_should_map_decrypted(resource_size_t phys_addr,
> + unsigned long size)
> +{
> + /* Check if the address is outside kernel usable area */
> + switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) {
> + case E820_TYPE_RESERVED:
> + case E820_TYPE_ACPI:
> + case E820_TYPE_NVS:
> + case E820_TYPE_UNUSABLE:
> + return true;
> + default:
> + break;
> + }
> +
> + return false;
> +}
> +
> +/*
> + * Examine the physical address to determine if it is EFI data. Check
> + * it against the boot params structure and EFI tables and memory types.
> + */
> +static bool memremap_is_efi_data(resource_size_t phys_addr,
> + unsigned long size)
> +{
> + u64 paddr;
> +
> + /* Check if the address is part of EFI boot/runtime data */
> + if (efi_enabled(EFI_BOOT)) {
Save indentation level:
if (!efi_enabled(EFI_BOOT))
return false;
> + paddr = boot_params.efi_info.efi_memmap_hi;
> + paddr <<= 32;
> + paddr |= boot_params.efi_info.efi_memmap;
> + if (phys_addr == paddr)
> + return true;
> +
> + paddr = boot_params.efi_info.efi_systab_hi;
> + paddr <<= 32;
> + paddr |= boot_params.efi_info.efi_systab;
So those two above look like could be two global vars which are
initialized somewhere in the EFI init path:
efi_memmap_phys and efi_systab_phys or so.
Matt ?
And then you won't need to create that paddr each time on the fly. I
mean, it's not a lot of instructions but still...
> + if (phys_addr == paddr)
> + return true;
> +
> + if (efi_table_address_match(phys_addr))
> + return true;
> +
> + switch (efi_mem_type(phys_addr)) {
> + case EFI_BOOT_SERVICES_DATA:
> + case EFI_RUNTIME_SERVICES_DATA:
> + return true;
> + default:
> + break;
> + }
> + }
> +
> + return false;
> +}
> +
> +/*
> + * Examine the physical address to determine if it is boot data by checking
> + * it against the boot params setup_data chain.
> + */
> +static bool memremap_is_setup_data(resource_size_t phys_addr,
> + unsigned long size)
> +{
> + struct setup_data *data;
> + u64 paddr, paddr_next;
> +
> + paddr = boot_params.hdr.setup_data;
> + while (paddr) {
> + bool is_setup_data = false;
You don't need that bool:
static bool memremap_is_setup_data(resource_size_t phys_addr,
unsigned long size)
{
struct setup_data *data;
u64 paddr, paddr_next;
paddr = boot_params.hdr.setup_data;
while (paddr) {
if (phys_addr == paddr)
return true;
data = memremap(paddr, sizeof(*data), MEMREMAP_WB | MEMREMAP_DEC);
paddr_next = data->next;
if ((phys_addr > paddr) && (phys_addr < (paddr + data->len))) {
memunmap(data);
return true;
}
memunmap(data);
paddr = paddr_next;
}
return false;
}
Flow is a bit clearer.
> +/*
> + * Examine the physical address to determine if it is boot data by checking
> + * it against the boot params setup_data chain (early boot version).
> + */
> +static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
> + unsigned long size)
> +{
> + struct setup_data *data;
> + u64 paddr, paddr_next;
> +
> + paddr = boot_params.hdr.setup_data;
> + while (paddr) {
> + bool is_setup_data = false;
> +
> + if (phys_addr == paddr)
> + return true;
> +
> + data = early_memremap_decrypted(paddr, sizeof(*data));
> +
> + paddr_next = data->next;
> +
> + if ((phys_addr > paddr) && (phys_addr < (paddr + data->len)))
> + is_setup_data = true;
> +
> + early_memunmap(data, sizeof(*data));
> +
> + if (is_setup_data)
> + return true;
> +
> + paddr = paddr_next;
> + }
> +
> + return false;
> +}
This one is begging to be unified with memremap_is_setup_data() to both
call a __ worker function.
> +
> +/*
> + * Architecture function to determine if RAM remap is allowed. By default, a
> + * RAM remap will map the data as encrypted. Determine if a RAM remap should
> + * not be done so that the data will be mapped decrypted.
> + */
> +bool arch_memremap_do_ram_remap(resource_size_t phys_addr, unsigned long size,
> + unsigned long flags)
So this function doesn't do anything - it replies to a yes/no question.
So the name should not say "do" but sound like a question. Maybe:
if (arch_memremap_can_remap( ... ))
or so...
> +{
> + if (!sme_active())
> + return true;
> +
> + if (flags & MEMREMAP_ENC)
> + return true;
> +
> + if (flags & MEMREMAP_DEC)
> + return false;
So this looks strange to me: both flags MEMREMAP_ENC and _DEC override
setup and efi data checking. But we want to remap setup and EFI data
*always* decrypted because that data was not encrypted as, as you say,
firmware doesn't run with SME active.
So my simple logic says that EFI stuff should *always* be mapped DEC,
regardless of flags. Ditto for setup data. So that check below should
actually *override* the flags checks and go before them, no?
> +
> + if (memremap_is_setup_data(phys_addr, size) ||
> + memremap_is_efi_data(phys_addr, size) ||
> + memremap_should_map_decrypted(phys_addr, size))
> + return false;
> +
> + return true;
> +}
> +
> +/*
> + * Architecture override of __weak function to adjust the protection attributes
> + * used when remapping memory. By default, early_memremp() will map the data
early_memremAp() - a is missing.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-05-15 18:35 UTC|newest]
Thread overview: 126+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-18 21:16 [PATCH v5 00/32] x86: Secure Memory Encryption (AMD) Tom Lendacky
2017-04-18 21:16 ` [PATCH v5 01/32] x86: Documentation for AMD Secure Memory Encryption (SME) Tom Lendacky
2017-04-19 9:02 ` Borislav Petkov
2017-04-19 14:23 ` Tom Lendacky
2017-04-19 15:38 ` Borislav Petkov
2017-04-19 9:52 ` David Howells
2017-04-18 21:16 ` [PATCH v5 02/32] x86/mm/pat: Set write-protect cache mode for full PAT support Tom Lendacky
2017-04-18 21:16 ` [PATCH v5 03/32] x86, mpparse, x86/acpi, x86/PCI, SFI: Use memremap for RAM mappings Tom Lendacky
2017-04-18 21:17 ` [PATCH v5 04/32] x86/CPU/AMD: Add the Secure Memory Encryption CPU feature Tom Lendacky
2017-04-18 21:17 ` [PATCH v5 05/32] x86/CPU/AMD: Handle SME reduction in physical address size Tom Lendacky
2017-04-20 16:59 ` Borislav Petkov
2017-04-20 17:29 ` Tom Lendacky
2017-04-20 18:52 ` Borislav Petkov
2017-04-18 21:17 ` [PATCH v5 06/32] x86/mm: Add Secure Memory Encryption (SME) support Tom Lendacky
2017-04-27 15:46 ` Borislav Petkov
2017-05-04 14:24 ` Tom Lendacky
2017-05-04 14:36 ` Borislav Petkov
2017-05-16 19:28 ` Tom Lendacky
2017-05-17 7:05 ` Borislav Petkov
2017-04-18 21:17 ` [PATCH v5 07/32] x86/mm: Add support to enable SME in early boot processing Tom Lendacky
[not found] ` <20170418211735.10190.29562.stgit-qCXWGYdRb2BnqfbPTmsdiZQ+2ll4COg0XqFh9Ls21Oc@public.gmane.org>
2017-04-21 14:55 ` Borislav Petkov
2017-04-21 21:40 ` Tom Lendacky
2017-04-18 21:17 ` [PATCH v5 08/32] x86/mm: Simplify p[g4um]d_page() macros Tom Lendacky
2017-04-18 21:17 ` [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption Tom Lendacky
2017-04-21 21:52 ` Dave Hansen
2017-04-24 15:53 ` Tom Lendacky
2017-04-24 15:57 ` Dave Hansen
2017-04-24 16:10 ` Tom Lendacky
2017-04-27 16:12 ` Borislav Petkov
2017-05-04 14:34 ` Tom Lendacky
2017-05-04 17:01 ` Borislav Petkov
2017-04-18 21:18 ` [PATCH v5 10/32] x86/mm: Extend early_memremap() support with additional attrs Tom Lendacky
2017-04-18 21:18 ` [PATCH v5 11/32] x86/mm: Add support for early encrypt/decrypt of memory Tom Lendacky
2017-04-18 21:18 ` [PATCH v5 12/32] x86/mm: Insure that boot memory areas are mapped properly Tom Lendacky
[not found] ` <20170418211822.10190.67435.stgit-qCXWGYdRb2BnqfbPTmsdiZQ+2ll4COg0XqFh9Ls21Oc@public.gmane.org>
2017-05-04 10:16 ` Borislav Petkov
2017-05-04 14:39 ` Tom Lendacky
2017-04-18 21:18 ` [PATCH v5 13/32] x86/boot/e820: Add support to determine the E820 type of an address Tom Lendacky
[not found] ` <20170418211831.10190.80158.stgit-qCXWGYdRb2BnqfbPTmsdiZQ+2ll4COg0XqFh9Ls21Oc@public.gmane.org>
2017-05-05 17:11 ` Borislav Petkov
2017-05-06 7:48 ` Ard Biesheuvel
2017-04-18 21:18 ` [PATCH v5 14/32] efi: Add an EFI table address match function Tom Lendacky
2017-05-15 18:09 ` Borislav Petkov
2017-05-16 21:53 ` Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 15/32] efi: Update efi_mem_type() to return an error rather than 0 Tom Lendacky
[not found] ` <20170418211900.10190.98158.stgit-qCXWGYdRb2BnqfbPTmsdiZQ+2ll4COg0XqFh9Ls21Oc@public.gmane.org>
2017-05-07 17:18 ` Borislav Petkov
2017-05-08 13:20 ` Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 16/32] x86/efi: Update EFI pagetable creation to work with SME Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 17/32] x86/mm: Add support to access boot related data in the clear Tom Lendacky
2017-05-15 18:35 ` Borislav Petkov [this message]
2017-05-17 18:54 ` Tom Lendacky
2017-05-18 9:02 ` Borislav Petkov
2017-05-19 20:50 ` Tom Lendacky
2017-05-21 7:16 ` Borislav Petkov
2017-05-30 16:46 ` Tom Lendacky
2017-05-31 11:31 ` Borislav Petkov
2017-05-18 19:50 ` Matt Fleming
2017-05-26 16:22 ` Tom Lendacky
2017-05-26 16:35 ` Borislav Petkov
2017-05-30 17:47 ` Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 18/32] x86, mpparse: Use memremap to map the mpf and mpc data Tom Lendacky
2017-05-16 8:36 ` Borislav Petkov
2017-05-17 20:26 ` Tom Lendacky
2017-05-18 9:03 ` Borislav Petkov
2017-04-18 21:19 ` [PATCH v5 19/32] x86/mm: Add support to access persistent memory in the clear Tom Lendacky
2017-05-16 14:04 ` Borislav Petkov
2017-05-19 19:52 ` Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 20/32] x86/mm: Add support for changing the memory encryption attribute Tom Lendacky
2017-04-18 21:19 ` [PATCH v5 21/32] x86, realmode: Decrypt trampoline area if memory encryption is active Tom Lendacky
2017-04-18 21:20 ` [PATCH v5 22/32] x86, swiotlb: DMA support for memory encryption Tom Lendacky
2017-05-16 14:27 ` Borislav Petkov
2017-05-19 19:54 ` Tom Lendacky
2017-04-18 21:20 ` [PATCH v5 23/32] swiotlb: Add warnings for use of bounce buffers with SME Tom Lendacky
2017-05-16 14:52 ` Borislav Petkov
2017-05-19 19:55 ` Tom Lendacky
2017-04-18 21:20 ` [PATCH v5 24/32] iommu/amd: Disable AMD IOMMU if memory encryption is active Tom Lendacky
2017-04-18 21:20 ` [PATCH v5 25/32] x86, realmode: Check for memory encryption on the APs Tom Lendacky
2017-04-18 21:20 ` [PATCH v5 26/32] x86, drm, fbdev: Do not specify encrypted memory for video mappings Tom Lendacky
2017-05-16 17:35 ` Borislav Petkov
2017-05-30 20:07 ` Tom Lendacky
2017-04-18 21:21 ` [PATCH v5 27/32] kvm: x86: svm: Enable Secure Memory Encryption within KVM Tom Lendacky
2017-04-18 21:21 ` [PATCH v5 28/32] x86/mm, kexec: Allow kexec to be used with SME Tom Lendacky
2017-05-17 19:17 ` Borislav Petkov
2017-05-19 20:45 ` Tom Lendacky
2017-05-19 20:58 ` Borislav Petkov
2017-05-19 21:07 ` Tom Lendacky
2017-05-19 21:28 ` Borislav Petkov
2017-05-19 21:38 ` Tom Lendacky
2017-05-26 4:17 ` Xunlei Pang
2017-05-27 2:17 ` Dave Young
2017-05-30 17:46 ` Tom Lendacky
2017-05-31 10:01 ` Borislav Petkov
2017-05-31 15:03 ` Xunlei Pang
2017-05-31 15:48 ` Borislav Petkov
2017-04-18 21:21 ` [PATCH v5 29/32] x86/mm: Add support to encrypt the kernel in-place Tom Lendacky
2017-05-18 12:46 ` Borislav Petkov
2017-05-25 22:24 ` Tom Lendacky
2017-05-26 16:25 ` Borislav Petkov
2017-05-30 16:39 ` Tom Lendacky
2017-05-31 9:51 ` Borislav Petkov
2017-05-31 13:12 ` Tom Lendacky
2017-04-18 21:22 ` [PATCH v5 30/32] x86/boot: Add early cmdline parsing for options with arguments Tom Lendacky
2017-04-18 21:22 ` [PATCH v5 31/32] x86: Add sysfs support for Secure Memory Encryption Tom Lendacky
2017-04-21 21:55 ` Dave Hansen
2017-04-27 7:25 ` Dave Young
2017-04-27 15:52 ` Dave Hansen
2017-04-28 5:32 ` Dave Young
2017-05-04 14:17 ` Tom Lendacky
2017-05-04 14:13 ` Tom Lendacky
2017-05-18 17:01 ` Borislav Petkov
2017-05-26 2:49 ` Dave Young
2017-05-26 5:04 ` Xunlei Pang
2017-05-26 15:47 ` Tom Lendacky
2017-04-18 21:22 ` [PATCH v5 32/32] x86/mm: Add support to make use of " Tom Lendacky
2017-04-21 18:56 ` Tom Lendacky
2017-05-19 11:30 ` Borislav Petkov
2017-05-19 20:16 ` Josh Poimboeuf
2017-05-19 20:29 ` Borislav Petkov
2017-05-30 15:48 ` Tom Lendacky
2017-05-31 9:15 ` Borislav Petkov
2017-05-30 15:46 ` Tom Lendacky
2017-05-19 11:27 ` Borislav Petkov
2017-05-30 14:38 ` Tom Lendacky
2017-05-30 14:55 ` Borislav Petkov
2017-05-30 15:37 ` Tom Lendacky
2017-05-31 8:49 ` Borislav Petkov
2017-05-31 13:37 ` Tom Lendacky
2017-05-31 14:12 ` Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170515183517.mb4k2gp2qobbuvtm@pd.tnic \
--to=bp@alien8.de \
--cc=arnd@arndb.de \
--cc=aryabinin@virtuozzo.com \
--cc=brijesh.singh@amd.com \
--cc=corbet@lwn.net \
--cc=dvyukov@google.com \
--cc=dyoung@redhat.com \
--cc=glider@google.com \
--cc=hpa@zytor.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=kasan-dev@googlegroups.com \
--cc=kexec@lists.infradead.org \
--cc=konrad.wilk@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=lwoodman@redhat.com \
--cc=matt@codeblueprint.co.uk \
--cc=mingo@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=riel@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=toshi.kani@hpe.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).