Re: [PATCH] KVM: x86: Preserve KVM_EXIT_DEBUG on emulated HLT with single-step

[Sean Christopherson <seanjc@google.com>]

On Thu, Jul 09, 2026, sashiko-bot@kernel.org wrote:
> KVM: x86: Preserve KVM_EXIT_DEBUG on emulated HLT with single-step
>     
> kvm_emulate_halt() calls kvm_skip_emulated_instruction() before
> kvm_emulate_halt_noskip(). When single-step debugging is active,
> the skip path can trigger a debug exception and set exit_reason to
> KVM_EXIT_DEBUG, but halt emulation then overwrites it with
> KVM_EXIT_HLT on the userspace LAPIC path. Return immediately when
> a debug exit is already pending so userspace debuggers receive the
> correct event.
> 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ff8812f3a1293..473a03f58e263 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -11701,10 +11701,16 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_emulate_halt_noskip);
> >  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
> >  {
> >  	int ret = kvm_skip_emulated_instruction(vcpu);
> > +
> >  	/*
> > -	 * TODO: we might be squashing a GUESTDBG_SINGLESTEP-triggered
> > -	 * KVM_EXIT_DEBUG here.
> > +	 * If kvm_skip_emulated_instruction() triggered a singlestep
> > +	 * debug exception, return to userspace immediately so the
> > +	 * debugger can process the event. Do not let halt emulation
> > +	 * overwrite the KVM_EXIT_DEBUG exit reason.
> >  	 */
> > +	if (vcpu->run->exit_reason == KVM_EXIT_DEBUG)
> > +		return ret;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does restricting this early
> return exclusively to KVM_EXIT_DEBUG overwrite other critical exit reasons?
> 
> If kvm_skip_emulated_instruction() fails (e.g., executing HLT inside an
> SGX enclave), it queues KVM_EXIT_INTERNAL_ERROR and returns 0.
> 
> Because this check only handles KVM_EXIT_DEBUG, emulation failures fall
> through to kvm_emulate_halt_noskip(), which then overwrites the exit reason
> with KVM_EXIT_HLT for the userspace LAPIC path.
> 
> > +
> >  	return kvm_emulate_halt_noskip(vcpu) && ret;
> >  }
> >  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_emulate_halt);
> 
> [Severity: High]
> This is a pre-existing issue, but it looks like the same issue is present in
> kvm_emulate_ap_reset_hold().
> 
> arch/x86/kvm/x86.c:kvm_emulate_ap_reset_hold() {
> 	...
> 	int ret = kvm_skip_emulated_instruction(vcpu);
> 
> 	return __kvm_emulate_halt(vcpu, KVM_MP_STATE_AP_RESET_HOLD,
> 				  KVM_EXIT_AP_RESET_HOLD) && ret;
> }
> 
> Because && evaluates left-to-right, __kvm_emulate_halt() executes
> unconditionally and overwrites the exit reason with KVM_EXIT_AP_RESET_HOLD,
> even if kvm_skip_emulated_instruction() already returned 0 to exit to
> userspace with a debug exception or emulation failure.
> 
> Does kvm_emulate_ap_reset_hold() need a similar fix?

I think we should simply delete the TODO.  Architecturally, the HLT side effects
effectively have priority over the single-step #DB because the such #DBs are in
the "Traps on the Previous Instruction" class of exceptions, i.e. by definition,
the previous instruction has to have retired to take the single-step #DB.

And in practice, running without an in-kernel local APIC is all but deprecated
these days, so I don't see any value in going out of our way to split hairs over
which userspace exit is the "best" in this case.