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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 21 Dec 2017 10:35:37 -0600 From: Tom Lendacky <thomas.lendacky@....com> To: Borislav Petkov <bp@...en8.de> Cc: x86@...nel.org, Brijesh Singh <brijesh.singh@....com>, linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de> Subject: Re: [PATCH v1 2/3] x86/mm: Prepare sme_encrypt_kernel() for PAGE aligned encryption On 12/21/2017 6:58 AM, Borislav Petkov wrote: > 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. > Should be doable, I'll take care of it. >> 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 > Got it. >> + * 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. Yup, I'll include the info about the (possible) extra PTE entries. > >> - 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. > Ok, I'll clean that up to move any pushes/pops to the beginning/end of the function, as well as moving some register saving earlier which may eliminate some push/pop instructions. Thanks, Tom > Thx. >
Powered by blists - more mailing lists