diff mbox series

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

Message ID 20220110133755.22238-3-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series S390x: CPU Topology Information | expand

Commit Message

Pierre Morel Jan. 10, 2022, 1:37 p.m. UTC
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

Comments

Janosch Frank Jan. 11, 2022, 12:27 p.m. UTC | #1
On 1/10/22 14:37, 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   | 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
> + */
> +
> +#ifndef _S390X_STSI_H_
> +#define _S390X_STSI_H_
> +
> +struct sysinfo_3_2_2 {
> +	uint8_t reserved[31];
> +	uint8_t count;
> +	struct {
> +		uint8_t reserved2[4];
> +		uint16_t total_cpus;
> +		uint16_t conf_cpus;
> +		uint16_t standby_cpus;
> +		uint16_t reserved_cpus;
> +		uint8_t name[8];
> +		uint32_t caf;
> +		uint8_t cpi[16];
> +		uint8_t reserved5[3];
> +		uint8_t ext_name_encoding;
> +		uint32_t reserved3;
> +		uint8_t uuid[16];
> +	} vm[8];
> +	uint8_t reserved4[1504];
> +	uint8_t ext_names[8][256];
> +};
> +
> +#endif  /* _S390X_STSI_H_ */
> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
> index a5b92863..3e11401e 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)

We could add a fc < 3 check to the vm_is_tcg() function and add a 
vm_is_lpar() which does a simple fc ==1 check.

> @@ -43,3 +44,41 @@ out:
>   	free_page(buf);
>   	return is_tcg;
>   }
> +
> +/**
> + * 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;
> +
> +	if (stsi(stsi_322, 3, 2, 2))
> +		goto out;
> +
> +	/*
> +	 * If the manufacturer string is "KVM/" in EBCDIC, then we
> +	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
> +	 */
> +	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));

So I had a look at this before Christmas and I think it's wrong.

QEMU will still set the cpi to KVM/LINUX if we are under tcg.
So we need to do add a !tcg check here and fix this comment.

I.e. we always have the KVM/LINUX cpi but if we're under TCG the 
manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and 
if we want to fix this at some point or not.

> +	initialized = true;
> +out:
> +	free_page(stsi_322);
> +	return is_kvm;
> +}
> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
> index 7abba0cc..44097b4a 100644
> --- a/lib/s390x/vm.h
> +++ b/lib/s390x/vm.h
> @@ -9,5 +9,6 @@
>   #define _S390X_VM_H_
>   
>   bool vm_is_tcg(void);
> +bool vm_is_kvm(void);
>   
>   #endif  /* _S390X_VM_H_ */
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 391f8849..1ed045e2 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -13,27 +13,8 @@
>   #include <asm/asm-offsets.h>
>   #include <asm/interrupt.h>
>   #include <smp.h>
> +#include "stsi.h"
>   
> -struct stsi_322 {
> -	uint8_t reserved[31];
> -	uint8_t count;
> -	struct {
> -		uint8_t reserved2[4];
> -		uint16_t total_cpus;
> -		uint16_t conf_cpus;
> -		uint16_t standby_cpus;
> -		uint16_t reserved_cpus;
> -		uint8_t name[8];
> -		uint32_t caf;
> -		uint8_t cpi[16];
> -		uint8_t reserved5[3];
> -		uint8_t ext_name_encoding;
> -		uint32_t reserved3;
> -		uint8_t uuid[16];
> -	} vm[8];
> -	uint8_t reserved4[1504];
> -	uint8_t ext_names[8][256];
> -};
>   static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>   
>   static void test_specs(void)
> @@ -91,7 +72,7 @@ static void test_3_2_2(void)
>   	/* EBCDIC for "KVM/" */
>   	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>   	const char vm_name_ext[] = "kvm-unit-test";
> -	struct stsi_322 *data = (void *)pagebuf;
> +	struct sysinfo_3_2_2 *data = (void *)pagebuf;
>   
>   	report_prefix_push("3.2.2");
>   
>
Claudio Imbrenda Jan. 11, 2022, 1:08 p.m. UTC | #2
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

> + */
> +
> +#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);

