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

[Tom Lendacky <thomas.lendacky@amd.com>]

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.
>