Re: [PATCH v9 5/6] x86/sev: Add SNP-specific unaccepted memory support

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

On 9/6/23 09:04, Christopher Schramm wrote:
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5c72067c06d4..b9c451f75d5e 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1543,11 +1543,13 @@ config X86_MEM_ENCRYPT
>>   config AMD_MEM_ENCRYPT
>>       bool "AMD Secure Memory Encryption (SME) support"
>>       depends on X86_64 && CPU_SUP_AMD
>> +    depends on EFI_STUB
>>       select DMA_COHERENT_POOL
>>       select ARCH_USE_MEMREMAP_PROT
>>       select INSTRUCTION_DECODER
>>       select ARCH_HAS_CC_PLATFORM
>>       select X86_MEM_ENCRYPT
>> +    select UNACCEPTED_MEMORY
>>       help
>>         Say yes to enable support for the encryption of system memory.
>>         This requires an AMD processor that supports Secure Memory
> 
> Unfortunately this makes AMD_MEM_ENCRYPT depend on EFI just to 
> unconditionally enable UNACCEPTED_MEMORY. It seems like an easy target to 
> make that optional, e.g. with a separate configuration item:
> 
> ---
> config AMD_UNACCEPTED_MEMORY
>         def_bool y
>         depends on AMD_MEM_ENCRYPT && EFI_STUB
>         select UNACCEPTED_MEMORY
> ---
> 
> Using that we can successfully build and run SNP VMs without UEFI/OVMF 
> (which we already did with earlier Linux versions).

This seems reasonable to me.

I would recommend naming it AMD_MEM_ENCRYPT_UNACCEPTED_MEMORY to keep the 
association and add a prompt along the lines of "AMD Secure Encrypted 
Virtualization (SEV) unaccepted memory support" (I don't really see this 
used with bare-metal, but who knows.)

I think there will need to be changes elsewhere in the kernel to support 
this, though. A quick build test with just AMD_UNACCEPTED_MEMORY not being 
set broke the build when EFI_STUB was still set to y.

> 
>  From a quick look at
> 
>    [PATCHv14 9/9] x86/tdx: Add unaccepted memory support
> 
> it actually seems very similar for INTEL_TDX_GUEST.
> 
> Ideally UNACCEPTED_MEMORY would not assume EFI either, but the 
> implementation actually clearly does.

@Kirill, is this something you are interested in having as well?

Thanks,
Tom