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

[Tom Lendacky]

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