lkml.org 
[lkml]   [2021]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC v1 21/26] x86/mm: Move force_dma_unencrypted() to common code
On Thu, Apr 01, 2021 at 01:06:29PM -0700, Dave Hansen wrote:
> On 2/5/21 3:38 PM, Kuppuswamy Sathyanarayanan wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Intel TDX doesn't allow VMM to access guest memory. Any memory that is
> > required for communication with VMM suppose to be shared explicitly by
>
> s/suppose to/must/

Right.

> > setting the bit in page table entry. The shared memory is similar to
> > unencrypted memory in AMD SME/SEV terminology.
>
> In addition to setting the page table bit, there's also a dance to go
> through to convert the memory. Please mention the procedure here at
> least. It's very different from SME.

"
After setting the shared bit, the conversion must be completed with
MapGPA TDVMALL. The call informs VMM about the conversion and makes it
remove the GPA from the S-EPT mapping.
"

> > force_dma_unencrypted() has to return true for TDX guest. Move it out of
> > AMD SME code.
>
> You lost me here. What does force_dma_unencrypted() have to do with
> host/guest shared memory?

"
AMD SEV makes force_dma_unencrypted() return true which triggers
set_memory_decrypted() calls on all DMA allocations. TDX will use the
same code path to make DMA allocations shared.
"

> > Introduce new config option X86_MEM_ENCRYPT_COMMON that has to be
> > selected by all x86 memory encryption features.
>
> Please also mention what will set it. I assume TDX guest support will
> set this option. It's probably also worth a sentence to say that
> force_dma_unencrypted() will have TDX-specific code added to it. (It
> will, right??)

"
Only AMD_MEM_ENCRYPT uses the option now. TDX will be the second one.
"

> > This is preparation for TDX changes in DMA code.
>
> Probably best to also mention that this effectively just moves code
> around. This patch should have no functional changes at runtime.

Isn't it what the subject says? :P

> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 0374d9f262a5..8fa654d61ac2 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1538,14 +1538,18 @@ config X86_CPA_STATISTICS
> > helps to determine the effectiveness of preserving large and huge
> > page mappings when mapping protections are changed.
> >
> > +config X86_MEM_ENCRYPT_COMMON
> > + select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> > + select DYNAMIC_PHYSICAL_MASK
> > + def_bool n
> > +
> > config AMD_MEM_ENCRYPT
> > bool "AMD Secure Memory Encryption (SME) support"
> > depends on X86_64 && CPU_SUP_AMD
> > select DMA_COHERENT_POOL
> > - select DYNAMIC_PHYSICAL_MASK
> > select ARCH_USE_MEMREMAP_PROT
> > - select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> > select INSTRUCTION_DECODER
> > + select X86_MEM_ENCRYPT_COMMON
> > help
> > Say yes to enable support for the encryption of system memory.
> > This requires an AMD processor that supports Secure Memory
> > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > index 30a3b30395ad..95e534cffa99 100644
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -257,10 +257,12 @@ static inline void slow_down_io(void)
> >
> > #endif
> >
> > -#ifdef CONFIG_AMD_MEM_ENCRYPT
> > #include <linux/jump_label.h>
> >
> > extern struct static_key_false sev_enable_key;
>
> This _looks_ odd. sev_enable_key went from being under
> CONFIG_AMD_MEM_ENCRYPT to being unconditionally referenced.

Not referenced, but declared.

> Could you explain a bit more?
>
> I would have expected it tot at *least* be tied to the new #ifdef.

Looks like a fixup got folded into a wrong patch. It supposed to be in
"x86/kvm: Use bounce buffers for TD guest".

This declaration allows to get away without any #ifdefs in
mem_encrypt_init() when !CONFIG_AMD_MEM_ENCRYPT: sev_active() is
false at compile-time and sev_enable_key never referenced.

Sathya, could move it to the right patch?

> > +#ifdef CONFIG_AMD_MEM_ENCRYPT
> > +
> > static inline bool sev_key_active(void)
> > {
> > return static_branch_unlikely(&sev_enable_key);
> > diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
> > index 5864219221ca..b31cb52bf1bd 100644
> > --- a/arch/x86/mm/Makefile
> ...

--
Kirill A. Shutemov

\
 
 \ /
  Last update: 2021-04-06 17:39    [W:0.179 / U:0.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site