Re: [Xen-devel] [PATCH] x86/nmi: Fix shootdown of pcpus running in VMX non-root mode

[Tim Deegan <tim@xxxxxxx>]

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.cooper3@xxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> 
> ---
> 
> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel