| Message ID | 20230307023946.14516-21-xin3.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: enable FRED for x86-64 | expand |
On Mon, Mar 06, 2023 at 06:39:32PM -0800, Xin Li wrote: > Add a machine check entry stub for FRED. > > Unlike IDT, no need to save/restore dr7 in FRED machine check handler. Given how fragile MCE is, the question should be, do we ever want hw breakpoints to happen while it is running? If the hw-breakpoint handler trips on the same memory fail that got us into the mce the first time, we're dead.
> > Unlike IDT, no need to save/restore dr7 in FRED machine check handler. > > Given how fragile MCE is, the question should be, do we ever want hw > breakpoints to happen while it is running? HW breakpoints still work if they are properly configured. > If the hw-breakpoint handler trips on the same memory fail that got us into the > mce the first time, we're dead. Right. Unless the MCIP bit is turned off any subsequent #MC goes to shutdown ("machine is screwed"). It's the kernel debugger's responsibility to decide how to proceed in such cases. But if the kernel debugger itself is in a screwed memory region, we are soooooo dead. Thanks! Xin
On Tue, Mar 21, 2023 at 12:04:47AM +0000, Li, Xin3 wrote: > > > Unlike IDT, no need to save/restore dr7 in FRED machine check handler. > > > > Given how fragile MCE is, the question should be, do we ever want hw > > breakpoints to happen while it is running? > > HW breakpoints still work if they are properly configured. > > > If the hw-breakpoint handler trips on the same memory fail that got us into the > > mce the first time, we're dead. > > Right. > > Unless the MCIP bit is turned off any subsequent #MC goes to shutdown > ("machine is screwed"). > > It's the kernel debugger's responsibility to decide how to proceed in such > cases. But if the kernel debugger itself is in a screwed memory region, we > are soooooo dead. Yeah, so I would much prefer, for robustness sake, to start out with not allowing #DB in MCE -- much like today.
> On Tue, Mar 21, 2023 at 12:04:47AM +0000, Li, Xin3 wrote: > > > > Unlike IDT, no need to save/restore dr7 in FRED machine check handler. > > > > > > Given how fragile MCE is, the question should be, do we ever want hw > > > breakpoints to happen while it is running? > > > > HW breakpoints still work if they are properly configured. > > > > > If the hw-breakpoint handler trips on the same memory fail that got > > > us into the mce the first time, we're dead. > > > > Right. > > > > Unless the MCIP bit is turned off any subsequent #MC goes to shutdown > > ("machine is screwed"). > > > > It's the kernel debugger's responsibility to decide how to proceed in > > such cases. But if the kernel debugger itself is in a screwed memory > > region, we are soooooo dead. > > Yeah, so I would much prefer, for robustness sake, to start out with not allowing > #DB in MCE -- much like today. Will disable #DB inside #MCE then.
diff --git a/arch/x86/include/asm/fred.h b/arch/x86/include/asm/fred.h index f928a03082af..54746e8c0a17 100644 --- a/arch/x86/include/asm/fred.h +++ b/arch/x86/include/asm/fred.h @@ -97,6 +97,7 @@ typedef DECLARE_FRED_HANDLER((*fred_handler)); DECLARE_FRED_HANDLER(fred_exc_nmi); DECLARE_FRED_HANDLER(fred_exc_debug); DECLARE_FRED_HANDLER(fred_exc_page_fault); +DECLARE_FRED_HANDLER(fred_exc_machine_check); #endif /* __ASSEMBLY__ */ diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 7832a69d170e..26fa7fa44f30 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -52,6 +52,7 @@ #include <asm/mce.h> #include <asm/msr.h> #include <asm/reboot.h> +#include <asm/fred.h> #include "internal.h" @@ -2111,6 +2112,16 @@ DEFINE_IDTENTRY_MCE_USER(exc_machine_check) exc_machine_check_user(regs); local_db_restore(dr7); } + +#ifdef CONFIG_X86_FRED +DEFINE_FRED_HANDLER(fred_exc_machine_check) +{ + if (user_mode(regs)) + exc_machine_check_user(regs); + else + exc_machine_check_kernel(regs); +} +#endif #else /* 32bit unified entry point */ DEFINE_IDTENTRY_RAW(exc_machine_check)