Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752538AbcKQMqA (ORCPT ); Thu, 17 Nov 2016 07:46:00 -0500 Received: from mail.skyhub.de ([78.46.96.112]:37336 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751532AbcKQMpz (ORCPT ); Thu, 17 Nov 2016 07:45:55 -0500 Date: Thu, 17 Nov 2016 13:20:15 +0100 From: Borislav Petkov 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 , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov Subject: Re: [RFC PATCH v3 09/20] x86: Insure that boot memory areas are mapped properly Message-ID: <20161117122015.kxnwjtgyzitxio2p@pd.tnic> References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003620.3280.20613.stgit@tlendack-t1.amdoffice.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161110003620.3280.20613.stgit@tlendack-t1.amdoffice.net> User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4165 Lines: 145 On Wed, Nov 09, 2016 at 06:36:20PM -0600, Tom Lendacky wrote: > The boot data and command line data are present in memory in an > un-encrypted state and are copied early in the boot process. The early > page fault support will map these areas as encrypted, so before attempting > to copy them, add unencrypted mappings so the data is accessed properly > when copied. > > For the initrd, encrypt this data in place. Since the future mapping of the > initrd area will be mapped as encrypted the data will be accessed properly. > > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/mem_encrypt.h | 13 ++++++++ > arch/x86/kernel/head64.c | 21 ++++++++++++-- > arch/x86/kernel/setup.c | 9 ++++++ > arch/x86/mm/mem_encrypt.c | 56 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 96 insertions(+), 3 deletions(-) ... > @@ -122,6 +131,12 @@ static void __init copy_bootdata(char *real_mode_data) > char * command_line; > unsigned long cmd_line_ptr; > > + /* > + * If SME is active, this will create un-encrypted mappings of the > + * boot data in advance of the copy operations ^ | Fullstop--+ > + */ > + sme_map_bootdata(real_mode_data); > + > memcpy(&boot_params, real_mode_data, sizeof boot_params); > sanitize_boot_params(&boot_params); > cmd_line_ptr = get_cmd_line_ptr(); ... > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index 06235b4..411210d 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -16,8 +16,11 @@ > > #include > #include > +#include > +#include > > extern pmdval_t early_pmd_flags; > +int __init __early_make_pgtable(unsigned long, pmdval_t); > > /* > * Since sme_me_mask is set early in the boot process it must reside in > @@ -126,6 +129,59 @@ void __init sme_early_mem_dec(resource_size_t paddr, unsigned long size) > } > } > > +static void __init *sme_bootdata_mapping(void *vaddr, unsigned long size) So this could be called __sme_map_bootdata(). "sme_bootdata_mapping" doesn't tell me what the function does as there's no verb in the name. > +{ > + unsigned long paddr = (unsigned long)vaddr - __PAGE_OFFSET; > + pmdval_t pmd_flags, pmd; > + void *ret = vaddr; That *ret ---> > + > + /* Use early_pmd_flags but remove the encryption mask */ > + pmd_flags = early_pmd_flags & ~sme_me_mask; > + > + do { > + pmd = (paddr & PMD_MASK) + pmd_flags; > + __early_make_pgtable((unsigned long)vaddr, pmd); > + > + vaddr += PMD_SIZE; > + paddr += PMD_SIZE; > + size = (size < PMD_SIZE) ? 0 : size - PMD_SIZE; size <= PMD_SIZE looks more obvious to me... > + } while (size); > + > + return ret; ---> is simply passing vaddr out. So the function can be just as well be void and you can do below: __sme_map_bootdata(real_mode_data, sizeof(boot_params)); boot_data = (struct boot_params *)real_mode_data; ... > +void __init sme_map_bootdata(char *real_mode_data) > +{ > + struct boot_params *boot_data; > + unsigned long cmdline_paddr; > + > + if (!sme_me_mask) > + return; > + > + /* > + * The bootdata will not be encrypted, so it needs to be mapped > + * as unencrypted data so it can be copied properly. > + */ > + boot_data = sme_bootdata_mapping(real_mode_data, sizeof(boot_params)); > + > + /* > + * Determine the command line address only after having established > + * the unencrypted mapping. > + */ > + cmdline_paddr = boot_data->hdr.cmd_line_ptr | > + ((u64)boot_data->ext_cmd_line_ptr << 32); <---- newline here. > + if (cmdline_paddr) > + sme_bootdata_mapping(__va(cmdline_paddr), COMMAND_LINE_SIZE); > +} > + > +void __init sme_encrypt_ramdisk(resource_size_t paddr, unsigned long size) > +{ > + if (!sme_me_mask) > + return; > + > + sme_early_mem_enc(paddr, size); > +} So this one could simply be called sme_encrypt_area() and be used for other things. There's nothing special about encrypting a ramdisk, by the looks of it. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.