[PATCH 1/2] selftests: kvm: remove print_skip()

[PATCH 1/2] selftests: kvm: remove print_skip()

[Muhammad Usama Anjum]

Replace print_skip() with ksft_exit_skip() to simplify the code and
directly use the skip API provided by kselftest.h.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 .../testing/selftests/kvm/aarch64/vgic_init.c  |  4 ++--
 .../testing/selftests/kvm/demand_paging_test.c |  3 +--
 tools/testing/selftests/kvm/dirty_log_test.c   |  8 +++-----
 .../testing/selftests/kvm/include/test_util.h  |  1 -
 tools/testing/selftests/kvm/lib/assert.c       |  6 ++----
 tools/testing/selftests/kvm/lib/test_util.c    | 11 -----------
 tools/testing/selftests/kvm/s390x/memop.c      | 18 ++++++------------
 .../selftests/kvm/x86_64/hyperv_cpuid.c        | 13 +++++--------
 .../kvm/x86_64/hyperv_extended_hypercalls.c    |  6 ++----
 .../selftests/kvm/x86_64/ucna_injection_test.c |  6 ++----
 10 files changed, 23 insertions(+), 53 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c
index b3b5fb0ff0a9a..556c3230eb093 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_init.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c
@@ -757,8 +757,8 @@ int main(int ac, char **av)
 	}
 
 	if (!cnt_impl) {
-		print_skip("No GICv2 nor GICv3 support");
-		exit(KSFT_SKIP);
+		ksft_exit_skip("No GICv2 nor GICv3 support\n");
 	}
+
 	return 0;
 }
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 0202b78f8680a..ae60b3a5fb9e5 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -353,8 +353,7 @@ int main(int argc, char *argv[])
 
 int main(void)
 {
-	print_skip("__NR_userfaultfd must be present for userfaultfd test");
-	return KSFT_SKIP;
+	ksft_exit_skip("__NR_userfaultfd must be present for userfaultfd test\n");
 }
 
 #endif /* __NR_userfaultfd */
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index aacf80f574391..e5d3b01ec9508 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -692,11 +692,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	uint32_t ring_buf_idx = 0;
 	int sem_val;
 
