2022-06-01 19:49:59

by Thomas Huth

[permalink] [raw]
Subject: [PATCH v4 0/4] KVM: s390: selftests: Provide TAP output in tests

This patch series is motivated by Shuah's suggestion here:

https://lore.kernel.org/kvm/[email protected]/

Many s390x KVM selftests do not output any information about which
tests have been run, so it's hard to say whether a test binary
contains a certain sub-test or not. To improve this situation let's
add some TAP output via the kselftest.h interface to these tests,
so that it easier to understand what has been executed or not.

v4:
- Rebased to include test_termination() now in the memop test
- Reworked the extension capability check in the memop test

v3:
- Added comments / fixed cosmetics according to Janosch's and
Janis' reviews of the v2 series
- Added Reviewed-by tags from the v2 series

v2:
- Reworked the extension checking in the first patch
- Make sure to always print the TAP 13 header in the second patch
- Reworked the SKIP printing in the third patch

Thomas Huth (4):
KVM: s390: selftests: Use TAP interface in the memop test
KVM: s390: selftests: Use TAP interface in the sync_regs test
KVM: s390: selftests: Use TAP interface in the tprot test
KVM: s390: selftests: Use TAP interface in the reset test

tools/testing/selftests/kvm/s390x/memop.c | 95 +++++++++++++++----
tools/testing/selftests/kvm/s390x/resets.c | 38 ++++++--
.../selftests/kvm/s390x/sync_regs_test.c | 87 +++++++++++++----
tools/testing/selftests/kvm/s390x/tprot.c | 29 +++++-
4 files changed, 197 insertions(+), 52 deletions(-)

--
2.31.1



2022-06-01 20:33:57

by Thomas Huth

[permalink] [raw]
Subject: [PATCH v4 3/4] KVM: s390: selftests: Use TAP interface in the tprot test

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.

Reviewed-by: Janosch Frank <[email protected]>
Reviewed-by: Janis Schoetterl-Glausch <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
tools/testing/selftests/kvm/s390x/tprot.c | 29 +++++++++++++++++++----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/s390x/tprot.c b/tools/testing/selftests/kvm/s390x/tprot.c
index c097b9db495e..14d74a9e7b3d 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)
@@ -63,12 +64,12 @@ static enum permission test_protection(void *addr, uint8_t key)
}

enum stage {
- STAGE_END,
STAGE_INIT_SIMPLE,
TEST_SIMPLE,
STAGE_INIT_FETCH_PROT_OVERRIDE,
TEST_FETCH_PROT_OVERRIDE,
TEST_STORAGE_PROT_OVERRIDE,
+ STAGE_END /* must be the last entry (it's the amount of tests) */
};

struct test {
@@ -182,7 +183,7 @@ static void guest_code(void)
GUEST_SYNC(perform_next_stage(&i, mapped_0));
}

-#define HOST_SYNC(vmp, stage) \
+#define HOST_SYNC_NO_TAP(vmp, stage) \
({ \
struct kvm_vm *__vm = (vmp); \
struct ucall uc; \
@@ -198,12 +199,21 @@ static void guest_code(void)
ASSERT_EQ(uc.args[1], __stage); \
})

