Re: [PATCH 0/5] KVM: x86/xen: Selftest fixes and a cleanup

From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH 0/5] KVM: x86/xen: Selftest fixes and a cleanup
Date: Wed, 10 Feb 2021 10:49:13 -0800	[thread overview]
Message-ID: <YCQqqb15kvdB81OC@google.com> (raw)
In-Reply-To: <dc334fe4-0ddc-b991-ad76-1d70c065fc16@redhat.com>

On Wed, Feb 10, 2021, Paolo Bonzini wrote:
> On 10/02/21 19:26, Sean Christopherson wrote:
> > Fix a '40' vs '0x40' bug in the new Xen shinfo selftest, and clean up some
> > other oddities that made root causing the problem far more painful than it
> > needed to be.
> > 
> > Note, Paolo already queued a patch from Vitaly that adds the tests to
> > .gitignore[*], i.e. patch 01 can likely be dropped.  I included it here
> > for completeness.
> > 
> > [*] https://lkml.kernel.org/r/20210129161821.74635-1-vkuznets@redhat.com
> > 
> > Sean Christopherson (5):
> >    KVM: selftests: Ignore recently added Xen tests' build output
> >    KVM: selftests: Fix size of memslots created by Xen tests
> >    KVM: selftests: Fix hex vs. decimal snafu in Xen test
> >    KVM: sefltests: Don't bother mapping GVA for Xen shinfo test
> >    KVM: x86/xen: Explicitly pad struct compat_vcpu_info to 64 bytes
> > 
> >   arch/x86/kvm/xen.h                                   | 11 ++++++-----
> >   tools/testing/selftests/kvm/.gitignore               |  2 ++
> >   tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c | 12 +++++-------
> >   tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c |  3 +--
> >   4 files changed, 14 insertions(+), 14 deletions(-)
> > 
> 
> Stupid question: how did you notice that?  In other words what broke for you
> and not for me?

I assume sheer dumb luck?  The test checks a single bit, so there's a 50/50
chance it will pass if whatever it's reading is non-zero.

If my math is correct (big if), the net effect is that the check was against
pvclock_vcpu_time_info.tsc_to_system_mul, which on my VM where I'm running the
test is always 0xcd4681c9.

==== Test Assertion Failure ====
  x86_64/xen_shinfo_test.c:161: ti->version && !(ti->version & 1)
  pid=1236 tid=1236 - Interrupted system call
     1	0x0000000000401623: main at xen_shinfo_test.c:160
     2	0x00007f303e261bf6: ?? ??:0
     3	0x00000000004016f9: _start at ??:?
  Bad time_info version cd4681c9