[PATCH 0/2] KVM: selftests: Verify sregs and events in sync_regs_test

[PATCH 0/2] KVM: selftests: Verify sregs and events in sync_regs_test

[Hemanth Selam]

sync_regs_test has never actually checked the sregs and events halves of
the kvm_run sync region.  compare_sregs() and compare_vcpu_events() have
had empty bodies since the test was written, so the checks run after
every KVM_RUN only ever compared the general purpose registers, and the
events half was never written to at all, hence the standing TODO in
set_and_verify_various().

Patch 1 fills in both comparison helpers.  Patch 2 exercises the events
half by masking NMIs through the sync region and verifying KVM applies
it.

To confirm the new checks catch something rather than merely passing,
the same faults were injected before and after the series on an AMD
host:

  fault injected                      before    after
  sregs.cr4 mismatch                  ignored   caught
  sregs.cs.dpl mismatch               ignored   caught
  events.interrupt.shadow mismatch    ignored   caught
  KVM_SYNC_X86_EVENTS left clean      n/a       caught

Every corruption goes unnoticed before the series and is reported after
it, e.g.

  x86/sync_regs_test.c:128: left->cr4 == right->cr4
  Register cr4 values did not match: 0x41221, 0x41220

Unmodified, the test passes 20/20 consecutive runs with the series
applied, each patch builds and passes on its own, and the remaining x86
selftests are unchanged either way (61 passed, 24 skipped, with
set_sregs_test failing identically before and after for unrelated
reasons).

Hemanth Selam (2):
  KVM: selftests: Actually compare sregs and events in sync_regs_test
  KVM: selftests: Verify the events half of the sync regs region

 .../selftests/kvm/x86/sync_regs_test.c        | 100 +++++++++++++++++-
 1 file changed, 98 insertions(+), 2 deletions(-)

-- 
2.43.7

[PATCH 1/2] KVM: selftests: Actually compare sregs and events in sync_regs_test

[Hemanth Selam]

compare_sregs() and compare_vcpu_events() have been empty since the test
was written, so the checks that req_and_verify_all_valid() and
set_and_verify_various() run after every KVM_RUN only ever compared the
general purpose registers.  KVM could return anything at all in the sregs
and events halves of the kvm_run sync region and the test would still
pass.

Fill both in, in the style of compare_regs(): every field of struct
kvm_sregs, including the segment and descriptor table members and the
interrupt bitmap, and every field of struct kvm_vcpu_events.  Both sides
of each comparison come from the same vCPU without it having run in
between, so they are expected to match exactly.

Assisted-by: Cursor:claude-opus-5
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
 .../selftests/kvm/x86/sync_regs_test.c        | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86/sync_regs_test.c b/tools/testing/selftests/kvm/x86/sync_regs_test.c
index ed0c21b427c9..c7d1f3cca16e 100644
--- a/tools/testing/selftests/kvm/x86/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86/sync_regs_test.c
@@ -70,13 +70,102 @@ static void compare_regs(struct kvm_regs *left, struct kvm_regs *right)
 #undef REG_COMPARE
 }
 
