Re: [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Feb 28, 2013 at 01:00:08PM +0000, Tim Deegan wrote:
> At 09:58 +0000 on 28 Feb (1362045494), Jan Beulich wrote:
> > >>> On 22.11.12 at 17:12, Jan Beulich wrote:
> > >>>> On 22.11.12 at 17:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > > > On 22/11/12 15:55, Jan Beulich wrote:
> > > >>>>> On 22.11.12 at 16:37, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > > >>> A quick solution would be to execute a noop function with
> > > >>> run_in_exception_handler().  Alternatively, I can code enable_nmi() or
> > > >>> so which does an inline iret to itself.  Which of these would you prefer?
> > > >> I would actually consider avoiding to run softirqs altogether in that
> > > >> case, just like native Linux doesn't do any event or scheduler
> > > >> processing in that case.
> > > > 
> > > > That would probably be the easiest solution.
> > > > 
> > > > I was already going to do the same for the rentrant NMI and MCE handling
> > > > case (and also the process pending upcalls checking), due to the
> > > > complexities of fixing the race condition at the end of the handler.
> > > > 
> > > > Unfortunately, I don't think I have time to look at this issue
> > > > immediately, but if it is ok to wait till the beginning of next week, I
> > > 
> > > That's fine of course.
> > 
> > So that was 3 months ago, and we're likely going to need to do
> > further unfixed releases if this won't move forward. Can you
> > estimate whether you'll be able to get back to this?
> 
> Let's make a step in the right direction before 4.3, at least:

FYI, this also fixes oprofile being able to collect HVM guest
data.

> 
> commit d278beed1df2d226911dce92295411018c9bba2f
> Author: Tim Deegan <tim@xen.org>
> Date:   Thu Feb 28 12:42:15 2013 +0000
> 
>     vmx: handle NMIs before re-enabling interrupts.
>     
>     Also, switch to calling do_nmi() and explicitly re-enabling NMIs
>     rather than raising a fake NMI and relying on the NMI processing to
>     IRET, since that handling code is likely to change a fair amount in
>     future.
>     
>     Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 5378928..462bb0f 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2313,6 +2313,13 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>          vector = intr_info & INTR_INFO_VECTOR_MASK;
>          if ( vector == TRAP_machine_check )
>              do_machine_check(regs);
> +        if ( vector == TRAP_machine_check
> +             && ((intr_info & INTR_INFO_INTR_TYPE_MASK) ==
> +                 (X86_EVENTTYPE_NMI << 8)) )
> +        {
> +            do_nmi(regs);
> +            enable_nmis();
> +        }
>          break;
>      case EXIT_REASON_MCE_DURING_VMENTRY:
>          do_machine_check(regs);
> @@ -2486,7 +2493,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>                   (X86_EVENTTYPE_NMI << 8) )
>                  goto exit_and_crash;
>              HVMTRACE_0D(NMI);
> -            self_nmi(); /* Real NMI, vector 2: normal processing. */
> +            /* Already handled above. */
>              break;
>          case TRAP_machine_check:
>              HVMTRACE_0D(MCE);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>