Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754173AbdLUM6O (ORCPT ); Thu, 21 Dec 2017 07:58:14 -0500 Received: from mail.skyhub.de ([5.9.137.197]:45538 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752285AbdLUM6N (ORCPT ); Thu, 21 Dec 2017 07:58:13 -0500 Date: Thu, 21 Dec 2017 13:58:00 +0100 From: Borislav Petkov To: Tom Lendacky Cc: x86@kernel.org, Brijesh Singh , linux-kernel@vger.kernel.org, Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner Subject: Re: [PATCH v1 2/3] x86/mm: Prepare sme_encrypt_kernel() for PAGE aligned encryption Message-ID: <20171221125800.wb5747hq4vbp5yhf@pd.tnic> References: <20171207233342.29646.12858.stgit@tlendack-t1.amdoffice.net> <20171207233402.29646.77306.stgit@tlendack-t1.amdoffice.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20171207233402.29646.77306.stgit@tlendack-t1.amdoffice.net> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6301 Lines: 198 On Thu, Dec 07, 2017 at 05:34:02PM -0600, Tom Lendacky wrote: > In preparation for encrypting more than just the kernel, the encryption > support in sme_encrypt_kernel() needs to support 4KB page aligned > encryption instead of just 2MB large page aligned encryption. ... > static void __init __sme_map_range(pgd_t *pgd, unsigned long vaddr, > unsigned long vaddr_end, > - unsigned long paddr, pmdval_t pmd_flags) > + unsigned long paddr, > + pmdval_t pmd_flags, pteval_t pte_flags) > { > - while (vaddr < vaddr_end) { > - sme_populate_pgd(pgd, vaddr, paddr, pmd_flags); > + if (vaddr & ~PMD_PAGE_MASK) { > + /* Start is not 2MB aligned, create PTE entries */ > + unsigned long pmd_start = ALIGN(vaddr, PMD_PAGE_SIZE); > + > + while (vaddr < pmd_start) { > + sme_populate_pgd(pgd, vaddr, paddr, pte_flags); > + > + vaddr += PAGE_SIZE; > + paddr += PAGE_SIZE; > + } > + } This chunk... > + > + while (vaddr < (vaddr_end & PMD_PAGE_MASK)) { > + sme_populate_pgd_large(pgd, vaddr, paddr, pmd_flags); > > vaddr += PMD_PAGE_SIZE; > paddr += PMD_PAGE_SIZE; > } > + > + if (vaddr_end & ~PMD_PAGE_MASK) { > + /* End is not 2MB aligned, create PTE entries */ > + while (vaddr < vaddr_end) { > + sme_populate_pgd(pgd, vaddr, paddr, pte_flags); > + > + vaddr += PAGE_SIZE; > + paddr += PAGE_SIZE; > + } > + } ... and this chunk look like could be extracted in a wrapper as they're pretty similar. > static void __init sme_map_range_encrypted(pgd_t *pgd, > @@ -582,7 +658,8 @@ static void __init sme_map_range_encrypted(pgd_t *pgd, > unsigned long vaddr_end, > unsigned long paddr) > { > - __sme_map_range(pgd, vaddr, vaddr_end, paddr, PMD_FLAGS_ENC); > + __sme_map_range(pgd, vaddr, vaddr_end, paddr, > + PMD_FLAGS_ENC, PTE_FLAGS_ENC); > } > > static void __init sme_map_range_decrypted(pgd_t *pgd, > @@ -590,7 +667,8 @@ static void __init sme_map_range_decrypted(pgd_t *pgd, > unsigned long vaddr_end, > unsigned long paddr) > { > - __sme_map_range(pgd, vaddr, vaddr_end, paddr, PMD_FLAGS_DEC); > + __sme_map_range(pgd, vaddr, vaddr_end, paddr, > + PMD_FLAGS_DEC, PTE_FLAGS_DEC); > } > > static void __init sme_map_range_decrypted_wp(pgd_t *pgd, > @@ -598,19 +676,20 @@ static void __init sme_map_range_decrypted_wp(pgd_t *pgd, > unsigned long vaddr_end, > unsigned long paddr) > { > - __sme_map_range(pgd, vaddr, vaddr_end, paddr, PMD_FLAGS_DEC_WP); > + __sme_map_range(pgd, vaddr, vaddr_end, paddr, > + PMD_FLAGS_DEC_WP, PTE_FLAGS_DEC_WP); > } > > static unsigned long __init sme_pgtable_calc(unsigned long len) > { > - unsigned long p4d_size, pud_size, pmd_size; > + unsigned long p4d_size, pud_size, pmd_size, pte_size; > unsigned long total; > > /* > * Perform a relatively simplistic calculation of the pagetable > - * entries that are needed. That mappings will be covered by 2MB > - * PMD entries so we can conservatively calculate the required > - * number of P4D, PUD and PMD structures needed to perform the > + * entries that are needed. That mappings will be covered mostly Those > + * by 2MB PMD entries so we can conservatively calculate the required > + * number of P4D, PUD, PMD and PTE structures needed to perform the > * mappings. Incrementing the count for each covers the case where > * the addresses cross entries. > */ > @@ -626,8 +705,9 @@ static unsigned long __init sme_pgtable_calc(unsigned long len) > } > pmd_size = (ALIGN(len, PUD_SIZE) / PUD_SIZE) + 1; > pmd_size *= sizeof(pmd_t) * PTRS_PER_PMD; > + pte_size = 2 * sizeof(pte_t) * PTRS_PER_PTE; This needs the explanation from the commit message about the 2 extra pages, as a comment above it. > - total = p4d_size + pud_size + pmd_size; > + total = p4d_size + pud_size + pmd_size + pte_size; > > /* > * Now calculate the added pagetable structures needed to populate > @@ -710,10 +790,13 @@ void __init sme_encrypt_kernel(void) > > /* > * The total workarea includes the executable encryption area and > - * the pagetable area. > + * the pagetable area. The start of the workarea is already 2MB > + * aligned, align the end of the workarea on a 2MB boundary so that > + * we don't try to create/allocate PTE entries from the workarea > + * before it is mapped. > */ > workarea_len = execute_len + pgtable_area_len; > - workarea_end = workarea_start + workarea_len; > + workarea_end = ALIGN(workarea_start + workarea_len, PMD_PAGE_SIZE); > > /* > * Set the address to the start of where newly created pagetable > diff --git a/arch/x86/mm/mem_encrypt_boot.S b/arch/x86/mm/mem_encrypt_boot.S > index 730e6d5..20cca86 100644 > --- a/arch/x86/mm/mem_encrypt_boot.S > +++ b/arch/x86/mm/mem_encrypt_boot.S > @@ -120,23 +120,33 @@ ENTRY(__enc_copy) > > wbinvd /* Invalidate any cache entries */ > > + push %r12 > + > /* Copy/encrypt 2MB at a time */ > + movq $PMD_PAGE_SIZE, %r12 > 1: > + cmpq %r12, %r9 > + jnb 2f > + movq %r9, %r12 > + > +2: > movq %r11, %rsi /* Source - decrypted kernel */ > movq %r8, %rdi /* Dest - intermediate copy buffer */ > - movq $PMD_PAGE_SIZE, %rcx /* 2MB length */ > + movq %r12, %rcx > rep movsb > > movq %r8, %rsi /* Source - intermediate copy buffer */ > movq %r10, %rdi /* Dest - encrypted kernel */ > - movq $PMD_PAGE_SIZE, %rcx /* 2MB length */ > + movq %r12, %rcx > rep movsb > > - addq $PMD_PAGE_SIZE, %r11 > - addq $PMD_PAGE_SIZE, %r10 > - subq $PMD_PAGE_SIZE, %r9 /* Kernel length decrement */ > + addq %r12, %r11 > + addq %r12, %r10 > + subq %r12, %r9 /* Kernel length decrement */ > jnz 1b /* Kernel length not zero? */ > > + pop %r12 > + > /* Restore PAT register */ > push %rdx /* Save original PAT value */ > movl $MSR_IA32_CR_PAT, %ecx Right, for this here can you pls do a cleanup pre-patch which does push push push /* meat of the function */ pop pop pop as now those pushes and pops are interspersed with the rest of the insns and that makes following through it harder. F17h has a stack engine so those streams of pushes and pops won't hurt perf and besides, this is not even perf-sensitive. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.