Re: [kvm-unit-tests PATCH v4 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Nico Boehr <nrb@linux.ibm.com>]

On Tue, 2022-02-08 at 14:27 +0100, Pierre Morel wrote:
> We need in several tests to check if the VM we are running in
> is KVM.
> Let's add the test.
> 
> To check the VM type we use the STSI 3.2.2 instruction, let's
> define it's response structure in a central header.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>

> ---
>  lib/s390x/stsi.h | 32 +++++++++++++++++++++++++++
>  lib/s390x/vm.c   | 56
> ++++++++++++++++++++++++++++++++++++++++++++++--
>  lib/s390x/vm.h   |  3 +++
>  s390x/stsi.c     | 23 ++------------------
>  4 files changed, 91 insertions(+), 23 deletions(-)
>  create mode 100644 lib/s390x/stsi.h
> 
> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
> new file mode 100644
> index 00000000..9b40664f
> --- /dev/null
> +++ b/lib/s390x/stsi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */

This was taken from stsi.c which is GPL 2 only, so this probably should
be as well.

> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
> index a5b92863..38886b76 100644
> --- a/lib/s390x/vm.c
> +++ b/lib/s390x/vm.c
> @@ -12,6 +12,7 @@
>  #include <alloc_page.h>
>  #include <asm/arch_def.h>
>  #include "vm.h"
> +#include "stsi.h"
>  
>  /**
>   * Detect whether we are running with TCG (instead of KVM)
> @@ -26,9 +27,13 @@ bool vm_is_tcg(void)
>         if (initialized)
>                 return is_tcg;
>  
> -       buf = alloc_page();
> -       if (!buf)
> +       if (!vm_is_vm()) {
> +               initialized = true;
>                 return false;
> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
> index a5b92863..38886b76 100644
> --- a/lib/s390x/vm.c
> +++ b/lib/s390x/vm.c
> @@ -12,6 +12,7 @@
>  #include <alloc_page.h>
>  #include <asm/arch_def.h>
>  #include "vm.h"
> +#include "stsi.h"
>  
>  /**
>   * Detect whether we are running with TCG (instead of KVM)
> @@ -26,9 +27,13 @@ bool vm_is_tcg(void)
>         if (initialized)
>                 return is_tcg;
>  
> -       buf = alloc_page();
> -       if (!buf)
> +       if (!vm_is_vm()) {
> +               initialized = true;
>                 return false;

I would personally prefer return is_tcg here to make it obvious we're
relying on the previous initalization to false for subsequent calls.

> +       }
> +
> +       buf = alloc_page();
> +       assert(buf);
>  
>         if (stsi(buf, 1, 1, 1))
>                 goto out;
> @@ -43,3 +48,50 @@ out:
>         free_page(buf);
>         return is_tcg;
>  }
> +
> +/**
> + * Detect whether we are running with KVM
> + */
> +

No newline here.

> +bool vm_is_kvm(void)
> +{
> +       /* EBCDIC for "KVM/" */
> +       const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
> +       static bool initialized;
> +       static bool is_kvm;

Might make sense to initizalize these to false to make it consistent
with vm_is_tcg().