+static void compare_segment(struct kvm_segment *left, struct kvm_segment *right,
+			    const char *name)
+{
+#define SEG_COMPARE(field) \
+	TEST_ASSERT(left->field == right->field, \
+		    "Segment %s." #field \
+		    " values did not match: 0x%llx, 0x%llx", \
+		    name, (unsigned long long)left->field, \
+		    (unsigned long long)right->field)
+	SEG_COMPARE(base);
+	SEG_COMPARE(limit);
+	SEG_COMPARE(selector);
+	SEG_COMPARE(type);
+	SEG_COMPARE(present);
+	SEG_COMPARE(dpl);
+	SEG_COMPARE(db);
+	SEG_COMPARE(s);
+	SEG_COMPARE(l);
+	SEG_COMPARE(g);
+	SEG_COMPARE(avl);
+	SEG_COMPARE(unusable);
+#undef SEG_COMPARE
+}
+
+static void compare_dtable(struct kvm_dtable *left, struct kvm_dtable *right,
+			   const char *name)
+{
+	TEST_ASSERT(left->base == right->base,
+		    "Descriptor table %s.base values did not match: 0x%llx, 0x%llx",
+		    name, left->base, right->base);
+	TEST_ASSERT(left->limit == right->limit,
+		    "Descriptor table %s.limit values did not match: 0x%x, 0x%x",
+		    name, left->limit, right->limit);
+}
+
 static void compare_sregs(struct kvm_sregs *left, struct kvm_sregs *right)
 {
+#define SREG_COMPARE(reg) \
+	TEST_ASSERT(left->reg == right->reg, \
+		    "Register " #reg \
+		    " values did not match: 0x%llx, 0x%llx", \
+		    left->reg, right->reg)
+	compare_segment(&left->cs, &right->cs, "cs");
+	compare_segment(&left->ds, &right->ds, "ds");
+	compare_segment(&left->es, &right->es, "es");
+	compare_segment(&left->fs, &right->fs, "fs");
+	compare_segment(&left->gs, &right->gs, "gs");
+	compare_segment(&left->ss, &right->ss, "ss");
+	compare_segment(&left->tr, &right->tr, "tr");
+	compare_segment(&left->ldt, &right->ldt, "ldt");
+	compare_dtable(&left->gdt, &right->gdt, "gdt");
+	compare_dtable(&left->idt, &right->idt, "idt");
+	SREG_COMPARE(cr0);
+	SREG_COMPARE(cr2);
+	SREG_COMPARE(cr3);
+	SREG_COMPARE(cr4);
+	SREG_COMPARE(cr8);
+	SREG_COMPARE(efer);
+	SREG_COMPARE(apic_base);
+#undef SREG_COMPARE
+	TEST_ASSERT(!memcmp(left->interrupt_bitmap, right->interrupt_bitmap,
+			    sizeof(left->interrupt_bitmap)),
+		    "interrupt_bitmap values did not match");
 }
 
 static void compare_vcpu_events(struct kvm_vcpu_events *left,
 				struct kvm_vcpu_events *right)
 {
+#define EVENT_COMPARE(field) \
+	TEST_ASSERT(left->field == right->field, \
+		    "Event " #field \
+		    " values did not match: 0x%llx, 0x%llx", \
+		    (unsigned long long)left->field, \
+		    (unsigned long long)right->field)
+	EVENT_COMPARE(exception.injected);
+	EVENT_COMPARE(exception.nr);
+	EVENT_COMPARE(exception.has_error_code);
+	EVENT_COMPARE(exception.pending);
+	EVENT_COMPARE(exception.error_code);
+	EVENT_COMPARE(interrupt.injected);
+	EVENT_COMPARE(interrupt.nr);
+	EVENT_COMPARE(interrupt.soft);
+	EVENT_COMPARE(interrupt.shadow);
+	EVENT_COMPARE(nmi.injected);
+	EVENT_COMPARE(nmi.pending);
+	EVENT_COMPARE(nmi.masked);
+	EVENT_COMPARE(sipi_vector);
+	EVENT_COMPARE(flags);
+	EVENT_COMPARE(smi.smm);
+	EVENT_COMPARE(smi.pending);
+	EVENT_COMPARE(smi.smm_inside_nmi);
+	EVENT_COMPARE(smi.latched_init);
+	EVENT_COMPARE(triple_fault.pending);
+	EVENT_COMPARE(exception_has_payload);
+	EVENT_COMPARE(exception_payload);
+#undef EVENT_COMPARE
 }
 
 #define TEST_SYNC_FIELDS   (KVM_SYNC_X86_REGS|KVM_SYNC_X86_SREGS|KVM_SYNC_X86_EVENTS)
-- 
2.43.7

Re: [PATCH 1/2] KVM: selftests: Actually compare sregs and events in sync_regs_test

[Hemanth Selam]

On Wed, Sep 02, 2026 at 12:22:09PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This is a pre-existing issue, but does this new assertion trigger test
> failures due to how __get_sregs() handles the interrupt_bitmap?

No.  Neither of the two tests that reach compare_sregs(),
req_and_verify_all_valid and set_and_verify_various, ever injects an
interrupt: the guest just loops on an IN from a port, and nothing in the
test writes interrupt_bitmap.  KVM allocates the kvm_run page with
__GFP_ZERO, so the sync region's copy starts out zero and stays zero,
and the KVM_GET_SREGS side is filled from a kzalloc()'d buffer.  Both
sides are all zeros at every point where the memcmp() runs, which I
confirmed by printing them, and the series passes 10/10 on an AMD host.

