diff mbox series

[v5,20/34] x86/fred: add a machine check entry stub for FRED

Message ID 20230307023946.14516-21-xin3.li@intel.com
State New
Headers
Series x86: enable FRED for x86-64 |

Commit Message

Li, Xin3 March 7, 2023, 2:39 a.m. UTC 
  Add a machine check entry stub for FRED.

Unlike IDT, no need to save/restore dr7 in FRED machine check handler.

Tested-by: Shan Kang <shan.kang@intel.com>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
 arch/x86/include/asm/fred.h    |  1 +
 arch/x86/kernel/cpu/mce/core.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

Comments

Peter Zijlstra March 20, 2023, 4 p.m. UTC | #1 
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.
Li, Xin3 March 21, 2023, 12:04 a.m. UTC | #2 
> > 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
Peter Zijlstra March 21, 2023, 8:59 a.m. UTC | #3 
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.
Li, Xin3 March 21, 2023, 4:38 p.m. UTC | #4 
> 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 mbox series

Patch

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)