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

Pierre Morel
2022-02-14T07:53:29Z

On 2/8/22 16:35, Nico Boehr wrote:
> 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.

OK

>
>> +       }
>> +
>> +       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.

ok

>
>> +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().
>

OK

--
Pierre Morel
IBM Lab Boeblingen