> This function sets the bit corresponding to the currently injected interrupt
> but never clears the prior contents of the array.

That part is right, and it is observable from userspace.  Planting a bit
in the sync region and running the vCPU with KVM_SYNC_X86_SREGS in
kvm_valid_regs but nothing in kvm_dirty_regs:

  run->s.regs.sregs.interrupt_bitmap[0] = 1ULL << 0x30;

leaves KVM_GET_SREGS reporting 0x0 while the sync region still reads
0x1000000000000 after the exit.  KVM refreshed sregs there and left the
caller's bit in place.

> Could this lead to spurious interrupt injections or corrupted state during
> live migration for any VMM using the KVM_SYNC_X86_SREGS API?

Handing that same stale bit back with KVM_SYNC_X86_SREGS set in
kvm_dirty_regs does inject it: __set_sregs()'s find_first_bit() picks up
vector 0x30, kvm_queue_interrupt() queues it, and the guest dies on an
unhandled 0x30.  So a VMM that read-modify-writes the sync region can
resubmit a vector KVM itself put there on an earlier exit, since nothing
clears the field once a bit is set.

I could not demonstrate KVM planting the bit itself here, as selftest VMs
have an in-kernel irqchip and KVM_INTERRUPT returns -ENXIO; that half
rests on __get_sregs() only ever doing set_bit().

Whether KVM should zero interrupt_bitmap before it ORs in the injected
vector, or clear it in store_regs(), is an ABI question rather than
something this test change should decide, and I am happy to send a patch
if that is the direction.  I would keep the memcmp() either way: it is
precisely the check that would catch a stale bit turning up in the sync
region.

Thanks,
Hemanth

[PATCH 2/2] KVM: selftests: Verify the events half of the sync regs region

[Hemanth Selam]

set_and_verify_various() writes to the regs and sregs halves of the
kvm_run sync region and checks that KVM picks them up, but it has never
done the same for events, hence the TODO.

Mask NMIs through the sync region and verify KVM applies it.  Masking is
a state change rather than an injected event, and the guest is a tight
loop that never executes IRET, so nothing can unmask NMIs behind the
test's back and the check does not depend on timing.

Assisted-by: Cursor:claude-opus-5
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
 tools/testing/selftests/kvm/x86/sync_regs_test.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/sync_regs_test.c b/tools/testing/selftests/kvm/x86/sync_regs_test.c
index c7d1f3cca16e..b417eb0ab0f0 100644
--- a/tools/testing/selftests/kvm/x86/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86/sync_regs_test.c
@@ -372,10 +372,15 @@ KVM_ONE_VCPU_TEST(sync_regs_test, set_and_verify_various, guest_code)
 	/* Set and verify various register values. */
 	run->s.regs.regs.rbx = 0xBAD1DEA;
 	run->s.regs.sregs.apic_base = 1 << 11;
-	/* TODO run->s.regs.events.XYZ = ABC; */
+	/*
+	 * The guest never executes IRET, so masking NMIs is a state change
+	 * KVM has to hand back unchanged, without needing an event injected.
+	 */
+	run->s.regs.events.nmi.masked = 1;
+	run->s.regs.events.flags = 0;
 
 	run->kvm_valid_regs = TEST_SYNC_FIELDS;
-	run->kvm_dirty_regs = KVM_SYNC_X86_REGS | KVM_SYNC_X86_SREGS;
+	run->kvm_dirty_regs = TEST_SYNC_FIELDS;
 	vcpu_run(vcpu);
 	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
 	TEST_ASSERT(run->s.regs.regs.rbx == 0xBAD1DEA + 1,
@@ -384,6 +389,8 @@ KVM_ONE_VCPU_TEST(sync_regs_test, set_and_verify_various, guest_code)
 	TEST_ASSERT(run->s.regs.sregs.apic_base == 1 << 11,
 		    "apic_base sync regs value incorrect 0x%llx.",
 		    run->s.regs.sregs.apic_base);
+	TEST_ASSERT(run->s.regs.events.nmi.masked,
+		    "events sync regs value incorrect, NMIs not masked");
 
 	vcpu_regs_get(vcpu, &regs);
 	compare_regs(&regs, &run->s.regs.regs);
-- 
2.43.7