| Message ID | 20230601182543.19036-1-kirill.shutemov@linux.intel.com |
|---|---|
| Headers | show |
| Series | mm, x86/cc, efi: Implement support for unaccepted memory | expand |
On Fri, Jun 02, 2023 at 02:10:05PM +0200, Borislav Petkov wrote: > On Thu, Jun 01, 2023 at 09:25:37PM +0300, Kirill A. Shutemov wrote: > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > index 043ca31c114e..231f1c70d1db 100644 > > --- a/drivers/firmware/efi/Kconfig > > +++ b/drivers/firmware/efi/Kconfig > > @@ -269,6 +269,20 @@ config EFI_COCO_SECRET > > virt/coco/efi_secret module to access the secrets, which in turn > > allows userspace programs to access the injected secrets. > > > > +config UNACCEPTED_MEMORY > > Why is this Kconfig symbol in this file? > > I'm thinking this needs to be somewhere generic, like in mm/Kconfig or > so... Unaccepted memory is an EFI feature. We can move it somewhere else, if other firmware/platform would support anything like it.
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
On 6/2/23 08:22, 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@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/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? > Maybe you can keep it similar to the v12 version with just a new error message, something like: if (early_is_tdx_guest()) { if (!tdx_accept_memory(start, end)) error("TDX error accepting memory\n"); } else { error("Cannot accept memory: unknown platform\n"); } And similar in arch/x86/include/asm/unaccepted_memory.h. Thanks, Tom > Thanks, > Tom > >> } >> void init_unaccepted_memory(void)
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@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? 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)
UEFI Specification version 2.9 introduces the concept of memory acceptance: some Virtual Machine platforms, such as Intel TDX or AMD SEV-SNP, requiring memory to be accepted before it can be used by the guest. Accepting happens via a protocol specific for the Virtual Machine platform. Accepting memory is costly and it makes VMM allocate memory for the accepted guest physical address range. It's better to postpone memory acceptance until memory is needed. It lowers boot time and reduces memory overhead. The kernel needs to know what memory has been accepted. Firmware communicates this information via memory map: a new memory type -- EFI_UNACCEPTED_MEMORY -- indicates such memory. Range-based tracking works fine for firmware, but it gets bulky for the kernel: e820 has to be modified on every page acceptance. It leads to table fragmentation, but there's a limited number of entries in the e820 table Another option is to mark such memory as usable in e820 and track if the range has been accepted in a bitmap. One bit in the bitmap represents 2MiB in the address space: one 4k page is enough to track 64GiB or physical address space. In the worst-case scenario -- a huge hole in the middle of the address space -- It needs 256MiB to handle 4PiB of the address space. Any unaccepted memory that is not aligned to 2M gets accepted upfront. The approach lowers boot time substantially. Boot to shell is ~2.5x faster for 4G TDX VM and ~4x faster for 64G. TDX-specific code isolated from the core of unaccepted memory support. It supposed to help to plug-in different implementation of unaccepted memory such as SEV-SNP. -- Fragmentation study -- Vlastimil and Mel were concern about effect of unaccepted memory on fragmentation prevention measures in page allocator. I tried to evaluate it, but it is tricky. As suggested I tried to run multiple parallel kernel builds and follow how often kmem:mm_page_alloc_extfrag gets hit. See results in the v9 of the patchset[1][2] [1] https://lore.kernel.org/all/20230330114956.20342-1-kirill.shutemov@linux.intel.com [2] https://lore.kernel.org/all/20230416191940.ex7ao43pmrjhru2p@box.shutemov.name -- The tree can be found here: https://github.com/intel/tdx.git guest-unaccepted-memory v13: - Fix few boot issues discovered by 0day; - Simplify tdx_accept_memory(): no need in MAP_GPA hypercall; - Update commit message for the first patch; - Add Reviewed-bys from Tom and Ard; v12: - Re-initialize 'unaccepted_table' variable from decompressor to cover some boot scenarios; - Add missing memblock_reserve() for the unaccepted memory configuration table (Mika); - Add efi.unaccepted into efi_tables (Tom); - Do not build tdx-shared.o for !TDX (Tom); - Typo fix (Liam) - Whitespace fix; - Reviewed-bys from Liam, Tom and Ard; v11: - Restructure the code to make it less x86-specific (suggested by Ard): + use EFI configuration table instead of zero-page to pass down bitmap; + do not imply 1bit == 2M in bitmap; + move bulk of the code under driver/firmware/efi; - The bitmap only covers unaccpeted memory now. All memory that is not covered by the bitmap assumed accepted; - Reviewed-by from Ard; v10: - Restructure code around zones_with_unaccepted_pages static brach to avoid unnecessary function calls (Suggested by Vlastimil); - Drop mentions of PageUnaccepted(); - Drop patches that add fake unaccepted memory support and sysfs handle to accept memory manually; - Add Reviewed-by from Vlastimil; v9: - Accept memory up to high watermark when kernel runs out of free memory; - Treat unaccepted memory as unusable in __zone_watermark_unusable_free(); - Per-zone unaccepted memory accounting; - All pages on unaccepted list are MAX_ORDER now; - accept_memory=eager in cmdline to pre-accept memory during the boot; - Implement fake unaccepted memory; - Sysfs handle to accept memory manually; - Drop PageUnaccepted(); - Rename unaccepted_pages static key to zones_with_unaccepted_pages; v8: - Rewrite core-mm support for unaccepted memory (patch 02/14); - s/UnacceptedPages/Unaccepted/ in meminfo; - Drop arch/x86/boot/compressed/compiler.h; - Fix build errors; - Adjust commit messages and comments; - Reviewed-bys from Dave and Borislav; - Rebased to tip/master. v7: - Rework meminfo counter to use PageUnaccepted() and move to generic code; - Fix range_contains_unaccepted_memory() on machines without unaccepted memory; - Add Reviewed-by from David; v6: - Fix load_unaligned_zeropad() on machine with unaccepted memory; - Clear PageUnaccepted() on merged pages, leaving it only on head; - Clarify error handling in allocate_e820(); - Fix build with CONFIG_UNACCEPTED_MEMORY=y, but without TDX; - Disable kexec at boottime instead of build conflict; - Rebased to tip/master; - Spelling fixes; - Add Reviewed-by from Mike and David; v5: - Updates comments and commit messages; + Explain options for unaccepted memory handling; - Expose amount of unaccepted memory in /proc/meminfo - Adjust check in page_expected_state(); - Fix error code handling in allocate_e820(); - Centralize __pa()/__va() definitions in the boot stub; - Avoid includes from the main kernel in the boot stub; - Use an existing hole in boot_param for unaccepted_memory, instead of adding to the end of the structure; - Extract allocate_unaccepted_memory() form allocate_e820(); - Complain if there's unaccepted memory, but kernel does not support it; - Fix vmstat counter; - Split up few preparatory patches; - Random readability adjustments; v4: - PageBuddyUnaccepted() -> PageUnaccepted; - Use separate page_type, not shared with offline; - Rework interface between core-mm and arch code; - Adjust commit messages; - Ack from Mike; Kirill A. Shutemov (9): mm: Add support for unaccepted memory efi/x86: Get full memory map in allocate_e820() efi/libstub: Implement support for unaccepted memory x86/boot/compressed: Handle unaccepted memory efi: Add unaccepted memory support efi/unaccepted: Avoid load_unaligned_zeropad() stepping into unaccepted memory x86/tdx: Make _tdx_hypercall() and __tdx_module_call() available in boot stub x86/tdx: Refactor try_accept_one() x86/tdx: Add unaccepted memory support arch/x86/Kconfig | 2 + arch/x86/boot/compressed/Makefile | 3 +- arch/x86/boot/compressed/efi.h | 10 + arch/x86/boot/compressed/error.c | 19 ++ arch/x86/boot/compressed/error.h | 1 + arch/x86/boot/compressed/kaslr.c | 35 ++- arch/x86/boot/compressed/mem.c | 73 ++++++ arch/x86/boot/compressed/misc.c | 7 + arch/x86/boot/compressed/misc.h | 6 + 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 | 102 +------- arch/x86/include/asm/efi.h | 2 + arch/x86/include/asm/shared/tdx.h | 53 ++++ arch/x86/include/asm/tdx.h | 19 -- arch/x86/include/asm/unaccepted_memory.h | 24 ++ arch/x86/platform/efi/efi.c | 3 + drivers/base/node.c | 7 + drivers/firmware/efi/Kconfig | 14 ++ drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/efi.c | 26 ++ drivers/firmware/efi/libstub/Makefile | 2 + drivers/firmware/efi/libstub/bitmap.c | 41 +++ drivers/firmware/efi/libstub/efistub.h | 6 + drivers/firmware/efi/libstub/find.c | 43 ++++ .../firmware/efi/libstub/unaccepted_memory.c | 234 ++++++++++++++++++ drivers/firmware/efi/libstub/x86-stub.c | 39 +-- drivers/firmware/efi/unaccepted_memory.c | 138 +++++++++++ fs/proc/meminfo.c | 5 + include/linux/efi.h | 13 +- include/linux/mm.h | 19 ++ include/linux/mmzone.h | 8 + mm/memblock.c | 9 + mm/mm_init.c | 7 + mm/page_alloc.c | 173 +++++++++++++ mm/vmstat.c | 3 + 37 files changed, 1074 insertions(+), 148 deletions(-) create mode 100644 arch/x86/boot/compressed/mem.c 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 create mode 100644 drivers/firmware/efi/libstub/bitmap.c create mode 100644 drivers/firmware/efi/libstub/find.c create mode 100644 drivers/firmware/efi/libstub/unaccepted_memory.c create mode 100644 drivers/firmware/efi/unaccepted_memory.c