[lkml]   [2022]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH 2/4] x86/tdx: Use ReportFatalError to report missing SEPT_VE_DISABLE
On Tue, Dec 13, 2022 at 03:06:07PM -0800, Dave Hansen wrote:
> On 12/9/22 05:25, Kirill A. Shutemov wrote:
> > The check for SEPT_VE_DISABLE happens early in the kernel boot where
> > earlyprintk is not yet functional. Kernel successfully detect broken
> > TD configuration and stops the kernel with panic(), but it cannot
> > communicate the reason to the user.
> Linux TDX guests require that the SEPT_VE_DISABLE "attribute" be set.
> If it is not set, the kernel is theoretically required to handle
> exceptions anywhere that kernel memory is accessed, including places
> like NMI handlers and in the syscall entry gap.
> Rather than even try to handle these exceptions, the kernel refuses to
> run if SEPT_VE_DISABLE is unset.
> However, the SEPT_VE_DISABLE detection and refusal code happens very
> early in boot, even before earlyprintk runs. Calling panic() will
> effectively just hang the system.
> Instead, call a TDX-specific panic() function. This makes a very simple
> TDVMCALL which gets a short error string out to the hypervisor without
> any console infrastructure.
> --
> Is that better?

Yes, thank you.

> Also, are you sure we want to do this? Is there any way to do this
> inside of panic() itself to get panic() itself to call tdx_panic() and
> get a short error message out to the hypervisor?
> Getting *all* users of panic this magic ability would be a lot better
> than giving it to one call-site of panic().
> I'm all for making the panic() path as short and simple as possible, but
> it would be nice if this fancy hypercall would get used in more than one
> spot.

Well, I don't see an obvious way to integrate this into panic().

There is panic_notifier_list and it kinda/sorta works, see the patch

But it breaks panic_notifier_list contract: the callback will never return
and no other callback will be able to do their stuff. panic_timeout is
also broken.

So ReportFatalError() is no good for the task. And I don't have anything
else :/

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 83ca9a7f0b75..81f9a964dc1f 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -7,6 +7,7 @@
#include <linux/cpufeature.h>
#include <linux/export.h>
#include <linux/io.h>
+#include <linux/panic_notifier.h>
#include <asm/coco.h>
#include <asm/tdx.h>
#include <asm/vmx.h>
@@ -146,8 +147,10 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)

-static void __noreturn tdx_panic(const char *msg)
+static int tdx_panic(struct notifier_block *this,
+ unsigned long event, void *ptr)
+ const char *msg = ptr;
struct tdx_hypercall_args args = {
@@ -219,7 +222,7 @@ static void tdx_parse_tdinfo(u64 *cc_mask)
if (td_attr & ATTR_DEBUG)
pr_warn("%s\n", msg);
- tdx_panic(msg);
+ panic(msg);

@@ -851,6 +854,10 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
return true;

+static struct notifier_block panic_block = {
+ .notifier_call = tdx_panic,
void __init tdx_early_init(void)
u64 cc_mask;
@@ -863,6 +870,7 @@ void __init tdx_early_init(void)


+ atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
Kiryl Shutsemau / Kirill A. Shutemov
 \ /
  Last update: 2022-12-15 18:19    [W:0.156 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site