-	if (!log_mode_supported()) {
-		print_skip("Log mode '%s' not supported",
-			   log_modes[host_log_mode].name);
-		return;
-	}
+	if (!log_mode_supported())
+		ksft_exit_skip("Log mode '%s' not supported\n",
+			       log_modes[host_log_mode].name);
 
 	/*
 	 * We reserve page table for 2 times of extra dirty mem which
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 3e473058849ff..472fce41737e0 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -35,7 +35,6 @@ static inline int _no_printf(const char *format, ...) { return 0; }
 #define pr_info(...) _no_printf(__VA_ARGS__)
 #endif
 
-void __printf(1, 2) print_skip(const char *fmt, ...);
 #define __TEST_REQUIRE(f, fmt, ...)				\
 do {								\
 	if (!(f))						\
diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index b49690658c606..33651f5b3a7fd 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -85,10 +85,8 @@ test_assert(bool exp, const char *exp_str,
 		}
 		va_end(ap);
 
-		if (errno == EACCES) {
-			print_skip("Access denied - Exiting");
-			exit(KSFT_SKIP);
-		}
+		if (errno == EACCES)
+			ksft_exit_skip("Access denied - Exiting\n");
 		exit(254);
 	}
 
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 8ed0b74ae8373..6e8ac25403bb3 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -121,17 +121,6 @@ struct timespec timespec_div(struct timespec ts, int divisor)
 	return timespec_add_ns((struct timespec){0}, ns);
 }
 
-void print_skip(const char *fmt, ...)
-{
-	va_list ap;
-
-	assert(fmt);
-	va_start(ap, fmt);
-	vprintf(fmt, ap);
-	va_end(ap);
-	puts(", skipping test");
-}
-
 bool thp_configured(void)
 {
 	int ret;
diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
index f2df7416be847..d7cd4b4eb6228 100644
--- a/tools/testing/selftests/kvm/s390x/memop.c
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -884,10 +884,8 @@ static void test_copy_key_fetch_prot_override(void)
 
 	guest_0_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, 0);
 	guest_last_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, last_page_addr);
-	if (guest_0_page != 0 || guest_last_page != last_page_addr) {
-		print_skip("did not allocate guest pages at required positions");
-		goto out;
-	}
+	if (guest_0_page != 0 || guest_last_page != last_page_addr)
+		ksft_exit_skip("did not allocate guest pages at required positions\n");
 
 	HOST_SYNC(t.vcpu, STAGE_INITED);
 	t.run->s.regs.crs[0] |= CR0_FETCH_PROTECTION_OVERRIDE;
@@ -923,10 +921,8 @@ static void test_errors_key_fetch_prot_override_not_enabled(void)
 
 	guest_0_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, 0);
 	guest_last_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, last_page_addr);
-	if (guest_0_page != 0 || guest_last_page != last_page_addr) {
-		print_skip("did not allocate guest pages at required positions");
-		goto out;
-	}
+	if (guest_0_page != 0 || guest_last_page != last_page_addr)
+		ksft_exit_skip("did not allocate guest pages at required positions\n");
 	HOST_SYNC(t.vcpu, STAGE_INITED);
 	HOST_SYNC(t.vcpu, STAGE_SKEYS_SET);
 
@@ -944,10 +940,8 @@ static void test_errors_key_fetch_prot_override_enabled(void)
 
 	guest_0_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, 0);
 	guest_last_page = vm_vaddr_alloc(t.kvm_vm, PAGE_SIZE, last_page_addr);
-	if (guest_0_page != 0 || guest_last_page != last_page_addr) {
-		print_skip("did not allocate guest pages at required positions");
-		goto out;
-	}
+	if (guest_0_page != 0 || guest_last_page != last_page_addr)
+		ksft_exit_skip("did not allocate guest pages at required positions");
 	HOST_SYNC(t.vcpu, STAGE_INITED);
 	t.run->s.regs.crs[0] |= CR0_FETCH_PROTECTION_OVERRIDE;
 	t.run->kvm_dirty_regs = KVM_SYNC_CRS;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 4f5881d4ef66d..695c45635d257 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -144,10 +144,9 @@ int main(int argc, char *argv[])
 	free((void *)hv_cpuid_entries);
 
 	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
-	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
-		print_skip("Enlightened VMCS is unsupported");
-		goto do_sys;
-	}
+	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS))
+		ksft_exit_skip("Enlightened VMCS is unsupported\n");
+
 	vcpu_enable_evmcs(vcpu);
 	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
 	test_hv_cpuid(hv_cpuid_entries, true);
@@ -155,10 +154,8 @@ int main(int argc, char *argv[])
 
 do_sys:
 	/* Test system ioctl version */
-	if (!kvm_has_cap(KVM_CAP_SYS_HYPERV_CPUID)) {
-		print_skip("KVM_CAP_SYS_HYPERV_CPUID not supported");
-		goto out;
-	}
+	if (!kvm_has_cap(KVM_CAP_SYS_HYPERV_CPUID))
+		ksft_exit_skip("KVM_CAP_SYS_HYPERV_CPUID not supported\n");
 
 	test_hv_cpuid_e2big(vm, NULL);
 
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
index 949e08e98f315..d37212a27990b 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
@@ -47,10 +47,8 @@ int main(void)
 
 	/* Verify if extended hypercalls are supported */
 	if (!kvm_cpuid_has(kvm_get_supported_hv_cpuid(),
-			   HV_ENABLE_EXTENDED_HYPERCALLS)) {
-		print_skip("Extended calls not supported by the kernel");
-		exit(KSFT_SKIP);
-	}
+			   HV_ENABLE_EXTENDED_HYPERCALLS))
+		ksft_exit_skip("Extended calls not supported by the kernel\n");
 
 	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 	run = vcpu->run;
