Re: [PATCH] KVM: VMX: Handle single-step #DB for EMULTYPE_SKIP on EPT misconfig

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Aug 23, 2019 at 02:31:15PM -0700, Sean Christopherson wrote:
> VMX's EPT misconfig flow to handle fast-MMIO path falls back to decoding
> the instruction to determine the instruction length when running as a
> guest (Hyper-V doesn't fill VMCS.VM_EXIT_INSTRUCTION_LEN because it's
> technically not defined for EPT misconfigs).  Rather than implement the
> slow skip in VMX's generic skip_emulated_instruction(),
> handle_ept_misconfig() directly calls kvm_emulate_instruction() with
> EMULTYPE_SKIP, which intentionally doesn't do single-step detection, and
> so handle_ept_misconfig() misses a single-step #DB.
> 
> Rework the EPT misconfig fallback case to route it through
> kvm_skip_emulated_instruction() so that single-step #DBs and interrupt
> shadow updates are handled automatically.  I.e. make VMX's slow skip
> logic match SVM's and have the SVM flow not intentionally avoid the
> shadow update.
> 
> Alternatively, the handle_ept_misconfig() could manually handle single-
> step detection, but that results in EMULTYPE_SKIP having split logic for
> the interrupt shadow vs. single-step #DBs, and split emulator logic is
> largely what led to this mess in the first place.
> 
> Modifying SVM to mirror VMX flow isn't really an option as SVM's case
> isn't limited to a specific exit reason, i.e. handling the slow skip in
> skip_emulated_instruction() is mandatory for all intents and purposes.
> 
> Drop VMX's skip_emulated_instruction() wrapper since it can now fail,
> and instead WARN if it fails unexpectedly, e.g. if exit_reason somehow
> becomes corrupted.
> 
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Fixes: d391f12070672 ("x86/kvm/vmx: do not use vm-exit instruction length for fast MMIO when running nested")
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> 
> *** LOOK HERE ***
> 
> This patch applies on top my recent emulation cleanup[1][2] as it has
> non-trivial conflicts, dealing with those seemed like a waste of time,
> and this doesn't seem like a candidate for stable.  Let me know if you'd
> prefer it to be respun without the dependency.
> 
> Sadly/ironically, this unwinds some of the logic that was recently
> added by Vitaly at my suggestion.  Hindsight is 20/20 and all that...
> 
> [1] https://lkml.kernel.org/r/20190823010709.24879-1-sean.j.christopherson@intel.com
> [2] https://patchwork.kernel.org/cover/11110331/

Paolo and/or Radim,

Please ignore this patch, I'll fold it into the aforementioned emulation
cleanup since I need to spin v2 of that series.