> +
> +	if (stsi(stsi_322, 3, 2, 2))
> +		goto out;
> +
> +	/*
> +	 * If the manufacturer string is "KVM/" in EBCDIC, then we
> +	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
> +	 */
> +	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));
> +	initialized = true;
> +out:
> +	free_page(stsi_322);
> +	return is_kvm;
> +}
> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
> index 7abba0cc..44097b4a 100644
> --- a/lib/s390x/vm.h
> +++ b/lib/s390x/vm.h
> @@ -9,5 +9,6 @@
>  #define _S390X_VM_H_
>  
>  bool vm_is_tcg(void);
> +bool vm_is_kvm(void);
>  
>  #endif  /* _S390X_VM_H_ */
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 391f8849..1ed045e2 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -13,27 +13,8 @@
>  #include <asm/asm-offsets.h>
>  #include <asm/interrupt.h>
>  #include <smp.h>
> +#include "stsi.h"
>  
> -struct stsi_322 {
> -	uint8_t reserved[31];
> -	uint8_t count;
> -	struct {
> -		uint8_t reserved2[4];
> -		uint16_t total_cpus;
> -		uint16_t conf_cpus;
> -		uint16_t standby_cpus;
> -		uint16_t reserved_cpus;
> -		uint8_t name[8];
> -		uint32_t caf;
> -		uint8_t cpi[16];
> -		uint8_t reserved5[3];
> -		uint8_t ext_name_encoding;
> -		uint32_t reserved3;
> -		uint8_t uuid[16];
> -	} vm[8];
> -	uint8_t reserved4[1504];
> -	uint8_t ext_names[8][256];
> -};
>  static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>  
>  static void test_specs(void)
> @@ -91,7 +72,7 @@ static void test_3_2_2(void)
>  	/* EBCDIC for "KVM/" */
>  	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>  	const char vm_name_ext[] = "kvm-unit-test";
> -	struct stsi_322 *data = (void *)pagebuf;
> +	struct sysinfo_3_2_2 *data = (void *)pagebuf;
>  
>  	report_prefix_push("3.2.2");
>
Pierre Morel Jan. 17, 2022, 2:57 p.m. UTC | #3
On 1/11/22 13:27, Janosch Frank wrote:
> On 1/10/22 14:37, 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   | 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
>> + */
>> +
>> +#ifndef _S390X_STSI_H_
>> +#define _S390X_STSI_H_
>> +
>> +struct sysinfo_3_2_2 {
>> +    uint8_t reserved[31];
>> +    uint8_t count;
>> +    struct {
>> +        uint8_t reserved2[4];
>> +        uint16_t total_cpus;
>> +        uint16_t conf_cpus;
>> +        uint16_t standby_cpus;
>> +        uint16_t reserved_cpus;
>> +        uint8_t name[8];
>> +        uint32_t caf;
>> +        uint8_t cpi[16];
>> +        uint8_t reserved5[3];
>> +        uint8_t ext_name_encoding;
>> +        uint32_t reserved3;
>> +        uint8_t uuid[16];
>> +    } vm[8];
>> +    uint8_t reserved4[1504];
>> +    uint8_t ext_names[8][256];
>> +};
>> +
>> +#endif  /* _S390X_STSI_H_ */
>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>> index a5b92863..3e11401e 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)
> 
> We could add a fc < 3 check to the vm_is_tcg() function and add a 

OK

> vm_is_lpar() which does a simple fc ==1 check.

hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?

Do you have an experimental return on this?

> 
>> @@ -43,3 +44,41 @@ out:
>>       free_page(buf);
>>       return is_tcg;
>>   }
>> +
>> +/**
>> + * 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;
>> +
>> +    if (stsi(stsi_322, 3, 2, 2))
>> +        goto out;
>> +
>> +    /*
>> +     * If the manufacturer string is "KVM/" in EBCDIC, then we
>> +     * are on KVM (otherwise the string is "IBM" in EBCDIC)
>> +     */
>> +    is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, 
>> sizeof(kvm_ebcdic));
> 
> So I had a look at this before Christmas and I think it's wrong.
> 
> QEMU will still set the cpi to KVM/LINUX if we are under tcg.
> So we need to do add a !tcg check here and fix this comment.
> 
> I.e. we always have the KVM/LINUX cpi but if we're under TCG the 
> manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and 
> if we want to fix this at some point or not.

