Received-SPF: pass (google.com: domain of linux-kernel+bounces-118680-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <2099346e-c657-4781-97e1-7dcb41f66c43@collabora.com>
Date: Tue, 26 Mar 2024 14:24:22 +0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
 Shuah Khan <shuah@kernel.org>, linux-kselftest@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: selftests: Use TAP in the steal_time test
To: Thomas Huth <thuth@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>,
 kvm@vger.kernel.org
References: <20231019095900.450467-1-thuth@redhat.com>
Content-Language: en-US
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
In-Reply-To: <20231019095900.450467-1-thuth@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 10/19/23 2:59 PM, Thomas Huth wrote:
> For easier use of the tests in automation and for having some
> status information for the user while the test is running, let's
> provide some TAP output in this test.
LGTM. I was thinking why kselftest.h hasn't been included. I found out that
test_util.h includes kselftest.h.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  NB: This patch does not use the interface from kselftest_harness.h
>      since it is not very suitable for the for-loop in this patch.
> 
>  tools/testing/selftests/kvm/steal_time.c | 46 ++++++++++++------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index 171adfb2a6cb..aa6149eb9ea1 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
> @@ -81,20 +81,18 @@ static void steal_time_init(struct kvm_vcpu *vcpu, uint32_t i)
>  static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx)
>  {
>  	struct kvm_steal_time *st = addr_gva2hva(vm, (ulong)st_gva[vcpu_idx]);
> -	int i;
>  
> -	pr_info("VCPU%d:\n", vcpu_idx);
> -	pr_info("    steal:     %lld\n", st->steal);
> -	pr_info("    version:   %d\n", st->version);
> -	pr_info("    flags:     %d\n", st->flags);
> -	pr_info("    preempted: %d\n", st->preempted);
> -	pr_info("    u8_pad:    ");
> -	for (i = 0; i < 3; ++i)
> -		pr_info("%d", st->u8_pad[i]);
> -	pr_info("\n    pad:       ");
> -	for (i = 0; i < 11; ++i)
> -		pr_info("%d", st->pad[i]);
> -	pr_info("\n");
> +	ksft_print_msg("VCPU%d:\n", vcpu_idx);
> +	ksft_print_msg("    steal:     %lld\n", st->steal);
> +	ksft_print_msg("    version:   %d\n", st->version);
> +	ksft_print_msg("    flags:     %d\n", st->flags);
> +	ksft_print_msg("    preempted: %d\n", st->preempted);
> +	ksft_print_msg("    u8_pad:    %d %d %d\n",
> +			st->u8_pad[0], st->u8_pad[1], st->u8_pad[2]);
> +	ksft_print_msg("    pad:       %d %d %d %d %d %d %d %d %d %d %d\n",
> +			st->pad[0], st->pad[1], st->pad[2], st->pad[3],
> +			st->pad[4], st->pad[5], st->pad[6], st->pad[7],
> +			st->pad[8], st->pad[9], st->pad[10]);
>  }
>  
>  #elif defined(__aarch64__)
> @@ -197,10 +195,10 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx)
>  {
>  	struct st_time *st = addr_gva2hva(vm, (ulong)st_gva[vcpu_idx]);
>  
> -	pr_info("VCPU%d:\n", vcpu_idx);
> -	pr_info("    rev:     %d\n", st->rev);
> -	pr_info("    attr:    %d\n", st->attr);
> -	pr_info("    st_time: %ld\n", st->st_time);
> +	ksft_print_msg("VCPU%d:\n", vcpu_idx);
> +	ksft_print_msg("    rev:     %d\n", st->rev);
> +	ksft_print_msg("    attr:    %d\n", st->attr);
> +	ksft_print_msg("    st_time: %ld\n", st->st_time);
>  }
>  
>  #endif
> @@ -267,7 +265,9 @@ int main(int ac, char **av)
>  	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, ST_GPA_BASE, 1, gpages, 0);
>  	virt_map(vm, ST_GPA_BASE, ST_GPA_BASE, gpages);
>  
> +	ksft_print_header();
>  	TEST_REQUIRE(is_steal_time_supported(vcpus[0]));
> +	ksft_set_plan(NR_VCPUS);
>  
>  	/* Run test on each VCPU */
>  	for (i = 0; i < NR_VCPUS; ++i) {
> @@ -308,14 +308,14 @@ int main(int ac, char **av)
>  			    run_delay, stolen_time);
>  
>  		if (verbose) {
> -			pr_info("VCPU%d: total-stolen-time=%ld test-stolen-time=%ld", i,
> -				guest_stolen_time[i], stolen_time);
> -			if (stolen_time == run_delay)
> -				pr_info(" (BONUS: guest test-stolen-time even exactly matches test-run_delay)");
> -			pr_info("\n");
> +			ksft_print_msg("VCPU%d: total-stolen-time=%ld test-stolen-time=%ld%s\n",
> +				       i, guest_stolen_time[i], stolen_time,
> +				       stolen_time == run_delay ?
> +				       " (BONUS: guest test-stolen-time even exactly matches test-run_delay)" : "");
>  			steal_time_dump(vm, i);
>  		}
> +		ksft_test_result_pass("vcpu%d\n", i);
>  	}
>  
> -	return 0;
> +	ksft_finished();        /* Print results and exit() accordingly */
>  }

-- 
BR,
Muhammad Usama Anjum