| Message ID | 20220414105322.577439-4-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: selftests: Provide TAP output in tests | expand |
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 */ > }; > > 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? or you don't want to print a skip because then the numbering in the planning doesn't match anymore? in which case, is there an easy way to fix it? > 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(); > }
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 */ >> }; >> >> 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? Thomas >> 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(); >> } >
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. > > Thomas > >>> 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(); >>> } >> >
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(-)