indeed I did not check this!!

> 
>> +    initialized = true;
>> +out:
>> +    free_page(stsi_322);
>> +    return is_kvm;
>> +}

...snip...

Thanks for the review, I make the changes.
Pierre
Pierre Morel Jan. 17, 2022, 3:05 p.m. UTC | #4
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
Janosch Frank Jan. 18, 2022, 8:35 a.m. UTC | #5
On 1/17/22 15:57, Pierre Morel wrote:
> 
> 
> On 1/11/22 13:27, Janosch Frank wrote:
>> On 1/10/22 14:37, 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   | 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
>>> + */
>>> +
>>> +#ifndef _S390X_STSI_H_
>>> +#define _S390X_STSI_H_
>>> +
>>> +struct sysinfo_3_2_2 {
>>> +    uint8_t reserved[31];
>>> +    uint8_t count;
>>> +    struct {
>>> +        uint8_t reserved2[4];
>>> +        uint16_t total_cpus;
>>> +        uint16_t conf_cpus;
>>> +        uint16_t standby_cpus;
>>> +        uint16_t reserved_cpus;
>>> +        uint8_t name[8];
>>> +        uint32_t caf;
>>> +        uint8_t cpi[16];
>>> +        uint8_t reserved5[3];
>>> +        uint8_t ext_name_encoding;
>>> +        uint32_t reserved3;
>>> +        uint8_t uuid[16];
>>> +    } vm[8];
>>> +    uint8_t reserved4[1504];
>>> +    uint8_t ext_names[8][256];
>>> +};
>>> +
>>> +#endif  /* _S390X_STSI_H_ */
>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>>> index a5b92863..3e11401e 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)
>>
>> We could add a fc < 3 check to the vm_is_tcg() function and add a
> 
> OK
> 
>> vm_is_lpar() which does a simple fc ==1 check.
> 
> hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
> do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?
> 

Right
I'll do some tests on the lpar stsi output and have a look what we get back.

> Do you have an experimental return on this?

ENOPARSE

> 
>>
>>> @@ -43,3 +44,41 @@ out:
>>>        free_page(buf);
>>>        return is_tcg;
>>>    }
>>> +
>>> +/**
>>> + * 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;
>>> +
>>> +    if (stsi(stsi_322, 3, 2, 2))
>>> +        goto out;
>>> +
>>> +    /*
>>> +     * If the manufacturer string is "KVM/" in EBCDIC, then we
>>> +     * are on KVM (otherwise the string is "IBM" in EBCDIC)
>>> +     */
>>> +    is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic,
>>> sizeof(kvm_ebcdic));
>>
>> So I had a look at this before Christmas and I think it's wrong.
>>
>> QEMU will still set the cpi to KVM/LINUX if we are under tcg.
>> So we need to do add a !tcg check here and fix this comment.
>>
>> I.e. we always have the KVM/LINUX cpi but if we're under TCG the
>> manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and
>> if we want to fix this at some point or not.
> 
> indeed I did not check this!!
> 
>>
>>> +    initialized = true;
>>> +out:
>>> +    free_page(stsi_322);
>>> +    return is_kvm;
>>> +}
> 
> ...snip...
> 
> Thanks for the review, I make the changes.
> Pierre
>
Pierre Morel Jan. 18, 2022, 5:07 p.m. UTC | #6
On 1/18/22 09:35, Janosch Frank wrote:
> On 1/17/22 15:57, Pierre Morel wrote:
>>
>>
>> On 1/11/22 13:27, Janosch Frank wrote:
>>> On 1/10/22 14:37, 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   | 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
>>>> + */
>>>> +
>>>> +#ifndef _S390X_STSI_H_
>>>> +#define _S390X_STSI_H_
>>>> +
>>>> +struct sysinfo_3_2_2 {
>>>> +    uint8_t reserved[31];
>>>> +    uint8_t count;
>>>> +    struct {
>>>> +        uint8_t reserved2[4];
>>>> +        uint16_t total_cpus;
>>>> +        uint16_t conf_cpus;
>>>> +        uint16_t standby_cpus;
>>>> +        uint16_t reserved_cpus;
>>>> +        uint8_t name[8];
>>>> +        uint32_t caf;
>>>> +        uint8_t cpi[16];
>>>> +        uint8_t reserved5[3];
>>>> +        uint8_t ext_name_encoding;
>>>> +        uint32_t reserved3;
>>>> +        uint8_t uuid[16];
>>>> +    } vm[8];
>>>> +    uint8_t reserved4[1504];
>>>> +    uint8_t ext_names[8][256];
>>>> +};
>>>> +
>>>> +#endif  /* _S390X_STSI_H_ */
>>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>>>> index a5b92863..3e11401e 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)
>>>
>>> We could add a fc < 3 check to the vm_is_tcg() function and add a
>>
>> OK
>>
>>> vm_is_lpar() which does a simple fc ==1 check.
>>
>> hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
>> do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?
>>
> 
> Right
> I'll do some tests on the lpar stsi output and have a look what we get 
> back.
> 
>> Do you have an experimental return on this?
> 
> ENOPARSE

