Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Thu, 14 Apr 2022 13:51:10 +0200
From:   Claudio Imbrenda <imbrenda@linux.ibm.com>
To:     Thomas Huth <thuth@redhat.com>
Cc:     kvm@vger.kernel.org,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        David Hildenbrand <david@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Shuah Khan <shuah@kernel.org>,
        Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Subject: Re: [PATCH 3/4] KVM: s390: selftests: Use TAP interface in the
 tprot test
Message-ID: <20220414135110.6b2baead@p-imbrenda>
In-Reply-To: <20220414105322.577439-4-thuth@redhat.com>
References: <20220414105322.577439-1-thuth@redhat.com>
        <20220414105322.577439-4-thuth@redhat.com>
Organization: IBM
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 14 Apr 2022 12:53:21 +0200
Thomas Huth <thuth@redhat.com> wrote:

> 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.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tools/testing/selftests/kvm/s390x/tprot.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/s390x/tprot.c b/tools/testing/selftests/kvm/s390x/tprot.c
> index c097b9db495e..a714b4206e95 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)
> @@ -69,6 +70,7 @@ enum stage {
>  	STAGE_INIT_FETCH_PROT_OVERRIDE,
>  	TEST_FETCH_PROT_OVERRIDE,
>  	TEST_STORAGE_PROT_OVERRIDE,
> +	NUM_STAGES			/* this must be the last entry */
>  };
>  
>  struct test {
> @@ -196,6 +198,7 @@ static void guest_code(void)
>  	}									\
>  	ASSERT_EQ(uc.cmd, UCALL_SYNC);						\
>  	ASSERT_EQ(uc.args[1], __stage);						\
> +	ksft_test_result_pass("" #stage "\n");					\
>  })
>  
>  int main(int argc, char *argv[])
> @@ -204,6 +207,9 @@ int main(int argc, char *argv[])
>  	struct kvm_run *run;
>  	vm_vaddr_t guest_0_page;
>  
> +	ksft_print_header();
> +	ksft_set_plan(NUM_STAGES - 1);	/* STAGE_END is not counted, thus - 1 */
> +
>  	vm = vm_create_default(VCPU_ID, 0, guest_code);
>  	run = vcpu_state(vm, VCPU_ID);
>  
> @@ -213,7 +219,7 @@ int main(int argc, char *argv[])
>  
>  	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");
> +		ksft_print_msg("Did not allocate page at 0 for fetch protection override tests\n");

will this print a skip, though?

or you don't want to print a skip because then the numbering in the
planning doesn't match anymore? in which case, is there an easy way to
fix it?

>  	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);
> @@ -224,4 +230,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();
>  }