Re: [PATCH] x86/mm: Create an SME workarea in the kernel for early encryption

["Lendacky, Thomas" <Thomas.Lendacky@amd.com>]

On 6/13/19 12:47 PM, Dave Hansen wrote:
> On 6/12/19 10:46 AM, Lendacky, Thomas wrote:
>> On 6/12/19 10:00 AM, Dave Hansen wrote:
>>> On 6/12/19 6:32 AM, Lendacky, Thomas wrote:
>>>> Create a section for SME in the vmlinux.lds.S.  Position it after "_end"
>>>> so that the memory will be reclaimed during boot and, since it is all
>>>> zeroes, it compresses well. 
>>>
>>> I don't think I realized that things after _end get reclaimed.  Do we do
>>> that at the same spot that we do init data or somewhere else?
>>
>> I was looking at the start of setup_arch() in arch/x86/kernel/setup.c,
>> where there's a memblock_reserve() done for the kernel (it reserves from
>> _text to __bss_stop, not all the way to _end, and later the brk area
>> is reserved). At that point, my take was that the memory outside the
>> reserved area is now available (and there's a comment below that to that
>> effect, also), so the .sme section would basically be discarded and
>> re-claimed for general page usage.
> 
> This seems awfully subtle.  This would be the only section treated this
> way because, as you note, even the '.brk' area ends up getting
> memblock_reserve()'d.  Also, this odd property is not commented on at all.
> 
> That's not the end of the world.  But, if we're going to do this, it
> seems like we need to move the:
> 
> 	/* Sections to be discarded /*
> 
> comment to up above your new area.  It also seems like we need something
> explicit in there near __bss_stop saying:
> 
> 	/*
>    	 * Everything between _text and here is automatically reserved
> 	 * in setup_arch().  Everything after here must either have its
> 	 * own memblock_reserve(), or it will be treated as available
> 	 * memory and freed at boot.
> 	 */

That all makes sense.

> 
> Actually, I wonder if we should add a symbol called
> '__end_of_kernel_reserve' and use *that* instead of __bss_stop in
> setup_arch().

If everyone thinks that's best, I can do that. Probably best as a
pre-patch and make this into a 2-patch series, then.

> 
> After I say all that...  Why can't you just stick your data in a normal,
> vanilla __init variable?  Wouldn't that be a lot less subtle?

The area needs to be outside of the kernel proper as the kernel is
encrypted "in place." So an __init variable won't work here.

Thanks,
Tom

>