:) you just answered.
I wanted to ask if you did tests which gave you "1" as result.
diff mbox series

Patch

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
+ */
+
+#ifndef _S390X_STSI_H_
+#define _S390X_STSI_H_
+
+struct sysinfo_3_2_2 {
+	uint8_t reserved[31];
+	uint8_t count;
+	struct {
+		uint8_t reserved2[4];
+		uint16_t total_cpus;
+		uint16_t conf_cpus;
+		uint16_t standby_cpus;
+		uint16_t reserved_cpus;
+		uint8_t name[8];
+		uint32_t caf;
+		uint8_t cpi[16];
+		uint8_t reserved5[3];
+		uint8_t ext_name_encoding;
+		uint32_t reserved3;
+		uint8_t uuid[16];
+	} vm[8];
+	uint8_t reserved4[1504];
+	uint8_t ext_names[8][256];
+};
+
+#endif  /* _S390X_STSI_H_ */
diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
index a5b92863..3e11401e 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)
@@ -43,3 +44,41 @@  out:
 	free_page(buf);
 	return is_tcg;
 }
+
+/**
+ * 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;
+
+	if (stsi(stsi_322, 3, 2, 2))
+		goto out;
+
+	/*
+	 * If the manufacturer string is "KVM/" in EBCDIC, then we
+	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
+	 */
+	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));
+	initialized = true;
+out:
+	free_page(stsi_322);
+	return is_kvm;
+}
diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
index 7abba0cc..44097b4a 100644
--- a/lib/s390x/vm.h
+++ b/lib/s390x/vm.h
@@ -9,5 +9,6 @@ 
 #define _S390X_VM_H_
 
 bool vm_is_tcg(void);
+bool vm_is_kvm(void);
 
 #endif  /* _S390X_VM_H_ */
diff --git a/s390x/stsi.c b/s390x/stsi.c
index 391f8849..1ed045e2 100644
--- a/s390x/stsi.c
+++ b/s390x/stsi.c
@@ -13,27 +13,8 @@ 
 #include <asm/asm-offsets.h>
 #include <asm/interrupt.h>
 #include <smp.h>
+#include "stsi.h"
 
-struct stsi_322 {
-	uint8_t reserved[31];
-	uint8_t count;
-	struct {
-		uint8_t reserved2[4];
-		uint16_t total_cpus;
-		uint16_t conf_cpus;
-		uint16_t standby_cpus;
-		uint16_t reserved_cpus;
-		uint8_t name[8];
-		uint32_t caf;
-		uint8_t cpi[16];
-		uint8_t reserved5[3];
-		uint8_t ext_name_encoding;
-		uint32_t reserved3;
-		uint8_t uuid[16];
-	} vm[8];
-	uint8_t reserved4[1504];
-	uint8_t ext_names[8][256];
-};
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 
 static void test_specs(void)
@@ -91,7 +72,7 @@  static void test_3_2_2(void)
 	/* EBCDIC for "KVM/" */
 	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
 	const char vm_name_ext[] = "kvm-unit-test";
-	struct stsi_322 *data = (void *)pagebuf;
+	struct sysinfo_3_2_2 *data = (void *)pagebuf;
 
 	report_prefix_push("3.2.2");