Patchwork [v1,3/3] x86/mm: Encrypt the initrd earlier for BSP microcode update

login
register
mail settings
Submitter Tom Lendacky
Date Dec. 7, 2017, 11:34 p.m.
Message ID <20171207233410.29646.27816.stgit@tlendack-t1.amdoffice.net>
Download mbox | patch
Permalink /patch/10101259/
State New
Headers show

Comments

Tom Lendacky - Dec. 7, 2017, 11:34 p.m.
Currently the BSP microcode update code examines the initrd very early
in the boot process.  If SME is active, the initrd is treated as being
encrypted but it has not been encrypted (in place) yet.  Update the
early boot code that encrypts the kernel to also encrypt the initrd so
that early BSP microcode updates work.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/mem_encrypt.h |    4 +-
 arch/x86/kernel/head64.c           |    4 +-
 arch/x86/kernel/setup.c            |   10 ------
 arch/x86/mm/mem_encrypt.c          |   62 +++++++++++++++++++++++++++++++-----
 arch/x86/mm/mem_encrypt_boot.S     |   46 +++++++++++++--------------
 5 files changed, 80 insertions(+), 46 deletions(-)
Borislav Petkov - Dec. 21, 2017, 2:49 p.m.
On Thu, Dec 07, 2017 at 05:34:10PM -0600, Tom Lendacky wrote:
> Currently the BSP microcode update code examines the initrd very early
> in the boot process.  If SME is active, the initrd is treated as being
> encrypted but it has not been encrypted (in place) yet.  Update the
> early boot code that encrypts the kernel to also encrypt the initrd so
> that early BSP microcode updates work.

...

> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 1f0efb8..60df247 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -731,11 +731,12 @@ static unsigned long __init sme_pgtable_calc(unsigned long len)
>  	return total;
>  }
>  
> -void __init sme_encrypt_kernel(void)
> +void __init sme_encrypt_kernel(struct boot_params *bp)
>  {
>  	unsigned long workarea_start, workarea_end, workarea_len;
>  	unsigned long execute_start, execute_end, execute_len;
>  	unsigned long kernel_start, kernel_end, kernel_len;
> +	unsigned long initrd_start, initrd_end, initrd_len;
>  	unsigned long pgtable_area_len;
>  	unsigned long decrypted_base;
>  	pgd_t *pgd;
> @@ -744,14 +745,15 @@ void __init sme_encrypt_kernel(void)
>  		return;
>  
>  	/*
> -	 * Prepare for encrypting the kernel by building new pagetables with
> -	 * the necessary attributes needed to encrypt the kernel in place.
> +	 * Prepare for encrypting the kernel and initrd by building new
> +	 * pagetables with the necessary attributes needed to encrypt the
> +	 * kernel in place.
>  	 *
>  	 *   One range of virtual addresses will map the memory occupied
> -	 *   by the kernel as encrypted.
> +	 *   by the kernel and initrd as encrypted.
>  	 *
>  	 *   Another range of virtual addresses will map the memory occupied
> -	 *   by the kernel as decrypted and write-protected.
> +	 *   by the kernel and initrd as decrypted and write-protected.
>  	 *
>  	 *     The use of write-protect attribute will prevent any of the
>  	 *     memory from being cached.
> @@ -762,6 +764,20 @@ void __init sme_encrypt_kernel(void)
>  	kernel_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE);
>  	kernel_len = kernel_end - kernel_start;
>  
> +	initrd_start = 0;
> +	initrd_end = 0;
> +	initrd_len = 0;
> +#ifdef CONFIG_BLK_DEV_INITRD
> +	initrd_len = (unsigned long)bp->hdr.ramdisk_size |
> +		     ((unsigned long)bp->ext_ramdisk_size << 32);
> +	if (initrd_len) {
> +		initrd_start = (unsigned long)bp->hdr.ramdisk_image |
> +			       ((unsigned long)bp->ext_ramdisk_image << 32);
> +		initrd_end = PAGE_ALIGN(initrd_start + initrd_len);
> +		initrd_len = initrd_end - initrd_start;
> +	}
> +#endif

In a prepatch, pls make get_ramdisk_image() and get_ramdisk_size() from
arch/x86/kernel/setup.c accessible to this code too. Also, add dummies
for the !CONFIG_BLK_DEV_INITRD case so that you can simply call them
here, regardless of the CONFIG_BLK_DEV_INITRD setting.

Then you won't need boot_params ptr either and that would simplify the
diff a bit.

Thx.
Tom Lendacky - Dec. 21, 2017, 4:48 p.m.
On 12/21/2017 8:49 AM, Borislav Petkov wrote:
> On Thu, Dec 07, 2017 at 05:34:10PM -0600, Tom Lendacky wrote:
>> Currently the BSP microcode update code examines the initrd very early
>> in the boot process.  If SME is active, the initrd is treated as being
>> encrypted but it has not been encrypted (in place) yet.  Update the
>> early boot code that encrypts the kernel to also encrypt the initrd so
>> that early BSP microcode updates work.
> 
> ...
> 
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index 1f0efb8..60df247 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -731,11 +731,12 @@ static unsigned long __init sme_pgtable_calc(unsigned long len)
>>  	return total;
>>  }
>>  
>> -void __init sme_encrypt_kernel(void)
>> +void __init sme_encrypt_kernel(struct boot_params *bp)
>>  {
>>  	unsigned long workarea_start, workarea_end, workarea_len;
>>  	unsigned long execute_start, execute_end, execute_len;
>>  	unsigned long kernel_start, kernel_end, kernel_len;
>> +	unsigned long initrd_start, initrd_end, initrd_len;
>>  	unsigned long pgtable_area_len;
>>  	unsigned long decrypted_base;
>>  	pgd_t *pgd;
>> @@ -744,14 +745,15 @@ void __init sme_encrypt_kernel(void)
>>  		return;
>>  
>>  	/*
>> -	 * Prepare for encrypting the kernel by building new pagetables with
>> -	 * the necessary attributes needed to encrypt the kernel in place.
>> +	 * Prepare for encrypting the kernel and initrd by building new
>> +	 * pagetables with the necessary attributes needed to encrypt the
>> +	 * kernel in place.
>>  	 *
>>  	 *   One range of virtual addresses will map the memory occupied
>> -	 *   by the kernel as encrypted.
>> +	 *   by the kernel and initrd as encrypted.
>>  	 *
>>  	 *   Another range of virtual addresses will map the memory occupied
>> -	 *   by the kernel as decrypted and write-protected.
>> +	 *   by the kernel and initrd as decrypted and write-protected.
>>  	 *
>>  	 *     The use of write-protect attribute will prevent any of the
>>  	 *     memory from being cached.
>> @@ -762,6 +764,20 @@ void __init sme_encrypt_kernel(void)
>>  	kernel_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE);
>>  	kernel_len = kernel_end - kernel_start;
>>  
>> +	initrd_start = 0;
>> +	initrd_end = 0;
>> +	initrd_len = 0;
>> +#ifdef CONFIG_BLK_DEV_INITRD
>> +	initrd_len = (unsigned long)bp->hdr.ramdisk_size |
>> +		     ((unsigned long)bp->ext_ramdisk_size << 32);
>> +	if (initrd_len) {
>> +		initrd_start = (unsigned long)bp->hdr.ramdisk_image |
>> +			       ((unsigned long)bp->ext_ramdisk_image << 32);
>> +		initrd_end = PAGE_ALIGN(initrd_start + initrd_len);
>> +		initrd_len = initrd_end - initrd_start;
>> +	}
>> +#endif
> 
> In a prepatch, pls make get_ramdisk_image() and get_ramdisk_size() from
> arch/x86/kernel/setup.c accessible to this code too. Also, add dummies
> for the !CONFIG_BLK_DEV_INITRD case so that you can simply call them
> here, regardless of the CONFIG_BLK_DEV_INITRD setting.
> 
> Then you won't need boot_params ptr either and that would simplify the
> diff a bit.

This is very early in the boot and the boot parameters have not been
copied to boot_params yet, so I need the pointer.  And since the routines
in arch/x86/kernel/setup.c also use boot_params, those would have to be
modified to accept a pointer rather than automatically using boot_params.
I'm not sure it's worth all that.

Thanks,
Tom

> 
> Thx.
>
Borislav Petkov - Dec. 21, 2017, 5:13 p.m.
On Thu, Dec 21, 2017 at 10:48:29AM -0600, Tom Lendacky wrote:
> This is very early in the boot and the boot parameters have not been
> copied to boot_params yet, so I need the pointer.  And since the routines
> in arch/x86/kernel/setup.c also use boot_params, those would have to be
> modified to accept a pointer rather than automatically using boot_params.
> I'm not sure it's worth all that.

I fear that this computation of the ramdisk address and size keeps
duplicating around the tree. And you could pass in struct boot_params *
to those functions but ok, it is simple enough, so whatever. We can
always clean it up later if it grows unwieldy...

Patch

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index c9459a4..22c5f3e 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -39,7 +39,7 @@  void __init sme_early_decrypt(resource_size_t paddr,
 
 void __init sme_early_init(void);
 
-void __init sme_encrypt_kernel(void);
+void __init sme_encrypt_kernel(struct boot_params *bp);
 void __init sme_enable(struct boot_params *bp);
 
 int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
@@ -67,7 +67,7 @@  static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
 
 static inline void __init sme_early_init(void) { }
 
-static inline void __init sme_encrypt_kernel(void) { }
+static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
 static inline void __init sme_enable(struct boot_params *bp) { }
 
 static inline bool sme_active(void) { return false; }
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 6a5d757..7ba5d81 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -157,8 +157,8 @@  unsigned long __head __startup_64(unsigned long physaddr,
 	p = fixup_pointer(&phys_base, physaddr);
 	*p += load_delta - sme_get_me_mask();
 
-	/* Encrypt the kernel (if SME is active) */
-	sme_encrypt_kernel();
+	/* Encrypt the kernel and related (if SME is active) */
+	sme_encrypt_kernel(bp);
 
 	/*
 	 * Return the SME encryption mask (if SME is active) to be used as a
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 8af2e8d..8f2af5d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -364,16 +364,6 @@  static void __init reserve_initrd(void)
 	    !ramdisk_image || !ramdisk_size)
 		return;		/* No initrd provided by bootloader */
 
-	/*
-	 * If SME is active, this memory will be marked encrypted by the
-	 * kernel when it is accessed (including relocation). However, the
-	 * ramdisk image was loaded decrypted by the bootloader, so make
-	 * sure that it is encrypted before accessing it. For SEV the
-	 * ramdisk will already be encrypted, so only do this for SME.
-	 */
-	if (sme_active())
-		sme_early_encrypt(ramdisk_image, ramdisk_end - ramdisk_image);
-
 	initrd_start = 0;
 
 	mapped_size = memblock_mem_size(max_pfn_mapped);
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 1f0efb8..60df247 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -731,11 +731,12 @@  static unsigned long __init sme_pgtable_calc(unsigned long len)
 	return total;
 }
 