+#define HOST_SYNC(vmp, stage) \
+({ \
+ HOST_SYNC_NO_TAP(vmp, stage); \
+ ksft_test_result_pass("" #stage "\n"); \
+})
+
int main(int argc, char *argv[])
{
struct kvm_vm *vm;
struct kvm_run *run;
vm_vaddr_t guest_0_page;

+ ksft_print_header();
+ ksft_set_plan(STAGE_END);
+
vm = vm_create_default(VCPU_ID, 0, guest_code);
run = vcpu_state(vm, VCPU_ID);

@@ -212,9 +222,14 @@ int main(int argc, char *argv[])
HOST_SYNC(vm, TEST_SIMPLE);

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");
- HOST_SYNC(vm, STAGE_INIT_FETCH_PROT_OVERRIDE);
+ if (guest_0_page != 0) {
+ /* Use NO_TAP so we don't get a PASS print */
+ HOST_SYNC_NO_TAP(vm, STAGE_INIT_FETCH_PROT_OVERRIDE);
+ ksft_test_result_skip("STAGE_INIT_FETCH_PROT_OVERRIDE - "
+ "Did not allocate page at 0\n");
+ } else {
+ 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);
run->s.regs.crs[0] |= CR0_FETCH_PROTECTION_OVERRIDE;
@@ -224,4 +239,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(); /* Print results and exit() accordingly */
}
--
2.31.1


2022-06-01 20:51:40

by Thomas Huth

[permalink] [raw]
Subject: [PATCH v4 2/4] KVM: s390: selftests: Use TAP interface in the sync_regs test

The sync_regs 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.
To be able to distinguish the different sub-tests more easily, we
also break up the huge main() function here in more fine grained
parts.

Acked-by: Janosch Frank <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
.../selftests/kvm/s390x/sync_regs_test.c | 87 ++++++++++++++-----
1 file changed, 66 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index caf7b8859a94..9510739e226d 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -21,6 +21,7 @@
#include "test_util.h"
#include "kvm_util.h"
#include "diag318_test_handler.h"
+#include "kselftest.h"

#define VCPU_ID 5

@@ -74,27 +75,9 @@ static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
#define TEST_SYNC_FIELDS (KVM_SYNC_GPRS|KVM_SYNC_ACRS|KVM_SYNC_CRS|KVM_SYNC_DIAG318)
#define INVALID_SYNC_FIELD 0x80000000

-int main(int argc, char *argv[])
+void test_read_invalid(struct kvm_vm *vm, struct kvm_run *run)
{
- struct kvm_vm *vm;
- struct kvm_run *run;
- struct kvm_regs regs;
- struct kvm_sregs sregs;
- int rv, cap;
-
- /* Tell stdout not to buffer its content */
- setbuf(stdout, NULL);
-
- cap = kvm_check_cap(KVM_CAP_SYNC_REGS);
- if (!cap) {
- print_skip("CAP_SYNC_REGS not supported");
- exit(KSFT_SKIP);
- }
-
- /* Create VM */
- vm = vm_create_default(VCPU_ID, 0, guest_code);
-
- run = vcpu_state(vm, VCPU_ID);
+ int rv;

/* Request reading invalid register set from VCPU. */
run->kvm_valid_regs = INVALID_SYNC_FIELD;
@@ -110,6 +93,11 @@ int main(int argc, char *argv[])
"Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n",
rv);
vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
+}
+
+void test_set_invalid(struct kvm_vm *vm, struct kvm_run *run)
+{
+ int rv;

/* Request setting invalid register set into VCPU. */
run->kvm_dirty_regs = INVALID_SYNC_FIELD;
@@ -125,6 +113,13 @@ int main(int argc, char *argv[])
"Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n",
rv);
vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
+}
+
+void test_req_and_verify_all_valid_regs(struct kvm_vm *vm, struct kvm_run *run)
+{
+ struct kvm_sregs sregs;
+ struct kvm_regs regs;
+ int rv;

/* Request and verify all valid register sets. */
run->kvm_valid_regs = TEST_SYNC_FIELDS;
@@ -146,6 +141,13 @@ int main(int argc, char *argv[])

vcpu_sregs_get(vm, VCPU_ID, &sregs);
compare_sregs(&sregs, &run->s.regs);
+}
+
+void test_set_and_verify_various_reg_values(struct kvm_vm *vm, struct kvm_run *run)
+{
+ struct kvm_sregs sregs;
+ struct kvm_regs regs;
+ int rv;

/* Set and verify various register values */
run->s.regs.gprs[11] = 0xBAD1DEA;
@@ -180,6 +182,11 @@ int main(int argc, char *argv[])

vcpu_sregs_get(vm, VCPU_ID, &sregs);
compare_sregs(&sregs, &run->s.regs);
+}
+
+void test_clear_kvm_dirty_regs_bits(struct kvm_vm *vm, struct kvm_run *run)
+{
+ int rv;

/* Clear kvm_dirty_regs bits, verify new s.regs values are
* overwritten with existing guest values.
@@ -200,8 +207,46 @@ int main(int argc, char *argv[])
TEST_ASSERT(run->s.regs.diag318 != 0x4B1D,
"diag318 sync regs value incorrect 0x%llx.",
run->s.regs.diag318);
+}
+
+struct testdef {
+ const char *name;
+ void (*test)(struct kvm_vm *vm, struct kvm_run *run);
+} testlist[] = {
+ { "read invalid", test_read_invalid },
+ { "set invalid", test_set_invalid },
+ { "request+verify all valid regs", test_req_and_verify_all_valid_regs },
+ { "set+verify various regs", test_set_and_verify_various_reg_values },
+ { "clear kvm_dirty_regs bits", test_clear_kvm_dirty_regs_bits },
+};
+
+int main(int argc, char *argv[])
+{
+ static struct kvm_run *run;
+ static struct kvm_vm *vm;
+ int idx;
+
+ /* Tell stdout not to buffer its content */
+ setbuf(stdout, NULL);
+
+ ksft_print_header();
+
+ if (!kvm_check_cap(KVM_CAP_SYNC_REGS))
+ ksft_exit_skip("CAP_SYNC_REGS not supported");
+
+ ksft_set_plan(ARRAY_SIZE(testlist));
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+
+ run = vcpu_state(vm, VCPU_ID);
+
+ for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
+ testlist[idx].test(vm, run);
+ ksft_test_result_pass("%s\n", testlist[idx].name);
+ }

kvm_vm_free(vm);

- return 0;
+ ksft_finished(); /* Print results and exit() accordingly */
}
--
2.31.1


2022-06-01 21:17:02

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] KVM: s390: selftests: Provide TAP output in tests

Am 31.05.22 um 12:15 schrieb Thomas Huth:
> This patch series is motivated by Shuah's suggestion here:
>
> https://lore.kernel.org/kvm/[email protected]/
>
> Many s390x KVM selftests do not output any information about which
> tests have been run, so it's hard to say whether a test binary
> contains a certain sub-test or not. To improve this situation let's
> add some TAP output via the kselftest.h interface to these tests,
> so that it easier to understand what has been executed or not.
>
> v4:
> - Rebased to include test_termination() now in the memop test
> - Reworked the extension capability check in the memop test
>
> v3:
> - Added comments / fixed cosmetics according to Janosch's and
> Janis' reviews of the v2 series
> - Added Reviewed-by tags from the v2 series
>
> v2:
> - Reworked the extension checking in the first patch
> - Make sure to always print the TAP 13 header in the second patch
> - Reworked the SKIP printing in the third patch
>
> Thomas Huth (4):
> KVM: s390: selftests: Use TAP interface in the memop test
> KVM: s390: selftests: Use TAP interface in the sync_regs test
> KVM: s390: selftests: Use TAP interface in the tprot test
> KVM: s390: selftests: Use TAP interface in the reset test
>
> tools/testing/selftests/kvm/s390x/memop.c | 95 +++++++++++++++----
> tools/testing/selftests/kvm/s390x/resets.c | 38 ++++++--
> .../selftests/kvm/s390x/sync_regs_test.c | 87 +++++++++++++----
> tools/testing/selftests/kvm/s390x/tprot.c | 29 +++++-
> 4 files changed, 197 insertions(+), 52 deletions(-)
>

Thanks applied and queued.