| Message ID | 20220414105322.577439-4-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: s390: selftests: Provide TAP output in tests | expand |
On 14/04/2022 14.33, Janis Schoetterl-Glausch wrote: > On 4/14/22 14:08, Thomas Huth wrote: >> On 14/04/2022 13.51, Claudio Imbrenda wrote: >>> On Thu, 14 Apr 2022 12:53:21 +0200 >>> Thomas Huth <thuth@redhat.com> wrote: >>> >>>> The tprot test currently does not have any output (unless one of >>>> the TEST_ASSERT statement fails), so it's hard to say for a user >>>> whether a certain new sub-test has been included in the binary or >>>> not. Let's make this a little bit more user-friendly and include >>>> some TAP output via the kselftests.h interface. >>>> >>>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>>> --- >>>> tools/testing/selftests/kvm/s390x/tprot.c | 12 +++++++++++- >>>> 1 file changed, 11 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/tools/testing/selftests/kvm/s390x/tprot.c b/tools/testing/selftests/kvm/s390x/tprot.c >>>> index c097b9db495e..a714b4206e95 100644 >>>> --- a/tools/testing/selftests/kvm/s390x/tprot.c >>>> +++ b/tools/testing/selftests/kvm/s390x/tprot.c >>>> @@ -8,6 +8,7 @@ >>>> #include <sys/mman.h> >>>> #include "test_util.h" >>>> #include "kvm_util.h" >>>> +#include "kselftest.h" >>>> #define PAGE_SHIFT 12 >>>> #define PAGE_SIZE (1 << PAGE_SHIFT) >>>> @@ -69,6 +70,7 @@ enum stage { >>>> STAGE_INIT_FETCH_PROT_OVERRIDE, >>>> TEST_FETCH_PROT_OVERRIDE, >>>> TEST_STORAGE_PROT_OVERRIDE, >>>> + NUM_STAGES /* this must be the last entry */ > > You could move STAGE_END down and use that instead. > >>>> }; >>>> struct test { >>>> @@ -196,6 +198,7 @@ static void guest_code(void) >>>> } \ >>>> ASSERT_EQ(uc.cmd, UCALL_SYNC); \ >>>> ASSERT_EQ(uc.args[1], __stage); \ >>>> + ksft_test_result_pass("" #stage "\n"); \ >>>> }) >>>> int main(int argc, char *argv[]) >>>> @@ -204,6 +207,9 @@ int main(int argc, char *argv[]) >>>> struct kvm_run *run; >>>> vm_vaddr_t guest_0_page; >>>> + ksft_print_header(); >>>> + ksft_set_plan(NUM_STAGES - 1); /* STAGE_END is not counted, thus - 1 */ >>>> + >>>> vm = vm_create_default(VCPU_ID, 0, guest_code); >>>> run = vcpu_state(vm, VCPU_ID); >>>> @@ -213,7 +219,7 @@ int main(int argc, char *argv[]) >>>> guest_0_page = vm_vaddr_alloc(vm, PAGE_SIZE, 0); >>>> if (guest_0_page != 0) >>>> - print_skip("Did not allocate page at 0 for fetch protection override tests"); >>>> + ksft_print_msg("Did not allocate page at 0 for fetch protection override tests\n"); >>> >>> will this print a skip, though? >> >> No, it's now only a message. >> >>> or you don't want to print a skip because then the numbering in the >>> planning doesn't match anymore? >> >> Right. >> >>> in which case, is there an easy way to fix it? >> >> Honestly, this part of the code is a little bit of a riddle to me - I wonder why this was using "print_skip()" at all, since the HOST_SYNC below is executed anyway... so this sounds rather like a warning message to me that says that the following test might not work as expected, instead of a real test-is-skipped message? >> >> Janis, could you please clarify the intention here? > > Both the host and the guest check the same condition independently, the host just to print the message, > then the guest is run and skips those stages. Ok. However, I'm not sure how to make this use ksft_test_result_skip() in a nice way now, though, without makeing the macro way uglier ... I'll have a try, but if that does not work out I'd suggest to simply keep the ksft_print_msg() here instead. Thomas
diff --git a/tools/testing/selftests/kvm/s390x/tprot.c b/tools/testing/selftests/kvm/s390x/tprot.c index c097b9db495e..a714b4206e95 100644 --- a/tools/testing/selftests/kvm/s390x/tprot.c +++ b/tools/testing/selftests/kvm/s390x/tprot.c @@ -8,6 +8,7 @@ #include <sys/mman.h> #include "test_util.h" #include "kvm_util.h" +#include "kselftest.h" #define PAGE_SHIFT 12 #define PAGE_SIZE (1 << PAGE_SHIFT) @@ -69,6 +70,7 @@ enum stage { STAGE_INIT_FETCH_PROT_OVERRIDE, TEST_FETCH_PROT_OVERRIDE, TEST_STORAGE_PROT_OVERRIDE, + NUM_STAGES /* this must be the last entry */ }; struct test { @@ -196,6 +198,7 @@ static void guest_code(void) } \ ASSERT_EQ(uc.cmd, UCALL_SYNC); \ ASSERT_EQ(uc.args[1], __stage); \ + ksft_test_result_pass("" #stage "\n"); \ }) int main(int argc, char *argv[]) @@ -204,6 +207,9 @@ int main(int argc, char *argv[]) struct kvm_run *run; vm_vaddr_t guest_0_page; + ksft_print_header(); + ksft_set_plan(NUM_STAGES - 1); /* STAGE_END is not counted, thus - 1 */ + vm = vm_create_default(VCPU_ID, 0, guest_code); run = vcpu_state(vm, VCPU_ID); @@ -213,7 +219,7 @@ int main(int argc, char *argv[]) guest_0_page = vm_vaddr_alloc(vm, PAGE_SIZE, 0); if (guest_0_page != 0) - print_skip("Did not allocate page at 0 for fetch protection override tests"); + ksft_print_msg("Did not allocate page at 0 for fetch protection override tests\n"); HOST_SYNC(vm, STAGE_INIT_FETCH_PROT_OVERRIDE); if (guest_0_page == 0) mprotect(addr_gva2hva(vm, (vm_vaddr_t)0), PAGE_SIZE, PROT_READ); @@ -224,4 +230,8 @@ int main(int argc, char *argv[]) run->s.regs.crs[0] |= CR0_STORAGE_PROTECTION_OVERRIDE; run->kvm_dirty_regs = KVM_SYNC_CRS; HOST_SYNC(vm, TEST_STORAGE_PROT_OVERRIDE); + + kvm_vm_free(vm); + + ksft_finished(); }
The tprot test currently does not have any output (unless one of the TEST_ASSERT statement fails), so it's hard to say for a user whether a certain new sub-test has been included in the binary or not. Let's make this a little bit more user-friendly and include some TAP output via the kselftests.h interface. Signed-off-by: Thomas Huth <thuth@redhat.com> --- tools/testing/selftests/kvm/s390x/tprot.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)