Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932345AbcLGNTW (ORCPT ); Wed, 7 Dec 2016 08:19:22 -0500 Received: from mail-wj0-f174.google.com ([209.85.210.174]:34209 "EHLO mail-wj0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932177AbcLGNTS (ORCPT ); Wed, 7 Dec 2016 08:19:18 -0500 Date: Wed, 7 Dec 2016 13:19:03 +0000 From: Matt Fleming To: Tom Lendacky Cc: linux-arch@vger.kernel.org, linux-efi@vger.kernel.org, kvm@vger.kernel.org, linux-doc@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, iommu@lists.linux-foundation.org, Rik van Riel , Radim =?utf-8?B?S3LEjW3DocWZ?= , Arnd Bergmann , Jonathan Corbet , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov , Ard Biesheuvel Subject: Re: [RFC PATCH v3 10/20] Add support to access boot related data in the clear Message-ID: <20161207131903.GU20785@codeblueprint.co.uk> References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003631.3280.73292.stgit@tlendack-t1.amdoffice.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161110003631.3280.73292.stgit@tlendack-t1.amdoffice.net> User-Agent: Mutt/1.5.24+41 (02bc14ed1569) (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3813 Lines: 120 On Wed, 09 Nov, at 06:36:31PM, Tom Lendacky wrote: > Boot data (such as EFI related data) is not encrypted when the system is > booted and needs to be accessed unencrypted. Add support to apply the > proper attributes to the EFI page tables and to the early_memremap and > memremap APIs to identify the type of data being accessed so that the > proper encryption attribute can be applied. > > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/e820.h | 1 > arch/x86/kernel/e820.c | 16 +++++++ > arch/x86/mm/ioremap.c | 89 ++++++++++++++++++++++++++++++++++++++++ > arch/x86/platform/efi/efi_64.c | 12 ++++- > drivers/firmware/efi/efi.c | 33 +++++++++++++++ > include/linux/efi.h | 2 + > kernel/memremap.c | 8 +++- > mm/early_ioremap.c | 18 +++++++- > 8 files changed, 172 insertions(+), 7 deletions(-) FWIW, I think this version is an improvement over all the previous ones. [...] > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c > index ff542cd..ee347c2 100644 > --- a/arch/x86/mm/ioremap.c > +++ b/arch/x86/mm/ioremap.c > @@ -20,6 +20,9 @@ > #include > #include > #include > +#include > +#include > +#include > > #include "physaddr.h" > > @@ -418,6 +421,92 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) > iounmap((void __iomem *)((unsigned long)addr & PAGE_MASK)); > } > > +static bool memremap_setup_data(resource_size_t phys_addr, > + unsigned long size) > +{ > + u64 paddr; > + > + if (phys_addr == boot_params.hdr.setup_data) > + return true; > + Why is the setup_data linked list not traversed when checking for matching addresses? Am I reading this incorrectly? I don't see how this can work. > + 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; > + if (phys_addr == paddr) > + return true; > + > + if (efi_table_address_match(phys_addr)) > + return true; > + > + return false; > +} > + > +static bool memremap_apply_encryption(resource_size_t phys_addr, > + unsigned long size) > +{ > + /* SME is not active, just return true */ > + if (!sme_me_mask) > + return true; > + > + /* Check if the address is part of the setup data */ > + if (memremap_setup_data(phys_addr, size)) > + return false; > + > + /* Check if the address is part of EFI boot/runtime data */ > + switch (efi_mem_type(phys_addr)) { > + case EFI_BOOT_SERVICES_DATA: > + case EFI_RUNTIME_SERVICES_DATA: > + return false; > + } EFI_LOADER_DATA is notable by its absence. We use that memory type for allocations inside of the EFI boot stub that are than used while the kernel is running. One use that comes to mind is for initrd files, see handle_cmdline_files(). Oh I see you handle that in PATCH 9, never mind. > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c > index 58b0f80..3f89179 100644 > --- a/arch/x86/platform/efi/efi_64.c > +++ b/arch/x86/platform/efi/efi_64.c > @@ -221,7 +221,13 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages) > if (efi_enabled(EFI_OLD_MEMMAP)) > return 0; > > - efi_scratch.efi_pgt = (pgd_t *)__pa(efi_pgd); > + /* > + * Since the PGD is encrypted, set the encryption mask so that when > + * this value is loaded into cr3 the PGD will be decrypted during > + * the pagetable walk. > + */ > + efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd); > + > pgd = efi_pgd; > > /* Do all callers of __pa() in arch/x86 need fixing up like this?