Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 31 Jul 2023 10:04:02 -0700
In-Reply-To: <20230731-91b64a6b787ba7e23b285785@orel>
Mime-Version: 1.0
References: <20230729003643.1053367-1-seanjc@google.com> <20230729003643.1053367-10-seanjc@google.com>
 <20230731-91b64a6b787ba7e23b285785@orel>
Message-ID: <ZMfpgu8bHH0jA8Si@google.com>
Subject: Re: [PATCH v4 09/34] KVM: selftests: Add a selftest for guest prints
 and formatted asserts
From:   Sean Christopherson <seanjc@google.com>
To:     Andrew Jones <ajones@ventanamicro.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, Marc Zyngier <maz@kernel.org>,
        Oliver Upton <oliver.upton@linux.dev>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>, kvm@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
        linux-kernel@vger.kernel.org, Thomas Huth <thuth@redhat.com>,
        "Philippe =?utf-8?Q?Mathieu-Daud=C3=A9?=" <philmd@linaro.org>,
        Aaron Lewis <aaronlewis@google.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Mon, Jul 31, 2023, Andrew Jones wrote:
> On Fri, Jul 28, 2023 at 05:36:18PM -0700, Sean Christopherson wrote:
> > From: Aaron Lewis <aaronlewis@google.com>
> > 
> > Add a test to exercise the various features in KVM selftest's local
> > snprintf() and compare them to LIBC's snprintf() to ensure they behave
> > the same.
> > 
> > This is not an exhaustive test.  KVM's local snprintf() does not
> > implement all the features LIBC does, e.g. KVM's local snprintf() does
> > not support floats or doubles, so testing for those features were
> > excluded.
> > 
> > Testing was added for the features that are expected to work to
> > support a minimal version of printf() in the guest.
> > 
> > Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  tools/testing/selftests/kvm/Makefile          |   1 +
> >  .../testing/selftests/kvm/guest_print_test.c  | 223 ++++++++++++++++++
> >  2 files changed, 224 insertions(+)
> >  create mode 100644 tools/testing/selftests/kvm/guest_print_test.c
> 
> I added this diff to this patch
> 
> diff --git a/tools/testing/selftests/kvm/guest_print_test.c b/tools/testing/selftests/kvm/guest_print_test.c
> index 3a9a5db9794e..602a23ea9f01 100644
> --- a/tools/testing/selftests/kvm/guest_print_test.c
> +++ b/tools/testing/selftests/kvm/guest_print_test.c
> @@ -115,7 +115,7 @@ static void run_test(struct kvm_vcpu *vcpu, const char *expected_printf,
>         while (1) {
>                 vcpu_run(vcpu);
>  
> -               TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> +               TEST_ASSERT(run->exit_reason == UCALL_EXIT_REASON,
>                             "Unexpected exit reason: %u (%s),\n",
>                             run->exit_reason,
>                             exit_reason_str(run->exit_reason));
> @@ -161,7 +161,7 @@ static void test_limits(void)
>         run = vcpu->run;
>         vcpu_run(vcpu);
>  
> -       TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> +       TEST_ASSERT(run->exit_reason == UCALL_EXIT_REASON,
>                     "Unexpected exit reason: %u (%s),\n",
>                     run->exit_reason,
>                     exit_reason_str(run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index 4cf69fa8bfba..4adf526dc378 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -6,8 +6,19 @@
>   */
>  #ifndef SELFTEST_KVM_UCALL_COMMON_H
>  #define SELFTEST_KVM_UCALL_COMMON_H
> +#include <linux/kvm.h>
>  #include "test_util.h"
>  
> +#if defined(__aarch64__)
> +#define UCALL_EXIT_REASON      KVM_EXIT_MMIO
> +#elif defined(__x86_64__)
> +#define UCALL_EXIT_REASON      KVM_EXIT_IO
> +#elif defined(__s390x__)
> +#define UCALL_EXIT_REASON      KVM_EXIT_S390_SIEIC
> +#elif defined(__riscv)
> +#define UCALL_EXIT_REASON      KVM_EXIT_RISCV_SBI
> +#endif
> +
>  /* Common ucalls */
>  enum {
>         UCALL_NONE,
> 
> and then compiled the test for riscv and it passed. I also ran all other
> riscv tests successfully.

Can I have your SoB for the ucall_common.h patch?  I'll write a changelog and fold
in a separate prep patch for that change.

> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index f65889f5a083..f2a8b3262f17 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -123,6 +123,7 @@ TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
> >  TEST_GEN_PROGS_x86_64 += demand_paging_test
> >  TEST_GEN_PROGS_x86_64 += dirty_log_test
> >  TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
> > +TEST_GEN_PROGS_x86_64 += guest_print_test

Argh, this is why ARM didn't fail for me, the test was only built for x86.  I'll
double check that ARM works with the above, and also enable the test for all
architectures.  If the printf stuff doesn't work on s390, then we definitely want
to know before this is fully merged.

Thanks Drew!