diff mbox series

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

Message ID 20240612104500.425012-1-usama.anjum@collabora.com (mailing list archive)
State New
Headers show
Series [1/2] selftests: kvm: remove print_skip() | expand

Commit Message

Muhammad Usama Anjum June 12, 2024, 10:44 a.m. UTC 
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(-)

Comments

Dev Jain June 12, 2024, 11:13 a.m. UTC | #1 
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);
Sean Christopherson June 12, 2024, 6:10 p.m. UTC | #2 
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.
kernel test robot June 15, 2024, 1:01 p.m. UTC | #3 
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:
diff mbox series

Patch

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);