Hi, At 11:25 +0000 on 09 Feb (1423477508), Andrew Cooper wrote: > In the case of a crash, nmi_shootdown_cpus() patches nmi_crash() into the IDT > of each processor, such that the next NMI it receives will force it into the > crash path. > > c/s 7dd3b06ff "vmx: fix handling of NMI VMEXIT" fixed one issue but > inadvertently introduced another. The original use of self_nmi() would follow > vector #2, but a direct call to do_nmi() does not. > > Introduce a function pointer which should be used in preference to direct > do_nmi() calls, which is updated on the crash path to point at do_nmi_crash() > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > CC: Jan Beulich <jbeul...@suse.com> > CC: Tim Deegan <t...@xen.org> > > --- > > This patch very certainly functions correctly (it is in active use now in a > customer escalation), but I was wondering how paranoid we should be about > interleaved reads/writes and whether an atomic write would be better? > Performance is not a issue at all but in a crash senario we don't want to be > taking any chances with correctness.
Yes, atomic updates sound like a good idea. Would it make sense to add a _get_gate() or similar so the vmx path can read the actual IDT rather than adding a _third_ place where we set what to do on NMI? If not... > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -522,9 +522,10 @@ DECLARE_TRAP_HANDLER(alignment_check); > #undef DECLARE_TRAP_HANDLER_CONST > #undef DECLARE_TRAP_HANDLER > > +extern void (*nmi_handler)(const struct cpu_user_regs *regs); ...please add a comment here describing what's going on - in particular that changing this variable doesn't change the handler for all NMI paths (and that for most purposes set_nmi_callback() is more suitable). Cheers, Tim. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