diff --git a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
index 57f157c06b393..1dcb37a1f0be9 100644
--- a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
+++ b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
@@ -273,10 +273,8 @@ int main(int argc, char *argv[])
 	kvm_ioctl(vm->kvm_fd, KVM_X86_GET_MCE_CAP_SUPPORTED,
 		  &supported_mcg_caps);
 
-	if (!(supported_mcg_caps & MCG_CMCI_P)) {
-		print_skip("MCG_CMCI_P is not supported");
-		exit(KSFT_SKIP);
-	}
+	if (!(supported_mcg_caps & MCG_CMCI_P))
+		ksft_exit_skip("MCG_CMCI_P is not supported\n");
 
 	ucna_vcpu = create_vcpu_with_mce_cap(vm, 0, true, ucna_injection_guest_code);
 	cmcidis_vcpu = create_vcpu_with_mce_cap(vm, 1, false, cmci_disabled_guest_code);
-- 
2.39.2

[PATCH 2/2] selftests: kvm: replace exit() with ksft_exit_fail_msg()

[Muhammad Usama Anjum]

The KSFT_FAIL, exit code must be used instead of exit(254). The 254 code
here seems like anciant relic. Its even better if we use
ksft_exit_fail_msg() which will print out "Bail out" meaning the test
exited without completing. This string is TAP protocol specific.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/kvm/lib/assert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index 33651f5b3a7fd..db648a7ac429b 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -87,7 +87,7 @@ test_assert(bool exp, const char *exp_str,
 
 		if (errno == EACCES)
 			ksft_exit_skip("Access denied - Exiting\n");
-		exit(254);
+		ksft_exit_fail_msg("\n");
 	}
 
 	return;
-- 
2.39.2

Re: [PATCH 1/2] selftests: kvm: remove print_skip()

[Dev Jain]

On 6/12/24 16:14, Muhammad Usama Anjum wrote:
>
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 4f5881d4ef66d..695c45635d257 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -144,10 +144,9 @@ int main(int argc, char *argv[])
>   	free((void *)hv_cpuid_entries);
>   
>   	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
> -	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
> -		print_skip("Enlightened VMCS is unsupported");
> -		goto do_sys;
> -	}
> +	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS))
> +		ksft_exit_skip("Enlightened VMCS is unsupported\n");
> +

Isn't it incorrect to delete 'goto do_sys'? ksft_exit_skip() will exit and the
program will never jump to that label. At other places too you have deleted the 'goto'.


>   	vcpu_enable_evmcs(vcpu);
>   	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
>   	test_hv_cpuid(hv_cpuid_entries, true);
> @@ -155,10 +154,8 @@ int main(int argc, char *argv[])
>   
>   do_sys:
>   	/* Test system ioctl version */
> -	if (!kvm_has_cap(KVM_CAP_SYS_HYPERV_CPUID)) {
> -		print_skip("KVM_CAP_SYS_HYPERV_CPUID not supported");
> -		goto out;
> -	}
> +	if (!kvm_has_cap(KVM_CAP_SYS_HYPERV_CPUID))
> +		ksft_exit_skip("KVM_CAP_SYS_HYPERV_CPUID not supported\n");
>   
>   	test_hv_cpuid_e2big(vm, NULL);
>   
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
> index 949e08e98f315..d37212a27990b 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_extended_hypercalls.c
> @@ -47,10 +47,8 @@ int main(void)
>   
>   	/* Verify if extended hypercalls are supported */
>   	if (!kvm_cpuid_has(kvm_get_supported_hv_cpuid(),
> -			   HV_ENABLE_EXTENDED_HYPERCALLS)) {
> -		print_skip("Extended calls not supported by the kernel");
> -		exit(KSFT_SKIP);
> -	}
> +			   HV_ENABLE_EXTENDED_HYPERCALLS))
> +		ksft_exit_skip("Extended calls not supported by the kernel\n");
>   
>   	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>   	run = vcpu->run;
> diff --git a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> index 57f157c06b393..1dcb37a1f0be9 100644
> --- a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
> @@ -273,10 +273,8 @@ int main(int argc, char *argv[])
>   	kvm_ioctl(vm->kvm_fd, KVM_X86_GET_MCE_CAP_SUPPORTED,
>   		  &supported_mcg_caps);
>   
> -	if (!(supported_mcg_caps & MCG_CMCI_P)) {
> -		print_skip("MCG_CMCI_P is not supported");
> -		exit(KSFT_SKIP);
> -	}
> +	if (!(supported_mcg_caps & MCG_CMCI_P))
> +		ksft_exit_skip("MCG_CMCI_P is not supported\n");
>   
>   	ucna_vcpu = create_vcpu_with_mce_cap(vm, 0, true, ucna_injection_guest_code);
>   	cmcidis_vcpu = create_vcpu_with_mce_cap(vm, 1, false, cmci_disabled_guest_code);

