Messages in this thread | | | Date | Fri, 2 Jun 2023 08:22:35 -0500 | Subject | Re: [PATCHv13 9/9] x86/tdx: Add unaccepted memory support | From | Tom Lendacky <> |
| |
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@linux.intel.com> > --- > 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?
Thanks, Tom
> } > > void init_unaccepted_memory(void) > diff --git a/arch/x86/boot/compressed/tdx-shared.c b/arch/x86/boot/compressed/tdx-shared.c > new file mode 100644 > index 000000000000..5ac43762fe13 > --- /dev/null > +++ b/arch/x86/boot/compressed/tdx-shared.c > @@ -0,0 +1,2 @@ > +#include "error.h" > +#include "../../coco/tdx/tdx-shared.c" > diff --git a/arch/x86/coco/tdx/Makefile b/arch/x86/coco/tdx/Makefile > index 46c55998557d..2c7dcbf1458b 100644 > --- a/arch/x86/coco/tdx/Makefile > +++ b/arch/x86/coco/tdx/Makefile > @@ -1,3 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0 > > -obj-y += tdx.o tdcall.o > +obj-y += tdx.o tdx-shared.o tdcall.o > diff --git a/arch/x86/coco/tdx/tdx-shared.c b/arch/x86/coco/tdx/tdx-shared.c > new file mode 100644 > index 000000000000..ef20ddc37b58 > --- /dev/null > +++ b/arch/x86/coco/tdx/tdx-shared.c > @@ -0,0 +1,71 @@ > +#include <asm/tdx.h> > +#include <asm/pgtable.h> > + > +static unsigned long try_accept_one(phys_addr_t start, unsigned long len, > + enum pg_level pg_level) > +{ > + unsigned long accept_size = page_level_size(pg_level); > + u64 tdcall_rcx; > + u8 page_size; > + > + if (!IS_ALIGNED(start, accept_size)) > + return 0; > + > + if (len < accept_size) > + return 0; > + > + /* > + * Pass the page physical address to the TDX module to accept the > + * pending, private page. > + * > + * Bits 2:0 of RCX encode page size: 0 - 4K, 1 - 2M, 2 - 1G. > + */ > + switch (pg_level) { > + case PG_LEVEL_4K: > + page_size = 0; > + break; > + case PG_LEVEL_2M: > + page_size = 1; > + break; > + case PG_LEVEL_1G: > + page_size = 2; > + break; > + default: > + return 0; > + } > + > + tdcall_rcx = start | page_size; > + if (__tdx_module_call(TDX_ACCEPT_PAGE, tdcall_rcx, 0, 0, 0, NULL)) > + return 0; > + > + return accept_size; > +} > + > +bool tdx_accept_memory(phys_addr_t start, phys_addr_t end) > +{ > + /* > + * For shared->private conversion, accept the page using > + * TDX_ACCEPT_PAGE TDX module call. > + */ > + while (start < end) { > + unsigned long len = end - start; > + unsigned long accept_size; > + > + /* > + * Try larger accepts first. It gives chance to VMM to keep > + * 1G/2M Secure EPT entries where possible and speeds up > + * process by cutting number of hypercalls (if successful). > + */ > + > + accept_size = try_accept_one(start, len, PG_LEVEL_1G); > + if (!accept_size) > + accept_size = try_accept_one(start, len, PG_LEVEL_2M); > + if (!accept_size) > + accept_size = try_accept_one(start, len, PG_LEVEL_4K); > + if (!accept_size) > + return false; > + start += accept_size; > + } > + > + return true; > +} > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c > index 0d5fe6e24e45..a9c4ba6c5c5d 100644 > --- a/arch/x86/coco/tdx/tdx.c > +++ b/arch/x86/coco/tdx/tdx.c > @@ -713,46 +713,6 @@ static bool tdx_cache_flush_required(void) > return true; > } > > -static unsigned long try_accept_one(phys_addr_t start, unsigned long len, > - enum pg_level pg_level) > -{ > - unsigned long accept_size = page_level_size(pg_level); > - u64 tdcall_rcx; > - u8 page_size; > - > - if (!IS_ALIGNED(start, accept_size)) > - return 0; > - > - if (len < accept_size) > - return 0; > - > - /* > - * Pass the page physical address to the TDX module to accept the > - * pending, private page. > - * > - * Bits 2:0 of RCX encode page size: 0 - 4K, 1 - 2M, 2 - 1G. > - */ > - switch (pg_level) { > - case PG_LEVEL_4K: > - page_size = 0; > - break; > - case PG_LEVEL_2M: > - page_size = 1; > - break; > - case PG_LEVEL_1G: > - page_size = 2; > - break; > - default: > - return 0; > - } > - > - tdcall_rcx = start | page_size; > - if (__tdx_module_call(TDX_ACCEPT_PAGE, tdcall_rcx, 0, 0, 0, NULL)) > - return 0; > - > - return accept_size; > -} > - > /* > * Inform the VMM of the guest's intent for this physical page: shared with > * the VMM or private to the guest. The VMM is expected to change its mapping > @@ -777,33 +737,9 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc) > if (_tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0)) > return false; > > - /* private->shared conversion requires only MapGPA call */ > - if (!enc) > - return true; > - > - /* > - * For shared->private conversion, accept the page using > - * TDX_ACCEPT_PAGE TDX module call. > - */ > - while (start < end) { > - unsigned long len = end - start; > - unsigned long accept_size; > - > - /* > - * Try larger accepts first. It gives chance to VMM to keep > - * 1G/2M Secure EPT entries where possible and speeds up > - * process by cutting number of hypercalls (if successful). > - */ > - > - accept_size = try_accept_one(start, len, PG_LEVEL_1G); > - if (!accept_size) > - accept_size = try_accept_one(start, len, PG_LEVEL_2M); > - if (!accept_size) > - accept_size = try_accept_one(start, len, PG_LEVEL_4K); > - if (!accept_size) > - return false; > - start += accept_size; > - } > + /* shared->private conversion requires memory to be accepted before use */ > + if (enc) > + return tdx_accept_memory(start, end); > > return true; > } > diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h > index 1ff0ee822961..19228beb4894 100644 > --- a/arch/x86/include/asm/shared/tdx.h > +++ b/arch/x86/include/asm/shared/tdx.h > @@ -91,5 +91,7 @@ struct tdx_module_output { > u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9, > struct tdx_module_output *out); > > +bool tdx_accept_memory(phys_addr_t start, phys_addr_t end); > + > #endif /* !__ASSEMBLY__ */ > #endif /* _ASM_X86_SHARED_TDX_H */ > diff --git a/arch/x86/include/asm/unaccepted_memory.h b/arch/x86/include/asm/unaccepted_memory.h > new file mode 100644 > index 000000000000..f0ab217b566f > --- /dev/null > +++ b/arch/x86/include/asm/unaccepted_memory.h > @@ -0,0 +1,24 @@ > +#ifndef _ASM_X86_UNACCEPTED_MEMORY_H > +#define _ASM_X86_UNACCEPTED_MEMORY_H > + > +#include <linux/efi.h> > +#include <asm/tdx.h> > + > +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; > + } > + > + panic("Cannot accept memory: unknown platform\n"); > +} > + > +static inline struct efi_unaccepted_memory *efi_get_unaccepted_table(void) > +{ > + if (efi.unaccepted == EFI_INVALID_TABLE_ADDR) > + return NULL; > + return __va(efi.unaccepted); > +} > +#endif
| |