-void __init sme_encrypt_kernel(void)
+void __init sme_encrypt_kernel(struct boot_params *bp)
 {
 	unsigned long workarea_start, workarea_end, workarea_len;
 	unsigned long execute_start, execute_end, execute_len;
 	unsigned long kernel_start, kernel_end, kernel_len;
+	unsigned long initrd_start, initrd_end, initrd_len;
 	unsigned long pgtable_area_len;
 	unsigned long decrypted_base;
 	pgd_t *pgd;
@@ -744,14 +745,15 @@  void __init sme_encrypt_kernel(void)
 		return;
 
 	/*
-	 * Prepare for encrypting the kernel by building new pagetables with
-	 * the necessary attributes needed to encrypt the kernel in place.
+	 * Prepare for encrypting the kernel and initrd by building new
+	 * pagetables with the necessary attributes needed to encrypt the
+	 * kernel in place.
 	 *
 	 *   One range of virtual addresses will map the memory occupied
-	 *   by the kernel as encrypted.
+	 *   by the kernel and initrd as encrypted.
 	 *
 	 *   Another range of virtual addresses will map the memory occupied
-	 *   by the kernel as decrypted and write-protected.
+	 *   by the kernel and initrd as decrypted and write-protected.
 	 *
 	 *     The use of write-protect attribute will prevent any of the
 	 *     memory from being cached.
@@ -762,6 +764,20 @@  void __init sme_encrypt_kernel(void)
 	kernel_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE);
 	kernel_len = kernel_end - kernel_start;
 
+	initrd_start = 0;
+	initrd_end = 0;
+	initrd_len = 0;
+#ifdef CONFIG_BLK_DEV_INITRD
+	initrd_len = (unsigned long)bp->hdr.ramdisk_size |
+		     ((unsigned long)bp->ext_ramdisk_size << 32);
+	if (initrd_len) {
+		initrd_start = (unsigned long)bp->hdr.ramdisk_image |
+			       ((unsigned long)bp->ext_ramdisk_image << 32);
+		initrd_end = PAGE_ALIGN(initrd_start + initrd_len);
+		initrd_len = initrd_end - initrd_start;
+	}
+#endif
+
 	/* Set the encryption workarea to be immediately after the kernel */
 	workarea_start = kernel_end;
 
@@ -784,6 +800,8 @@  void __init sme_encrypt_kernel(void)
 	 */
 	pgtable_area_len = sizeof(pgd_t) * PTRS_PER_PGD;
 	pgtable_area_len += sme_pgtable_calc(execute_end - kernel_start) * 2;
+	if (initrd_len)
+		pgtable_area_len += sme_pgtable_calc(initrd_len) * 2;
 
 	/* PUDs and PMDs needed in the current pagetables for the workarea */
 	pgtable_area_len += sme_pgtable_calc(execute_len + pgtable_area_len);
@@ -820,9 +838,9 @@  void __init sme_encrypt_kernel(void)
 
 	/*
 	 * A new pagetable structure is being built to allow for the kernel
-	 * to be encrypted. It starts with an empty PGD that will then be
-	 * populated with new PUDs and PMDs as the encrypted and decrypted
-	 * kernel mappings are created.
+	 * and initrd to be encrypted. It starts with an empty PGD that will
+	 * then be populated with new PUDs and PMDs as the encrypted and
+	 * decrypted kernel mappings are created.
 	 */
 	pgd = pgtable_area;
 	memset(pgd, 0, sizeof(*pgd) * PTRS_PER_PGD);
@@ -835,6 +853,12 @@  void __init sme_encrypt_kernel(void)
 	 * the base of the mapping.
 	 */
 	decrypted_base = (pgd_index(workarea_end) + 1) & (PTRS_PER_PGD - 1);
+	if (initrd_len) {
+		unsigned long check_base;
+
+		check_base = (pgd_index(initrd_end) + 1) & (PTRS_PER_PGD - 1);
+		decrypted_base = max(decrypted_base, check_base);
+	}
 	decrypted_base <<= PGDIR_SHIFT;
 
 	/* Add encrypted kernel (identity) mappings */
@@ -844,7 +868,19 @@  void __init sme_encrypt_kernel(void)
 	sme_map_range_decrypted_wp(pgd, kernel_start + decrypted_base,
 				   kernel_end + decrypted_base, kernel_start);
 
-	/* Add decrypted workarea mappings to both kernel mappings */
+	if (initrd_len) {
+		/* Add encrypted initrd (identity) mappings */
+		sme_map_range_encrypted(pgd, initrd_start, initrd_end,
+					initrd_start);
+		/*
+		 * Add decrypted, write-protected initrd (non-identity) mappings
+		 */
+		sme_map_range_decrypted_wp(pgd, initrd_start + decrypted_base,
+					   initrd_end + decrypted_base,
+					   initrd_start);
+	}
+
+	/* Add decrypted workarea mappings to both mappings */
 	sme_map_range_decrypted(pgd, workarea_start, workarea_end,
 				workarea_start);
 	sme_map_range_decrypted(pgd, workarea_start + decrypted_base,
@@ -854,6 +890,10 @@  void __init sme_encrypt_kernel(void)
 	/* Perform the encryption */
 	sme_encrypt_execute(kernel_start, kernel_start + decrypted_base,
 			    kernel_len, workarea_start, (unsigned long)pgd);
+	if (initrd_len)
+		sme_encrypt_execute(initrd_start, initrd_start + decrypted_base,
+				    initrd_len, workarea_start,
+				    (unsigned long)pgd);
 
 	/*
 	 * At this point we are running encrypted.  Remove the mappings for
@@ -863,6 +903,10 @@  void __init sme_encrypt_kernel(void)
 	sme_clear_pgd(pgd, kernel_start + decrypted_base,
 		      kernel_end + decrypted_base);
 
+	if (initrd_len)
+		sme_clear_pgd(pgd, initrd_start + decrypted_base,
+			      initrd_end + decrypted_base);
+
 	sme_clear_pgd(pgd, workarea_start + decrypted_base,
 		      workarea_end + decrypted_base);
 
diff --git a/arch/x86/mm/mem_encrypt_boot.S b/arch/x86/mm/mem_encrypt_boot.S
index 20cca86..6132015 100644
--- a/arch/x86/mm/mem_encrypt_boot.S
+++ b/arch/x86/mm/mem_encrypt_boot.S
@@ -22,9 +22,9 @@  ENTRY(sme_encrypt_execute)
 
 	/*
 	 * Entry parameters:
-	 *   RDI - virtual address for the encrypted kernel mapping
-	 *   RSI - virtual address for the decrypted kernel mapping
-	 *   RDX - length of kernel
+	 *   RDI - virtual address for the encrypted mapping
+	 *   RSI - virtual address for the decrypted mapping
+	 *   RDX - length to encrypt
 	 *   RCX - virtual address of the encryption workarea, including:
 	 *     - stack page (PAGE_SIZE)
 	 *     - encryption routine page (PAGE_SIZE)
@@ -41,9 +41,9 @@  ENTRY(sme_encrypt_execute)
 	addq	$PAGE_SIZE, %rax	/* Workarea encryption routine */
 
 	push	%r12
-	movq	%rdi, %r10		/* Encrypted kernel */
-	movq	%rsi, %r11		/* Decrypted kernel */
-	movq	%rdx, %r12		/* Kernel length */
+	movq	%rdi, %r10		/* Encrypted area */
+	movq	%rsi, %r11		/* Decrypted area */
+	movq	%rdx, %r12		/* Area length */
 
 	/* Copy encryption routine into the workarea */
 	movq	%rax, %rdi				/* Workarea encryption routine */
@@ -52,10 +52,10 @@  ENTRY(sme_encrypt_execute)
 	rep	movsb
 
 	/* Setup registers for call */
-	movq	%r10, %rdi		/* Encrypted kernel */
-	movq	%r11, %rsi		/* Decrypted kernel */
+	movq	%r10, %rdi		/* Encrypted area */
+	movq	%r11, %rsi		/* Decrypted area */
 	movq	%r8, %rdx		/* Pagetables used for encryption */
-	movq	%r12, %rcx		/* Kernel length */
+	movq	%r12, %rcx		/* Area length */
 	movq	%rax, %r8		/* Workarea encryption routine */
 	addq	$PAGE_SIZE, %r8		/* Workarea intermediate copy buffer */
 
@@ -71,7 +71,7 @@  ENDPROC(sme_encrypt_execute)
 
 ENTRY(__enc_copy)
 /*
- * Routine used to encrypt kernel.
+ * Routine used to encrypt memory in place.
  *   This routine must be run outside of the kernel proper since
  *   the kernel will be encrypted during the process. So this
  *   routine is defined here and then copied to an area outside
@@ -79,19 +79,19 @@  ENTRY(__enc_copy)
  *   during execution.
  *
  *   On entry the registers must be:
- *     RDI - virtual address for the encrypted kernel mapping
- *     RSI - virtual address for the decrypted kernel mapping
+ *     RDI - virtual address for the encrypted mapping
+ *     RSI - virtual address for the decrypted mapping
  *     RDX - address of the pagetables to use for encryption
- *     RCX - length of kernel
+ *     RCX - length of area
  *      R8 - intermediate copy buffer
  *
  *     RAX - points to this routine
  *
- * The kernel will be encrypted by copying from the non-encrypted
- * kernel space to an intermediate buffer and then copying from the
- * intermediate buffer back to the encrypted kernel space. The physical
- * addresses of the two kernel space mappings are the same which
- * results in the kernel being encrypted "in place".
+ * The area will be encrypted by copying from the non-encrypted
+ * memory space to an intermediate buffer and then copying from the
+ * intermediate buffer back to the encrypted memory space. The physical
+ * addresses of the two mappings are the same which results in the area
+ * being encrypted "in place".
  */
 	/* Enable the new page tables */
 	mov	%rdx, %cr3
@@ -114,9 +114,9 @@  ENTRY(__enc_copy)
 	pop	%rdx			/* RDX contains original PAT value */
 	pop	%rcx
 
-	movq	%rcx, %r9		/* Save kernel length */
-	movq	%rdi, %r10		/* Save encrypted kernel address */
-	movq	%rsi, %r11		/* Save decrypted kernel address */
+	movq	%rcx, %r9		/* Save area length */
+	movq	%rdi, %r10		/* Save encrypted area address */
+	movq	%rsi, %r11		/* Save decrypted area address */
 
 	wbinvd				/* Invalidate any cache entries */
 
@@ -130,13 +130,13 @@  ENTRY(__enc_copy)
 	movq	%r9, %r12
 
 2:
-	movq	%r11, %rsi		/* Source - decrypted kernel */
+	movq	%r11, %rsi		/* Source - decrypted area */
 	movq	%r8, %rdi		/* Dest   - intermediate copy buffer */
 	movq	%r12, %rcx
 	rep	movsb
 
 	movq	%r8, %rsi		/* Source - intermediate copy buffer */
-	movq	%r10, %rdi		/* Dest   - encrypted kernel */
+	movq	%r10, %rdi		/* Dest   - encrypted area */
 	movq	%r12, %rcx
 	rep	movsb