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

Pierre Morel
2022-01-17T15:03:58Z

On 1/11/22 14:08, Claudio Imbrenda wrote:
> On Mon, 10 Jan 2022 14:37:53 +0100
> Pierre Morel <pmorel@linux.ibm.com> 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 | 39 +++++++++++++++++++++++++++++++++++++++
>> lib/s390x/vm.h | 1 +
>> s390x/stsi.c | 23 ++---------------------
>> 4 files changed, 74 insertions(+), 21 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..02cc94a6
>> --- /dev/null
>> +++ b/lib/s390x/stsi.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Structures used to Store System Information
>> + *
>> + * Copyright (c) 2021 IBM Inc
>
> Copyright IBM Corp. 2021

OK

>
>> + */
>> +
>> +#ifndef _S390X_STSI_H_
>> +#define _S390X_STSI_H_
>
> [...]
>
>> +
>> +/**
>> + * Detect whether we are running with KVM
>> + */
>> +
>> +bool vm_is_kvm(void)
>> +{
>> + /* EBCDIC for "KVM/" */
>> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>> + static bool initialized;
>> + static bool is_kvm;
>> + struct sysinfo_3_2_2 *stsi_322;
>> +
>> + if (initialized)
>> + return is_kvm;
>> +
>> + if (stsi_get_fc() < 3) {
>> + initialized = true;
>> + return is_kvm;
>> + }
>> +
>> + stsi_322 = alloc_page();
>> + if (!stsi_322)
>> + return false;
>
> I don't like returning false if the allocation fails.
> The allocation should not fail: assert(stsi_322);

OK, right.

>
>> +
>> + if (stsi(stsi_322, 3, 2, 2))
>> + goto out;
>> +

Thanks for the review,

Pierre

--
Pierre Morel
IBM Lab Boeblingen