Re: [Patch v3 12/13] KVM: selftests: Make vCPU exit reason test assertion common.

[Sean Christopherson]

On Fri, Dec 09, 2022, David Matlack wrote:
> On Mon, Dec 05, 2022 at 11:14:29AM -0800, Vipin Sharma wrote:
> > Make ASSERT_EXIT_REASON() macro and replace all exit reason test assert
> > statements with it.
> > 
> > No functional changes intended.
> > 
> > Suggested-by: David Matlack <dmatlack@xxxxxxxxxx>
> > Signed-off-by: Vipin Sharma <vipinsh@xxxxxxxxxx>
> 
> Reviewed-by: David Matlack <dmatlack@xxxxxxxxxx>
> 
> > ---
> >  .../testing/selftests/kvm/aarch64/psci_test.c |  4 +--
> >  .../testing/selftests/kvm/include/test_util.h | 10 ++++++++
> [...]
> >  .../selftests/kvm/x86_64/xapic_ipi_test.c     |  6 +----
> >  .../selftests/kvm/x86_64/xen_shinfo_test.c    |  7 +-----
> >  .../selftests/kvm/x86_64/xen_vmcall_test.c    |  5 +---
> >  44 files changed, 71 insertions(+), 293 deletions(-)
> 
> Nice diff stat :)

I like the diffstat too, but I think we need a slightly different name for the
macro.  "EXIT_REASON" can be interpreted as "hardware exit reason" or "KVM exit
(to userspace) reason".  Most usage will be fairly obvious, but I'd like to avoid
confusion when swapping between selftests and KVM-unit-tests (which often asserts
on the hardware exit reason).   The name will be a bit longer, but I don't think
line length with ever be a problem.  And most people will just copy+paste so finger
fatigue likeliy won't be an issue either.

I also vote to prefix it with TEST, i.e. TEST_ASSERT_KVM_EXIT_REASON(), for
consistency and to make it very clear it's a TEST_ASSERT() wrapper.  ASSERT_EQ()
makes me twitch every time I see it.  This is definitely a personal preference 
(or problem) I can survive with though :-)