The memop 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]>
Signed-off-by: Thomas Huth <[email protected]>
---
tools/testing/selftests/kvm/s390x/memop.c | 95 ++++++++++++++++++-----
1 file changed, 77 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
index 49f26f544127..e704c6fa5758 100644
--- a/tools/testing/selftests/kvm/s390x/memop.c
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -14,6 +14,7 @@
#include "test_util.h"
#include "kvm_util.h"
+#include "kselftest.h"
enum mop_target {
LOGICAL,
@@ -691,34 +692,92 @@ static void test_errors(void)
kvm_vm_free(t.kvm_vm);
}
+struct testdef {
+ const char *name;
+ void (*test)(void);
+ int extension;
+} testlist[] = {
+ {
+ .name = "simple copy",
+ .test = test_copy,
+ },
+ {
+ .name = "generic error checks",
+ .test = test_errors,
+ },
+ {
+ .name = "copy with storage keys",
+ .test = test_copy_key,
+ .extension = 1,
+ },
+ {
+ .name = "copy with key storage protection override",
+ .test = test_copy_key_storage_prot_override,
+ .extension = 1,
+ },
+ {
+ .name = "copy with key fetch protection",
+ .test = test_copy_key_fetch_prot,
+ .extension = 1,
+ },
+ {
+ .name = "copy with key fetch protection override",
+ .test = test_copy_key_fetch_prot_override,
+ .extension = 1,
+ },
+ {
+ .name = "error checks with key",
+ .test = test_errors_key,
+ .extension = 1,
+ },
+ {
+ .name = "termination",
+ .test = test_termination,
+ .extension = 1,
+ },
+ {
+ .name = "error checks with key storage protection override",
+ .test = test_errors_key_storage_prot_override,
+ .extension = 1,
+ },
+ {
+ .name = "error checks without key fetch prot override",
+ .test = test_errors_key_fetch_prot_override_not_enabled,
+ .extension = 1,
+ },
+ {
+ .name = "error checks with key fetch prot override",
+ .test = test_errors_key_fetch_prot_override_enabled,
+ .extension = 1,
+ },
+};
+
int main(int argc, char *argv[])
{
- int memop_cap, extension_cap;
+ int memop_cap, extension_cap, idx;
setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
+ ksft_print_header();
+
memop_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP);
extension_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
if (!memop_cap) {
- print_skip("CAP_S390_MEM_OP not supported");
- exit(KSFT_SKIP);
+ ksft_exit_skip("CAP_S390_MEM_OP not supported.\n");
}
- test_copy();
- if (extension_cap > 0) {
- test_copy_key();
- test_copy_key_storage_prot_override();
- test_copy_key_fetch_prot();
- test_copy_key_fetch_prot_override();
- test_errors_key();
- test_termination();
- test_errors_key_storage_prot_override();
- test_errors_key_fetch_prot_override_not_enabled();
- test_errors_key_fetch_prot_override_enabled();
- } else {
- print_skip("storage key memop extension not supported");
+ ksft_set_plan(ARRAY_SIZE(testlist));
+
+ for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
+ if (testlist[idx].extension >= extension_cap) {
+ testlist[idx].test();
+ ksft_test_result_pass("%s\n", testlist[idx].name);
+ } else {
+ ksft_test_result_skip("%s - extension level %d not supported\n",
+ testlist[idx].name,
+ testlist[idx].extension);
+ }
}
- test_errors();
- return 0;
+ ksft_finished(); /* Print results and exit() accordingly */
}
--
2.31.1
On 5/31/22 12:15, Thomas Huth wrote:
> The memop 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]>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> tools/testing/selftests/kvm/s390x/memop.c | 95 ++++++++++++++++++-----
> 1 file changed, 77 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
> index 49f26f544127..e704c6fa5758 100644
> --- a/tools/testing/selftests/kvm/s390x/memop.c
> +++ b/tools/testing/selftests/kvm/s390x/memop.c
> @@ -14,6 +14,7 @@
>
[...]
> int main(int argc, char *argv[])
> {
> - int memop_cap, extension_cap;
> + int memop_cap, extension_cap, idx;
>
> setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
>
> + ksft_print_header();
> +
> memop_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP);
> extension_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
> if (!memop_cap) {
> - print_skip("CAP_S390_MEM_OP not supported");
> - exit(KSFT_SKIP);
> + ksft_exit_skip("CAP_S390_MEM_OP not supported.\n");
> }
>
> - test_copy();
> - if (extension_cap > 0) {
> - test_copy_key();
> - test_copy_key_storage_prot_override();
> - test_copy_key_fetch_prot();
> - test_copy_key_fetch_prot_override();
> - test_errors_key();
> - test_termination();
> - test_errors_key_storage_prot_override();
> - test_errors_key_fetch_prot_override_not_enabled();
> - test_errors_key_fetch_prot_override_enabled();
> - } else {
> - print_skip("storage key memop extension not supported");
> + ksft_set_plan(ARRAY_SIZE(testlist));
> +
> + for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
> + if (testlist[idx].extension >= extension_cap) {
This is reversed, should be
if (testlist[idx].extension <= extension_cap) {
or
if (extension_cap >= testlist[idx].extension) {
I'd prefer the latter.
> + testlist[idx].test();
> + ksft_test_result_pass("%s\n", testlist[idx].name);
> + } else {
> + ksft_test_result_skip("%s - extension level %d not supported\n",
> + testlist[idx].name,
> + testlist[idx].extension);
> + }
> }
> - test_errors();
>
> - return 0;
> + ksft_finished(); /* Print results and exit() accordingly */
> }
On 14/06/2022 12.38, Janis Schoetterl-Glausch wrote:
> On 5/31/22 12:15, Thomas Huth wrote:
>> The memop 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]>
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>> tools/testing/selftests/kvm/s390x/memop.c | 95 ++++++++++++++++++-----
>> 1 file changed, 77 insertions(+), 18 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
>> index 49f26f544127..e704c6fa5758 100644
>> --- a/tools/testing/selftests/kvm/s390x/memop.c
>> +++ b/tools/testing/selftests/kvm/s390x/memop.c
>> @@ -14,6 +14,7 @@
>>
>
> [...]
>
>> int main(int argc, char *argv[])
>> {
>> - int memop_cap, extension_cap;
>> + int memop_cap, extension_cap, idx;
>>
>> setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
>>
>> + ksft_print_header();
>> +
>> memop_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP);
>> extension_cap = kvm_check_cap(KVM_CAP_S390_MEM_OP_EXTENSION);
>> if (!memop_cap) {
>> - print_skip("CAP_S390_MEM_OP not supported");
>> - exit(KSFT_SKIP);
>> + ksft_exit_skip("CAP_S390_MEM_OP not supported.\n");
>> }
>>
>> - test_copy();
>> - if (extension_cap > 0) {
>> - test_copy_key();
>> - test_copy_key_storage_prot_override();
>> - test_copy_key_fetch_prot();
>> - test_copy_key_fetch_prot_override();
>> - test_errors_key();
>> - test_termination();
>> - test_errors_key_storage_prot_override();
>> - test_errors_key_fetch_prot_override_not_enabled();
>> - test_errors_key_fetch_prot_override_enabled();
>> - } else {
>> - print_skip("storage key memop extension not supported");
>> + ksft_set_plan(ARRAY_SIZE(testlist));
>> +
>> + for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
>> + if (testlist[idx].extension >= extension_cap) {
>
> This is reversed, should be
>
> if (testlist[idx].extension <= extension_cap) {
> or
> if (extension_cap >= testlist[idx].extension) {
Drat! The patch is already in Paolo's queue ... could you please send a
patch to fix this, so that Paolo can either squash it (not sure whether
that's still feasible) or queue it, too?
> I'd prefer the latter.
Me too.
Thanks,
Thomas
Fix the inverted logic of the memop extension capability check.
Fixes: 97da92c0ff92 ("KVM: s390: selftests: Use TAP interface in the memop test")
Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
---
Here you go.
Hope it doesn't get lost as a reply, but I can always resend
and it's not super critical after all.
tools/testing/selftests/kvm/s390x/memop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
index e704c6fa5758..e1056f20dfa1 100644
--- a/tools/testing/selftests/kvm/s390x/memop.c
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -769,7 +769,7 @@ int main(int argc, char *argv[])
ksft_set_plan(ARRAY_SIZE(testlist));
for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
- if (testlist[idx].extension >= extension_cap) {
+ if (extension_cap >= testlist[idx].extension) {
testlist[idx].test();
ksft_test_result_pass("%s\n", testlist[idx].name);
} else {
--
2.32.0
On 14/06/2022 18.26, Janis Schoetterl-Glausch wrote:
> Fix the inverted logic of the memop extension capability check.
>
> Fixes: 97da92c0ff92 ("KVM: s390: selftests: Use TAP interface in the memop test")
> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
> ---
>
>
> Here you go.
> Hope it doesn't get lost as a reply, but I can always resend
> and it's not super critical after all.
>
>
> tools/testing/selftests/kvm/s390x/memop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
> index e704c6fa5758..e1056f20dfa1 100644
> --- a/tools/testing/selftests/kvm/s390x/memop.c
> +++ b/tools/testing/selftests/kvm/s390x/memop.c
> @@ -769,7 +769,7 @@ int main(int argc, char *argv[])
> ksft_set_plan(ARRAY_SIZE(testlist));
>
> for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
> - if (testlist[idx].extension >= extension_cap) {
> + if (extension_cap >= testlist[idx].extension) {
> testlist[idx].test();
> ksft_test_result_pass("%s\n", testlist[idx].name);
> } else {
Thanks!
Reviewed-by: Thomas Huth <[email protected]>
Paolo, could you please queue this directly as a fix for the "Use TAP
interface in the memop test" patch that is currently already in your "next"
branch? (or in case you rebase that branch, squash it directly into that patch?)
Thanks,
Thomas
On 6/14/22 18:26, Janis Schoetterl-Glausch wrote:
> Fix the inverted logic of the memop extension capability check.
>
> Fixes: 97da92c0ff92 ("KVM: s390: selftests: Use TAP interface in the memop test")
> Signed-off-by: Janis Schoetterl-Glausch <[email protected]>
> ---
>
>
> Here you go.
> Hope it doesn't get lost as a reply, but I can always resend
> and it's not super critical after all.
>
>
> tools/testing/selftests/kvm/s390x/memop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
> index e704c6fa5758..e1056f20dfa1 100644
> --- a/tools/testing/selftests/kvm/s390x/memop.c
> +++ b/tools/testing/selftests/kvm/s390x/memop.c
> @@ -769,7 +769,7 @@ int main(int argc, char *argv[])
> ksft_set_plan(ARRAY_SIZE(testlist));
>
> for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) {
> - if (testlist[idx].extension >= extension_cap) {
> + if (extension_cap >= testlist[idx].extension) {
> testlist[idx].test();
> ksft_test_result_pass("%s\n", testlist[idx].name);
> } else {
Done, thanks!
Paolo