Re: [PATCH v2 3/8] KVM: selftests: Delete dead ucall code

[David Matlack <dmatlack@xxxxxxxxxx>]

On Thu, Oct 27, 2022 at 4:44 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Oct 18, 2022, David Matlack wrote:
> > Delete a bunch of code related to ucall handling from
> > smaller_maxphyaddr_emulation_test. The only thing
> > smaller_maxphyaddr_emulation_test needs to check is that the vCPU exits
> > with UCALL_DONE after the second vcpu_run().
> >
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > ---
> >  .../smaller_maxphyaddr_emulation_test.c       | 54 +------------------
> >  1 file changed, 2 insertions(+), 52 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
> > index c5353ad0e06d..d6e71549ca08 100644
> > --- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c
> > @@ -90,64 +90,15 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
> >       vcpu_regs_set(vcpu, &regs);
> >  }
> >
> > -static void do_guest_assert(struct ucall *uc)
> > -{
> > -     REPORT_GUEST_ASSERT(*uc);
> > -}
> > -
> > -static void check_for_guest_assert(struct kvm_vcpu *vcpu)
> > +static void assert_ucall_done(struct kvm_vcpu *vcpu)
>
> I vote to delete this helper too, it's used exactly once and doesn't exactly make
> the code more readable.
>
> >       TEST_ASSERT(get_ucall(vcpu, &uc) == UCALL_DONE,
> >                   "Unexpected ucall command: %lu, expected UCALL_DONE (%d)",
> >                   uc.cmd, UCALL_DONE);
>
> I believe the warning is due to gcc resolving the VA args inputs to test_assert()
> before the call to get_ucall().  One thought:
>
>   uint64_t cmd = get_ucall(vcpu, NULL);
>
>   TEST_ASSERT(cmd == UCALL_DONE, ...)
>

I think you're right. And only gcc complains, which is how I missed
it. We can kill 2 birds (gcc warning + delete helper) with:

  ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);