On Wed, Aug 11, 2021 at 10:52:55AM -0500, Tom Lendacky wrote:
On 8/11/21 7:19 AM, Kirill A. Shutemov wrote:
On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:
On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:
On 7/27/21 3:26 PM, Tom Lendacky wrote:
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index de01903c3735..cafed6456d45 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -19,7 +19,7 @@ #include <linux/start_kernel.h> #include <linux/io.h> #include <linux/memblock.h> -#include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <linux/pgtable.h> #include <asm/processor.h> @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr, * there is no need to zero it after changing the memory encryption * attribute. */ - if (mem_encrypt_active()) { + if (prot_guest_has(PATTR_MEM_ENCRYPT)) { vaddr = (unsigned long)__start_bss_decrypted; vaddr_end = (unsigned long)__end_bss_decrypted;
Since this change is specific to AMD, can you replace PATTR_MEM_ENCRYPT with prot_guest_has(PATTR_SME) || prot_guest_has(PATTR_SEV). It is not used in TDX.
This is a direct replacement for now.
With current implementation of prot_guest_has() for TDX it breaks boot for me.
Looking at code agains, now I *think* the reason is accessing a global variable from __startup_64() inside TDX version of prot_guest_has().
__startup_64() is special. If you access any global variable you need to use fixup_pointer(). See comment before __startup_64().
I'm not sure how you get away with accessing sme_me_mask directly from there. Any clues? Maybe just a luck and complier generates code just right for your case, I donno.
Hmm... yeah, could be that the compiler is using rip-relative addressing for it because it lives in the .data section?
I guess. It has to be fixed. It may break with complier upgrade or any random change around the code.
BTW, does it work with clang for you?
For the static variables in mem_encrypt_identity.c I did an assembler rip relative LEA, but probably could have passed physaddr to sme_enable() and used a fixup_pointer() style function, instead.
Sounds like a plan.
A separate point is that TDX version of prot_guest_has() relies on cpu_feature_enabled() which is not ready at this point.
Does TDX have to do anything special to make memory able to be shared with the hypervisor?
Yes. But there's nothing that required any changes in early boot. It handled in ioremap/set_memory.
You might have to use something that is available earlier than cpu_feature_enabled() in that case (should you eventually support kvmclock).
Maybe.
I think __bss_decrypted fixup has to be done if sme_me_mask is non-zero. Or just do it uncoditionally because it's NOP for sme_me_mask == 0.
For SNP, we'll have to additionally call the HV to update the RMP to make the memory shared. But that could also be done unconditionally since the early_snp_set_memory_shared() routine will check for SNP before doing anything.