[PATCH v2 00/11] KVM: SVM: Fix soft int/ex re-injection

Sean Christopherson posted 11 patches 4 years ago
There is a newer version of this series
arch/x86/include/asm/kvm_host.h               |   2 +-
arch/x86/kvm/svm/nested.c                     |  46 ++++-
arch/x86/kvm/svm/svm.c                        | 169 ++++++++++++------
arch/x86/kvm/svm/svm.h                        |   7 +-
arch/x86/kvm/trace.h                          |  31 ++--
arch/x86/kvm/vmx/vmx.c                        |   4 +-
arch/x86/kvm/x86.c                            |  20 ++-
tools/testing/selftests/kvm/.gitignore        |   3 +-
tools/testing/selftests/kvm/Makefile          |   1 +
.../selftests/kvm/include/x86_64/svm_util.h   |   2 +
.../kvm/x86_64/svm_nested_soft_inject_test.c  | 149 +++++++++++++++
11 files changed, 351 insertions(+), 83 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c
[PATCH v2 00/11] KVM: SVM: Fix soft int/ex re-injection
Posted by Sean Christopherson 4 years ago
Fix soft interrupt/exception reinjection on SVM.

The underlying issue is that SVM simply retries INT* instructions instead
of reinjecting the soft interupt/exception if an exception VM-Exit occurred
during vectoring.  Lack of reinjection breaks nested virtualization if
the injected event came from L1 and the VM-Exit is not forwarded to L1,
as there is no instruction to retry.  More fundamentally, retrying the
instruction is wrong as it can produce side effects that shouldn't occur,
e.g. code #DBs.

VMX has been fixed since commit 66fd3f7f901f ("KVM: Do not re-execute
INTn instruction."), but SVM was left behind.  Probably because fixing
SVM is a mess due to NRIPS not being supported on all architectures, and
due to it being poorly implemented (with respect to soft events) when it
is supported.

Opportunistically clean up related tracepoints to make debugging related
issues less painful in the future.

The last patch is not-signed-off-by as I think it needs broader review
and feedback before KVM drops support for CPUs that are old, but not
thaaaaat old.

The tracepoint output looks like:

    kvm_inj_exception: #GP (0x0) 
    kvm_inj_exception: #UD       
    kvm_inj_exception: #DE       
    kvm_inj_exception: #DE [reinjected]
    kvm_inj_exception: #BP [reinjected]
    kvm_inj_exception: #NP (0x18) [reinjected]

and for "irqs":

    kvm_inj_virq: Soft/INTn 0x20 [reinjected]
    kvm_inj_virq: Soft/INTn 0x19 [reinjected]
    kvm_inj_virq: IRQ 0x20
    kvm_inj_virq: IRQ 0xf1

v2:
  - Collect reviews. [Maxim]
  - Drop a stale comment midway through. [Paolo]
  - Correctly handle (at least as correctly as SVM allows) the scenario
    where an injected soft interrupt/exception has no backing insn. [Maxim]
  - Tag reinjected exceptions in the tracepoint. [Maxim]
  - Use the correct L2 RIP (hopefully) in svm_set_nested_state. [Maciej]
  - Fix a BUG that can be triggered by userspace.
  - Fix the error code FIXME in the exception tracepoint.
  - Differentiate soft vs. hard "IRQ" injection in tracepoint.
  - Assert that the first soft int is injected on the correct RIP in
    the selftest.

v1:
  https://lore.kernel.org/all/20220402010903.727604-1-seanjc@google.com

Maciej's original series:
  https://lore.kernel.org/all/cover.1646944472.git.maciej.szmigiero@oracle.com

Maciej S. Szmigiero (3):
  KVM: nSVM: Sync next_rip field from vmcb12 to vmcb02
  KVM: SVM: Don't BUG if userspace injects a soft interrupt with GIF=0
  KVM: selftests: nSVM: Add svm_nested_soft_inject_test

Sean Christopherson (8):
  KVM: SVM: Unwind "speculative" RIP advancement if INTn injection
    "fails"
  KVM: SVM: Stuff next_rip on emulated INT3 injection if NRIPS is
    supported
  KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction
  KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"
  KVM: x86: Trace re-injected exceptions
  KVM: x86: Print error code in exception injection tracepoint iff valid
  KVM: x86: Differentiate Soft vs. Hard IRQs vs. reinjected in
    tracepoint
  KVM: SVM: Drop support for CPUs without NRIPS (NextRIP Save) support

 arch/x86/include/asm/kvm_host.h               |   2 +-
 arch/x86/kvm/svm/nested.c                     |  46 ++++-
 arch/x86/kvm/svm/svm.c                        | 169 ++++++++++++------
 arch/x86/kvm/svm/svm.h                        |   7 +-
 arch/x86/kvm/trace.h                          |  31 ++--
 arch/x86/kvm/vmx/vmx.c                        |   4 +-
 arch/x86/kvm/x86.c                            |  20 ++-
 tools/testing/selftests/kvm/.gitignore        |   3 +-
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/x86_64/svm_util.h   |   2 +
 .../kvm/x86_64/svm_nested_soft_inject_test.c  | 149 +++++++++++++++
 11 files changed, 351 insertions(+), 83 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c


base-commit: 2a39d8b39bffdaf1a4223d0d22f07baee154c8f3
-- 
2.36.0.rc2.479.g8af0fa9b8e-goog
Re: [PATCH v2 00/11] KVM: SVM: Fix soft int/ex re-injection
Posted by Maciej S. Szmigiero 4 years ago
On 23.04.2022 04:14, Sean Christopherson wrote:
> Fix soft interrupt/exception reinjection on SVM.
> 

Thanks for the patch set Sean, I can't see anything being obviously wrong
during a static code review - just small nits.

Will test it practically tomorrow and report the results.

Thanks,
Maciej
Re: [PATCH v2 00/11] KVM: SVM: Fix soft int/ex re-injection
Posted by Maciej S. Szmigiero 4 years ago
On 26.04.2022 01:01, Maciej S. Szmigiero wrote:
> On 23.04.2022 04:14, Sean Christopherson wrote:
>> Fix soft interrupt/exception reinjection on SVM.
>>
> 
> Thanks for the patch set Sean, I can't see anything being obviously wrong
> during a static code review - just small nits.
> 
> Will test it practically tomorrow and report the results.

I've tested this patch set and it seems to work fine with respect
to soft {exception,interrupt} re-injection and next_rip field consistency.

I have prepared a draft of an updated version at [1] with the following
further changes:
* "Downgraded" the commit affecting !nrips CPUs to just drop nested SVM
support for such parts instead of SVM support in general,

* Added a fix for L1/L2 NMI state confusion during L1 -> L2 NMI re-injection,

* Updated the new KVM self-test to also check for the NMI injection
scenario being fixed (that was found causing issues with a real guest),

* Changed "kvm_inj_virq" trace event "reinjected" field type to bool.

Will post a v3 patch set (with proper SoBs, etc.) if there are no further
comments or objections.

Thanks,
Maciej

[1]: https://github.com/maciejsszmigiero/linux/commits/svm_next_rip-sc