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

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

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