| Message ID | 20221018214612.3445074-4-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Fix and clean up emulator_error_test | expand |
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@google.com> > --- > .../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, ®s); > } > > -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, ...) In file included from x86_64/smaller_maxphyaddr_emulation_test.c:11: include/test_util.h: In function ‘assert_ucall_done’: include/test_util.h:54:9: warning: ‘uc.cmd’ is used uninitialized [-Wuninitialized] 54 | test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__) | ^~~~~~~~~~~ x86_64/smaller_maxphyaddr_emulation_test.c:69:22: note: ‘uc’ declared here 69 | struct ucall uc; | ^~
On Thu, Oct 27, 2022 at 4:44 PM Sean Christopherson <seanjc@google.com> 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@google.com> > > --- > > .../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, ®s); > > } > > > > -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);
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, ®s); } -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) { struct ucall uc; - if (vcpu->run->exit_reason == KVM_EXIT_IO && - get_ucall(vcpu, &uc) == UCALL_ABORT) { - do_guest_assert(&uc); - } -} - -static void process_ucall_done(struct kvm_vcpu *vcpu) -{ - struct kvm_run *run = vcpu->run; - struct ucall uc; - - check_for_guest_assert(vcpu); - - TEST_ASSERT(run->exit_reason == KVM_EXIT_IO, - "Unexpected exit reason: %u (%s)", - run->exit_reason, - exit_reason_str(run->exit_reason)); - TEST_ASSERT(get_ucall(vcpu, &uc) == UCALL_DONE, "Unexpected ucall command: %lu, expected UCALL_DONE (%d)", uc.cmd, UCALL_DONE); } -static uint64_t process_ucall(struct kvm_vcpu *vcpu) -{ - struct kvm_run *run = vcpu->run; - struct ucall uc; - - TEST_ASSERT(run->exit_reason == KVM_EXIT_IO, - "Unexpected exit reason: %u (%s)", - run->exit_reason, - exit_reason_str(run->exit_reason)); - - switch (get_ucall(vcpu, &uc)) { - case UCALL_SYNC: - break; - case UCALL_ABORT: - do_guest_assert(&uc); - break; - case UCALL_DONE: - process_ucall_done(vcpu); - break; - default: - TEST_ASSERT(false, "Unexpected ucall"); - } - - return uc.cmd; -} - int main(int argc, char *argv[]) { struct kvm_vcpu *vcpu; @@ -184,8 +135,7 @@ int main(int argc, char *argv[]) vcpu_run(vcpu); process_exit_on_emulation_error(vcpu); vcpu_run(vcpu); - - TEST_ASSERT(process_ucall(vcpu) == UCALL_DONE, "Expected UCALL_DONE"); + assert_ucall_done(vcpu); kvm_vm_free(vm);
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@google.com> --- .../smaller_maxphyaddr_emulation_test.c | 54 +------------------ 1 file changed, 2 insertions(+), 52 deletions(-)