On Mon, May 11, 2026, Piotr Zarycki wrote: > On Mon, May 11, 2026 at 07:56:50AM -0700, Sean Christopherson wrote: > > On Mon, May 11, 2026, Piotr Zarycki wrote: > > > Use KVM_SYNC_X86_VALID_FIELDS instead of open-coding the same three > > > flags, ensuring the test stays in sync with any future additions. > > > > > > Signed-off-by: Piotr Zarycki <[email protected]> > > > --- > > > tools/testing/selftests/kvm/x86/sync_regs_test.c | 3 +-- > > > 1 file changed, 1 insertion(+), 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 e0c52321f87c..8f9239f9d357 100644 > > > --- a/tools/testing/selftests/kvm/x86/sync_regs_test.c > > > +++ b/tools/testing/selftests/kvm/x86/sync_regs_test.c > > > @@ -79,7 +79,7 @@ static void compare_vcpu_events(struct kvm_vcpu_events > > > *left, > > > { > > > } > > > > > > -#define TEST_SYNC_FIELDS > > > (KVM_SYNC_X86_REGS|KVM_SYNC_X86_SREGS|KVM_SYNC_X86_EVENTS) > > > +#define TEST_SYNC_FIELDS KVM_SYNC_X86_VALID_FIELDS > > > > Explicitly defining the set of fields to test is very deliberate. (a) we > > want > > to detect ABI breakage, e.g. if KVM removes a valid field. (b) if a field > > is > > added in the future, the test likely needs to be updated, i.e. would break > > if > > KVM extends KVM_SYNC_X86_VALID_FIELDS. (c) KVM_SYNC_X86_VALID_FIELDS > > probably > > shouldn't be defined in a uapi header; for all intents and purposes it's a > > KVM- > > internal details. > > > > > #define INVALID_SYNC_FIELD 0x80000000 > > > > > > /* > > > @@ -255,7 +255,6 @@ KVM_ONE_VCPU_TEST(sync_regs_test, > > > req_and_verify_all_valid, guest_code) > > > struct kvm_regs regs; > > > > > > /* Request and verify all valid register sets. */ > > > - /* TODO: BUILD TIME CHECK: TEST_ASSERT(KVM_SYNC_X86_NUM_FIELDS != 3); */ > > > run->kvm_valid_regs = TEST_SYNC_FIELDS; > > > vcpu_run(vcpu); > > > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > > -- > > > 2.54.0 > > > > > Thanks for the explanation. What's the preferred way to implement the > build-time check mentioned in the TODO?
Honestly? Just delete it. It should be quite obvious if someone updates the test to add more fields, without actually adding coverage for the new field(s).