Re: [PATCH 1/2] selftests: kvm: remove print_skip()

[Sean Christopherson]

On Wed, Jun 12, 2024, Dev Jain wrote:
> 
> On 6/12/24 16:14, Muhammad Usama Anjum wrote:
> > 
> > 
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > index 4f5881d4ef66d..695c45635d257 100644
> > --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > @@ -144,10 +144,9 @@ int main(int argc, char *argv[])
> >   	free((void *)hv_cpuid_entries);
> >   	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
> > -	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
> > -		print_skip("Enlightened VMCS is unsupported");
> > -		goto do_sys;
> > -	}
> > +	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS))
> > +		ksft_exit_skip("Enlightened VMCS is unsupported\n");
> > +
> 
> Isn't it incorrect to delete 'goto do_sys'? ksft_exit_skip() will exit and the
> program will never jump to that label. At other places too you have deleted the 'goto'.

Ya, exiting instead of continuing on will break these tests.

Re: [PATCH 2/2] selftests: kvm: replace exit() with ksft_exit_fail_msg()

[Sean Christopherson]

On Wed, Jun 12, 2024, Muhammad Usama Anjum wrote:
> The KSFT_FAIL, exit code must be used instead of exit(254).

This needs more justification.  KVM selftests have worked just fine for 6+ years
using exit(254), so stating they "must" use KSFT_FAIL is obviously not true.

I'm not personally opposed to switching to KSFT_FAIL, but it is a potentially
breaking change.  E.g. some of Google's internal test infrastructure explicitly
relies on the exit code being 254.  I don't _think_ that infrastructure interacts
with KVM selftests, nor do I think that forcing upstream KVM selftests to sacrifice
TAP compliance just to play nice with someone's crusty test infrastructure is a
good tradeoff, but this and all of the TAP compliance work needs to be done with
more thought and care.

> The 254 code here seems like anciant relic.

As above, AFAICT it comes from Google's internal test infrastructure (KVM selftests
came from Google).

> Its even better if we use ksft_exit_fail_msg() which will print out "Bail
> out" meaning the test exited without completing. This string is TAP protocol
> specific.

This is debatable and not obviously correct.  The documentation says:

  Bail out!
  As an emergency measure a test script can decide that further tests are
  useless (e.g. missing dependencies) and testing should stop immediately. In
  that case the test script prints the magic words

which suggests that a test should only emit "Bail out!" if it wants to stop
entirely.  We definitely don't want KVM selftests to bail out if a TEST_ASSERT()
fails in one testcase.

> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  tools/testing/selftests/kvm/lib/assert.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
> index 33651f5b3a7fd..db648a7ac429b 100644
> --- a/tools/testing/selftests/kvm/lib/assert.c
> +++ b/tools/testing/selftests/kvm/lib/assert.c
> @@ -87,7 +87,7 @@ test_assert(bool exp, const char *exp_str,
>  
>  		if (errno == EACCES)
>  			ksft_exit_skip("Access denied - Exiting\n");
> -		exit(254);
> +		ksft_exit_fail_msg("\n");
>  	}
>  
>  	return;
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] selftests: kvm: replace exit() with ksft_exit_fail_msg()

