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

[Pierre Morel <pmorel@linux.ibm.com>]

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