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

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

[suryasaimadhu]

From: sai madhu <suryasaimadhu369@gmail.com>

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.

Signed-off-by: sai madhu <suryasaimadhu369@gmail.com>
---
 arch/x86/kvm/x86.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0626e835e9eb..2fd6ad49050c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8658,10 +8658,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;
+
 	return kvm_emulate_halt_noskip(vcpu) && ret;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_emulate_halt);
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The fix to preserve KVM_EXIT_DEBUG during instruction skipping was not applied to kvm_emulate_ap_reset_hold().
- [High] kvm_emulate_halt() restricts early return exclusively to KVM_EXIT_DEBUG, overwriting other critical exit reasons like KVM_EXIT_INTERNAL_ERROR.
--

--- Patch [1]: [PATCH] KVM: x86: Preserve KVM_EXIT_DEBUG on emulated HLT with single-step ---
commit a928dfda27cebab2c60b683589e4cf4ae12635fb
Author: sai madhu <suryasaimadhu369@gmail.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709053949.211165-1-SaiMadhu.KoyyalaHariVenkata@amd.com?part=1

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

[Sean Christopherson]

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.