[Muhammad Usama Anjum]

Hi Sean,

Thank you for replying in detail. I wasn't aware of true origin of these tests.

On 6/13/24 12:01 AM, Sean Christopherson wrote:
> On Wed, Jun 12, 2024, Muhammad Usama Anjum wrote:
>> The KSFT_FAIL, exit code must be used instead of exit(254).
> 
> This needs more justification.  KVM selftests have worked just fine for 6+ years
> using exit(254), so stating they "must" use KSFT_FAIL is obviously not true.
The selftests scripts read the exit code and mark the test status
pass/fail. Maybe selftests run_tests target isn't being used or this code
path wasn't being triggered.

> 
> I'm not personally opposed to switching to KSFT_FAIL, but it is a potentially
> breaking change.  E.g. some of Google's internal test infrastructure explicitly
> relies on the exit code being 254.  I don't _think_ that infrastructure interacts
> with KVM selftests, nor do I think that forcing upstream KVM selftests to sacrifice
> TAP compliance just to play nice with someone's crusty test infrastructure is a
> good tradeoff, but this and all of the TAP compliance work needs to be done with
> more thought and care.
You have given your perspective from KVM selftest suite perspective. I've
been thinking from kselftests subsystem perspective that how TAP compliance
and exit codes help the entire subsystem. It is understandable from KVM
suite's perspective as not all the suites are compliant and work the same.

> 
>> The 254 code here seems like anciant relic.
> 
> As above, AFAICT it comes from Google's internal test infrastructure (KVM selftests
> came from Google).
> 
>> Its even better if we use ksft_exit_fail_msg() which will print out "Bail
>> out" meaning the test exited without completing. This string is TAP protocol
>> specific.
> 
> This is debatable and not obviously correct.  The documentation says:
> 
>   Bail out!
>   As an emergency measure a test script can decide that further tests are
>   useless (e.g. missing dependencies) and testing should stop immediately. In
>   that case the test script prints the magic words
> 
> which suggests that a test should only emit "Bail out!" if it wants to stop
> entirely.  We definitely don't want KVM selftests to bail out if a TEST_ASSERT()
> fails in one testcase.
But KVM tests are bailing out if assert fails, exit(254) is being called
which stops the further execution of the test cases. It is same as
ksft_exit_fail_msg() behavior.

> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>  tools/testing/selftests/kvm/lib/assert.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
>> index 33651f5b3a7fd..db648a7ac429b 100644
>> --- a/tools/testing/selftests/kvm/lib/assert.c
>> +++ b/tools/testing/selftests/kvm/lib/assert.c
>> @@ -87,7 +87,7 @@ test_assert(bool exp, const char *exp_str,
>>  
>>  		if (errno == EACCES)
>>  			ksft_exit_skip("Access denied - Exiting\n");
>> -		exit(254);
>> +		ksft_exit_fail_msg("\n");
>>  	}
>>  
>>  	return;
>> -- 
>> 2.39.2
>>
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 2/2] selftests: kvm: replace exit() with ksft_exit_fail_msg()

[Sean Christopherson]

On Thu, Jun 13, 2024, Muhammad Usama Anjum wrote:
> > As above, AFAICT it comes from Google's internal test infrastructure (KVM selftests
> > came from Google).
> > 
> >> Its even better if we use ksft_exit_fail_msg() which will print out "Bail
> >> out" meaning the test exited without completing. This string is TAP protocol
> >> specific.
> > 
> > This is debatable and not obviously correct.  The documentation says:
> > 
> >   Bail out!
> >   As an emergency measure a test script can decide that further tests are
> >   useless (e.g. missing dependencies) and testing should stop immediately. In
> >   that case the test script prints the magic words
> > 
> > which suggests that a test should only emit "Bail out!" if it wants to stop
> > entirely.  We definitely don't want KVM selftests to bail out if a TEST_ASSERT()
> > fails in one testcase.
> But KVM tests are bailing out if assert fails, exit(254) is being called
> which stops the further execution of the test cases.

