Re: [PATCHv13 9/9] x86/tdx: Add unaccepted memory support
From: Kirill A. Shutemov
Date: Fri Jun 02 2023 - 10:35:47 EST
On Fri, Jun 02, 2023 at 08:22:35AM -0500, Tom Lendacky wrote:
> On 6/1/23 13:25, Kirill A. Shutemov wrote:
> > Hookup TDX-specific code to accept memory.
> >
> > Accepting the memory is done with ACCEPT_PAGE module call on every page
> > in the range. MAP_GPA hypercall is not required as the unaccepted memory
> > is considered private already.
> >
> > Extract the part of tdx_enc_status_changed() that does memory acceptance
> > in a new helper. Move the helper tdx-shared.c. It is going to be used by
> > both main kernel and decompressor.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > ---
> > arch/x86/Kconfig | 2 +
> > arch/x86/boot/compressed/Makefile | 2 +-
> > arch/x86/boot/compressed/error.c | 19 +++++++
> > arch/x86/boot/compressed/error.h | 1 +
> > arch/x86/boot/compressed/mem.c | 35 +++++++++++-
> > arch/x86/boot/compressed/tdx-shared.c | 2 +
> > arch/x86/coco/tdx/Makefile | 2 +-
> > arch/x86/coco/tdx/tdx-shared.c | 71 ++++++++++++++++++++++++
> > arch/x86/coco/tdx/tdx.c | 70 +----------------------
> > arch/x86/include/asm/shared/tdx.h | 2 +
> > arch/x86/include/asm/unaccepted_memory.h | 24 ++++++++
> > 11 files changed, 160 insertions(+), 70 deletions(-)
> > create mode 100644 arch/x86/boot/compressed/tdx-shared.c
> > create mode 100644 arch/x86/coco/tdx/tdx-shared.c
> > create mode 100644 arch/x86/include/asm/unaccepted_memory.h
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 53bab123a8ee..5c72067c06d4 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -884,9 +884,11 @@ config INTEL_TDX_GUEST
> > bool "Intel TDX (Trust Domain Extensions) - Guest Support"
> > depends on X86_64 && CPU_SUP_INTEL
> > depends on X86_X2APIC
> > + depends on EFI_STUB
> > select ARCH_HAS_CC_PLATFORM
> > select X86_MEM_ENCRYPT
> > select X86_MCE
> > + select UNACCEPTED_MEMORY
> > help
> > Support running as a guest under Intel TDX. Without this support,
> > the guest kernel can not boot or run under TDX.
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index cc4978123c30..b13a58021086 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -106,7 +106,7 @@ ifdef CONFIG_X86_64
> > endif
> > vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
> > -vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdx.o $(obj)/tdcall.o
> > +vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdx.o $(obj)/tdcall.o $(obj)/tdx-shared.o
> > vmlinux-objs-$(CONFIG_UNACCEPTED_MEMORY) += $(obj)/mem.o
> > vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
> > diff --git a/arch/x86/boot/compressed/error.c b/arch/x86/boot/compressed/error.c
> > index c881878e56d3..5313c5cb2b80 100644
> > --- a/arch/x86/boot/compressed/error.c
> > +++ b/arch/x86/boot/compressed/error.c
> > @@ -22,3 +22,22 @@ void error(char *m)
> > while (1)
> > asm("hlt");
> > }
> > +
> > +/* EFI libstub provides vsnprintf() */
> > +#ifdef CONFIG_EFI_STUB
> > +void panic(const char *fmt, ...)
> > +{
> > + static char buf[1024];
> > + va_list args;
> > + int len;
> > +
> > + va_start(args, fmt);
> > + len = vsnprintf(buf, sizeof(buf), fmt, args);
> > + va_end(args);
> > +
> > + if (len && buf[len - 1] == '\n')
> > + buf[len - 1] = '\0';
> > +
> > + error(buf);
> > +}
> > +#endif
> > diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h
> > index 1de5821184f1..86fe33b93715 100644
> > --- a/arch/x86/boot/compressed/error.h
> > +++ b/arch/x86/boot/compressed/error.h
> > @@ -6,5 +6,6 @@
> > void warn(char *m);
> > void error(char *m) __noreturn;
> > +void panic(const char *fmt, ...) __noreturn __cold;
> > #endif /* BOOT_COMPRESSED_ERROR_H */
> > diff --git a/arch/x86/boot/compressed/mem.c b/arch/x86/boot/compressed/mem.c
> > index 4ecf26576a77..d2b6948a7801 100644
> > --- a/arch/x86/boot/compressed/mem.c
> > +++ b/arch/x86/boot/compressed/mem.c
> > @@ -2,11 +2,44 @@
> > #include "error.h"
> > #include "misc.h"
> > +#include "tdx.h"
> > +#include <asm/shared/tdx.h>
> > +
> > +/*
> > + * accept_memory() and process_unaccepted_memory() called from EFI stub which
> > + * runs before decompresser and its early_tdx_detect().
> > + *
> > + * Enumerate TDX directly from the early users.
> > + */
> > +static bool early_is_tdx_guest(void)
> > +{
> > + static bool once;
> > + static bool is_tdx;
> > +
> > + if (!IS_ENABLED(CONFIG_INTEL_TDX_GUEST))
> > + return false;
> > +
> > + if (!once) {
> > + u32 eax, sig[3];
> > +
> > + cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax,
> > + &sig[0], &sig[2], &sig[1]);
> > + is_tdx = !memcmp(TDX_IDENT, sig, sizeof(sig));
> > + once = true;
> > + }
> > +
> > + return is_tdx;
> > +}
> > void arch_accept_memory(phys_addr_t start, phys_addr_t end)
> > {
> > /* Platform-specific memory-acceptance call goes here */
> > - error("Cannot accept memory");
> > + if (early_is_tdx_guest()) {
> > + if (tdx_accept_memory(start, end))
> > + return;
> > + }
> > +
> > + error("Cannot accept memory: unknown platform\n");
>
> So this is a change in this version. If tdx_accept_memory() fails, you'll
> report unknown platform. Wouldn't it be better to have an error message that
> indicates a failure in the accept path?
Urgh.. Didn't read the error message on the rework.
diff --git a/arch/x86/boot/compressed/mem.c b/arch/x86/boot/compressed/mem.c
index d2b6948a7801..a0d24df1004d 100644
--- a/arch/x86/boot/compressed/mem.c
+++ b/arch/x86/boot/compressed/mem.c
@@ -35,11 +35,11 @@ void arch_accept_memory(phys_addr_t start, phys_addr_t end)
{
/* Platform-specific memory-acceptance call goes here */
if (early_is_tdx_guest()) {
- if (tdx_accept_memory(start, end))
- return;
+ if (!tdx_accept_memory(start, end))
+ panic("TDX: Failed to accept memory\n");
+ } else {
+ error("Cannot accept memory: unknown platform\n");
}
-
- error("Cannot accept memory: unknown platform\n");
}
void init_unaccepted_memory(void)
diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h
index f0ab217b566f..572514e36fde 100644
--- a/arch/x86/include/asm/unaccepted_memory.h
+++ b/arch/x86/include/asm/unaccepted_memory.h
@@ -8,11 +8,11 @@ static inline void arch_accept_memory(phys_addr_t start, phys_addr_t end)
{
/* Platform-specific memory-acceptance call goes here */
if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
- if (tdx_accept_memory(start, end))
- return;
+ if (!tdx_accept_memory(start, end))
+ panic("TDX: Failed to accept memory\n");
+ } else {
+ panic("Cannot accept memory: unknown platform\n");
}
-
- panic("Cannot accept memory: unknown platform\n");
}
static inline struct efi_unaccepted_memory *efi_get_unaccepted_table(void)
--
Kiryl Shutsemau / Kirill A. Shutemov