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

Janosch Frank
2022-01-11T12:27:16Z

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");
>
>