Not if the TEST_ASSERT() fires from within a test fixture, in which case the
magic in tools/testing/selftests/kselftest_harness.h captures the failure but
continues on with the next test.

Re: [PATCH 1/2] selftests: kvm: remove print_skip()

[kernel test robot]

Hi Muhammad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kvm/queue]
[also build test WARNING on kvmarm/next kvms390/next linus/master v6.10-rc3 next-20240613]
[cannot apply to kvm/linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/selftests-kvm-replace-exit-with-ksft_exit_fail_msg/20240612-184820
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
patch link:    https://lore.kernel.org/r/20240612104500.425012-1-usama.anjum%40collabora.com
patch subject: [PATCH 1/2] selftests: kvm: remove print_skip()
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240615/202406151641.BjqL765q-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202406151641.BjqL765q-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> x86_64/hyperv_cpuid.c:155:1: warning: unused label 'do_sys' [-Wunused-label]
     155 | do_sys:
         | ^~~~~~~
>> x86_64/hyperv_cpuid.c:165:1: warning: unused label 'out' [-Wunused-label]
     165 | out:
         | ^~~~
   2 warnings generated.


vim +/do_sys +155 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c

7edcb73433276d Vitaly Kuznetsov     2018-12-10  128  
7edcb73433276d Vitaly Kuznetsov     2018-12-10  129  int main(int argc, char *argv[])
7edcb73433276d Vitaly Kuznetsov     2018-12-10  130  {
7edcb73433276d Vitaly Kuznetsov     2018-12-10  131  	struct kvm_vm *vm;
813e38cd6d7b42 Sean Christopherson  2022-06-14  132  	const struct kvm_cpuid2 *hv_cpuid_entries;
5c6e31b3bc4b52 Sean Christopherson  2022-02-15  133  	struct kvm_vcpu *vcpu;
7edcb73433276d Vitaly Kuznetsov     2018-12-10  134  
7ed397d107d461 Sean Christopherson  2022-05-27  135  	TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
7edcb73433276d Vitaly Kuznetsov     2018-12-10  136  
5c6e31b3bc4b52 Sean Christopherson  2022-02-15  137  	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  138  
8b460692fee46a Vitaly Kuznetsov     2020-09-29  139  	/* Test vCPU ioctl version */
5c6e31b3bc4b52 Sean Christopherson  2022-02-15  140  	test_hv_cpuid_e2big(vm, vcpu);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  141  
768e9a61856b75 Sean Christopherson  2022-06-02  142  	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  143  	test_hv_cpuid(hv_cpuid_entries, false);
813e38cd6d7b42 Sean Christopherson  2022-06-14  144  	free((void *)hv_cpuid_entries);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  145  
1ecbb337fa107c Sean Christopherson  2022-06-14  146  	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
a9af8b5d3d419a Muhammad Usama Anjum 2024-06-12  147  	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS))
a9af8b5d3d419a Muhammad Usama Anjum 2024-06-12  148  		ksft_exit_skip("Enlightened VMCS is unsupported\n");
a9af8b5d3d419a Muhammad Usama Anjum 2024-06-12  149  
768e9a61856b75 Sean Christopherson  2022-06-02  150  	vcpu_enable_evmcs(vcpu);
768e9a61856b75 Sean Christopherson  2022-06-02  151  	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  152  	test_hv_cpuid(hv_cpuid_entries, true);
813e38cd6d7b42 Sean Christopherson  2022-06-14  153  	free((void *)hv_cpuid_entries);
8b460692fee46a Vitaly Kuznetsov     2020-09-29  154  
8b460692fee46a Vitaly Kuznetsov     2020-09-29 @155  do_sys:

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki