[0/7] KVM: VMX: Handle NMI VM-Exits in noinstr section

  Move NMI VM-Exit handling into vmx_vcpu_enter_exit() to fix a (mostly
benign?) bug where NMIs can be unblocked prior to servicing the NMI that
triggered the VM-Exit, e.g. if instrumentation triggers a fault and thus
an IRET.  I deliberately didn't tag any of these for stable@ as the odds
of me screwing something up or of a backport going sideways seems higher
than out-of-order NMIs causing major problems.

The bulk of this series is just getting various helpers/paths ready for
noinstr usage.

I kept the use of a direct call to a dedicated entry point for NMIs
(doubled down really).  AFAICT, there are no issues with the direct call
in the current code, and I don't know enough about FRED to know if using
INT $2 would be better or worse, i.e. less churn seemed like the way to
go.  And if reverting to INT $2 in the future is desirable, splitting NMI
and IRQ handling makes it quite easy to do so as all the relevant code
that needs to be ripped out is isolated.

Sean Christopherson (7):
  KVM: x86: Make vmx_get_exit_qual() and vmx_get_intr_info()
    noinstr-friendly
  KVM: VMX: Allow VM-Fail path of VMREAD helper to be instrumented
  KVM: VMX: Always inline eVMCS read/write helpers
  KVM: VMX: Always inline to_vmx() and to_kvm_vmx()
  x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too
  KVM: VMX: Provide separate subroutines for invoking NMI vs. IRQ
    handlers
  KVM: VMX: Handle NMI VM-Exits in noinstr region

 arch/x86/include/asm/idtentry.h | 16 +++-----
 arch/x86/kernel/nmi.c           |  8 ++--
 arch/x86/kvm/kvm_cache_regs.h   | 12 ++++++
 arch/x86/kvm/vmx/hyperv.h       | 20 ++++-----
 arch/x86/kvm/vmx/vmcs.h         |  4 +-
 arch/x86/kvm/vmx/vmenter.S      | 72 ++++++++++++++++++---------------
 arch/x86/kvm/vmx/vmx.c          | 55 +++++++++++++------------
 arch/x86/kvm/vmx/vmx.h          | 18 ++++-----
 arch/x86/kvm/vmx/vmx_ops.h      |  2 +
 arch/x86/kvm/x86.h              |  6 +--
 10 files changed, 117 insertions(+), 96 deletions(-)


base-commit: 208f1c64e255fe3a29083880818e010ebdf585c6
  

> I kept the use of a direct call to a dedicated entry point for NMIs
> (doubled down really).  AFAICT, there are no issues with the direct call
> in the current code, and I don't know enough about FRED to know if using
> INT $2 would be better or worse, i.e. less churn seemed like the way to
> go.  And if reverting to INT $2 in the future is desirable, splitting NMI
> and IRQ handling makes it quite easy to do so as all the relevant code
> that needs to be ripped out is isolated.

Thanks for making this change.

There is no big difference between "int $2" and calling the NMI handler explicitly.

Xin
  

Sean,

Is this merged into x86 KVM tree?

Thanks!
    Xin

> -----Original Message-----
> From: Sean Christopherson <seanjc@google.com>
> Sent: Monday, December 12, 2022 10:09 PM
> To: Christopherson,, Sean <seanjc@google.com>; Paolo Bonzini
> <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Peter Zijlstra
> <peterz@infradead.org>; Lutomirski, Andy <luto@kernel.org>; Thomas Gleixner
> <tglx@linutronix.de>
> Subject: [PATCH 0/7] KVM: VMX: Handle NMI VM-Exits in noinstr section
> 
> Move NMI VM-Exit handling into vmx_vcpu_enter_exit() to fix a (mostly
> benign?) bug where NMIs can be unblocked prior to servicing the NMI that
> triggered the VM-Exit, e.g. if instrumentation triggers a fault and thus an IRET.  I
> deliberately didn't tag any of these for stable@ as the odds of me screwing
> something up or of a backport going sideways seems higher than out-of-order
> NMIs causing major problems.
> 
> The bulk of this series is just getting various helpers/paths ready for noinstr
> usage.
> 
> I kept the use of a direct call to a dedicated entry point for NMIs (doubled down
> really).  AFAICT, there are no issues with the direct call in the current code, and I
> don't know enough about FRED to know if using INT $2 would be better or worse,
> i.e. less churn seemed like the way to go.  And if reverting to INT $2 in the future
> is desirable, splitting NMI and IRQ handling makes it quite easy to do so as all the
> relevant code that needs to be ripped out is isolated.
> 
> Sean Christopherson (7):
>   KVM: x86: Make vmx_get_exit_qual() and vmx_get_intr_info()
>     noinstr-friendly
>   KVM: VMX: Allow VM-Fail path of VMREAD helper to be instrumented
>   KVM: VMX: Always inline eVMCS read/write helpers
>   KVM: VMX: Always inline to_vmx() and to_kvm_vmx()
>   x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too
>   KVM: VMX: Provide separate subroutines for invoking NMI vs. IRQ
>     handlers
>   KVM: VMX: Handle NMI VM-Exits in noinstr region
> 
>  arch/x86/include/asm/idtentry.h | 16 +++-----
>  arch/x86/kernel/nmi.c           |  8 ++--
>  arch/x86/kvm/kvm_cache_regs.h   | 12 ++++++
>  arch/x86/kvm/vmx/hyperv.h       | 20 ++++-----
>  arch/x86/kvm/vmx/vmcs.h         |  4 +-
>  arch/x86/kvm/vmx/vmenter.S      | 72 ++++++++++++++++++---------------
>  arch/x86/kvm/vmx/vmx.c          | 55 +++++++++++++------------
>  arch/x86/kvm/vmx/vmx.h          | 18 ++++-----
>  arch/x86/kvm/vmx/vmx_ops.h      |  2 +
>  arch/x86/kvm/x86.h              |  6 +--
>  10 files changed, 117 insertions(+), 96 deletions(-)
> 
> 
> base-commit: 208f1c64e255fe3a29083880818e010ebdf585c6
> --
> 2.39.0.rc1.256.g54fd8350bd-goog
  

On Wed, Jan 18, 2023, Li, Xin3 wrote:
> Sean,
> 
> Is this merged into x86 KVM tree?

No, I want reviews for the KVM patches before merging, and need acks for the
non-KVM changes.
  

> > Is this merged into x86 KVM tree?
> 
> No, I want reviews for the KVM patches before merging, and need acks for the
> non-KVM changes.

I guess you want Peter Zijlstra, or some other x86 maintainers, to ack it.
  

On Tue, Dec 13, 2022 at 06:09:05AM +0000, Sean Christopherson wrote:

> Sean Christopherson (7):
>   KVM: x86: Make vmx_get_exit_qual() and vmx_get_intr_info()
>     noinstr-friendly
>   KVM: VMX: Allow VM-Fail path of VMREAD helper to be instrumented
>   KVM: VMX: Always inline eVMCS read/write helpers
>   KVM: VMX: Always inline to_vmx() and to_kvm_vmx()
>   x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too
>   KVM: VMX: Provide separate subroutines for invoking NMI vs. IRQ
>     handlers
>   KVM: VMX: Handle NMI VM-Exits in noinstr region

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
  

On Tue, 13 Dec 2022 06:09:05 +0000, Sean Christopherson wrote:
> Move NMI VM-Exit handling into vmx_vcpu_enter_exit() to fix a (mostly
> benign?) bug where NMIs can be unblocked prior to servicing the NMI that
> triggered the VM-Exit, e.g. if instrumentation triggers a fault and thus
> an IRET.  I deliberately didn't tag any of these for stable@ as the odds
> of me screwing something up or of a backport going sideways seems higher
> than out-of-order NMIs causing major problems.
> 
> [...]

Applied to kvm-x86 vmx, thanks!

[1/7] KVM: x86: Make vmx_get_exit_qual() and vmx_get_intr_info() noinstr-friendly
      https://github.com/kvm-x86/linux/commit/fc9465be8aad
[2/7] KVM: VMX: Allow VM-Fail path of VMREAD helper to be instrumented
      https://github.com/kvm-x86/linux/commit/8578f59657c5
[3/7] KVM: VMX: Always inline eVMCS read/write helpers
      https://github.com/kvm-x86/linux/commit/11633f69506d
[4/7] KVM: VMX: Always inline to_vmx() and to_kvm_vmx()
      https://github.com/kvm-x86/linux/commit/432727f1cb6e
[5/7] x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too
      https://github.com/kvm-x86/linux/commit/54a3b70a75dc
[6/7] KVM: VMX: Provide separate subroutines for invoking NMI vs. IRQ handlers
      https://github.com/kvm-x86/linux/commit/4f76e86f7e0d
[7/7] KVM: VMX: Handle NMI VM-Exits in noinstr region
      https://github.com/kvm-x86/linux/commit/11df586d774f

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes