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.
