[PATCH RFC 0/1] Fix ICEBP handling after KVM debug exits

[PATCH RFC 0/1] Fix ICEBP handling after KVM debug exits

[Saul Freedman]

To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
    Marcelo Tosatti <mtosatti@redhat.com>,
    kvm@vger.kernel.org

KVM normally recognizes ICEBP, advances RIP, and injects #DB itself.
When userspace enables hardware breakpoints, however, KVM reports the
pre-skip #DB through KVM_EXIT_DEBUG.  QEMU currently reinjects it at the
old RIP, causing a debug-exit livelock.

The patch is marked RFC because KVM_EXIT_DEBUG does not preserve KVM's
exact ICEBP classification.  The proposed compatibility heuristic accepts
a causeless #DB whose instruction stream consists only of prefixes and
0xf1.  A durable long-term ABI might instead let KVM expose its exact
classification or advance the instruction before exiting; I would like
feedback from both QEMU and KVM reviewers on that direction.

Validation on an Intel VT-x host:

- Built x86_64-softmmu from current master (ae4f3443209).
- checkpatch reports zero errors and warnings.
- Ran a real Linux guest that executes kernel-mode ICEBP while an
  unrelated hardware breakpoint is installed through QEMU's GDB stub.
  Unpatched QEMU stalls after ICEBP_KERNEL_BEFORE; patched QEMU reaches
  ICEBP_KERNEL_AFTER.

I did not include the hardware reproduction in the patch because it needs
real VT-x, a matching guest kernel build, and a loadable module.  I can
turn the fixture into an optional avocado test if reviewers prefer.

Saul Freedman (1):
  target/i386: skip ICEBP before reinjecting #DB

 target/i386/kvm/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

-- 
2.55.0

[PATCH RFC 1/1] target/i386: skip ICEBP before reinjecting #DB

[Saul Freedman]

KVM handles an ICEBP trap itself when userspace debugging is disabled:
it advances RIP and queues #DB for delivery to the guest.  When hardware
breakpoints are enabled, KVM instead reports the #DB to userspace before
advancing RIP.

QEMU currently reinjects that event without advancing RIP, so the guest
returns to ICEBP and traps again indefinitely.  KVM_EXIT_DEBUG does not
preserve KVM's exact ICEBP classification, but ICEBP has no mutable DR6
cause bits and is a one-byte opcode that may be preceded by instruction
prefixes.  Use those properties to recognize the instruction, advance RIP
past its complete encoding, and reinject the trap-like #DB.

Also convert the DR6 value reported by KVM_EXIT_DEBUG to the exception
payload form expected by KVM_SET_VCPU_EVENTS.  Clearing the active-low
bits leaves breakpoint, single-step, task-switch and RTM payload bits
unchanged.

The fix was tested on Intel VT-x with a real Linux guest executing a
kernel-mode ICEBP instruction while an unrelated hardware breakpoint was
installed through QEMU's GDB stub.

Signed-off-by: Saul Freedman <fre3dm4n@gmail.com>
---
 target/i386/kvm/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 644c45f..c02a155 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -6240,6 +6240,48 @@ void kvm_arch_remove_all_gdbstub_hw_breakpoints(void)
 
 static CPUWatchpoint hw_watchpoint;
 
+static int kvm_debug_exit_icebp_len(CPUState *cs, CPUX86State *env,
+                                    const struct kvm_debug_exit_arch *arch_info)
+{
+    int offset;
+
+    if (arch_info->exception != EXCP01_DB ||
+        arch_info->dr6 & ~DR6_FIXED_1) {
+        return 0;
+    }
+
+    /* An x86 instruction is at most fifteen bytes, including prefixes. */
+    for (offset = 0; offset < 15; offset++) {
+        uint8_t opcode;
+
+        if (cpu_memory_rw_debug(cs, arch_info->pc + offset,
+                                &opcode, sizeof(opcode), false) != 0) {
+            break;
+        }
+
+        if (opcode == 0xf1) {
+            return offset + 1;
+        }
+
+        switch (opcode) {
+        case 0x26: case 0x2e: case 0x36: case 0x3e:
+        case 0x64: case 0x65: case 0x66: case 0x67:
+        case 0xf0: case 0xf2: case 0xf3:
+            continue;
+        default:
+            if ((env->hflags & HF_CS64_MASK) &&
+                opcode >= 0x40 && opcode <= 0x4f) {
+                continue;
+            }
+            break;
+        }
+
+        break;
+    }
+
+    return 0;
+}
+
 static int kvm_handle_debug(X86CPU *cpu,
                             struct kvm_debug_exit_arch *arch_info)
 {
@@ -6280,13 +6322,31 @@ static int kvm_handle_debug(X86CPU *cpu,
         ret = EXCP_DEBUG;
     }
     if (ret == 0) {
+        uint64_t exception_payload = arch_info->dr6;
+
         cpu_synchronize_state(cs);
+
+        if (arch_info->exception == EXCP01_DB) {
+            int icebp_len;
+
+            icebp_len = kvm_debug_exit_icebp_len(cs, env, arch_info);
+            if (icebp_len != 0) {
+                env->eip += icebp_len;
+            }
+
+            /*
+             * KVM_EXIT_DEBUG reports an architectural DR6 image, while
+             * exception payloads use the VMX pending-debug format.
+             */
+            exception_payload &= ~DR6_FIXED_1;
+        }
+
         assert(env->exception_nr == -1);
 
         /* pass to guest */
         kvm_queue_exception(env, arch_info->exception,
                             arch_info->exception == EXCP01_DB,
-                            arch_info->dr6);
+                            exception_payload);
         env->has_error_code = 0;
     }
 
-- 
2.55.0