Re: [kvm-unit-tests PATCH v2 3/5] lib: s390: rename and refactor vm.[ch]

[Janosch Frank <frankja@linux.ibm.com>]

On 3/31/22 18:04, Claudio Imbrenda wrote:
> Refactor and rename vm.[ch] to hardware.[ch]
> 
> * Rename vm.[ch] to hardware.[ch]
> * Consolidate all detection functions into detect_host, which returns
>    what host system the test is running on
> * Rename vm_is_* functions to host_is_*, which are then just wrappers
>    around detect_host
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Nice

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>   s390x/Makefile       |  2 +-
>   lib/s390x/hardware.h | 40 +++++++++++++++++++
>   lib/s390x/vm.h       | 15 --------
>   lib/s390x/hardware.c | 69 +++++++++++++++++++++++++++++++++
>   lib/s390x/vm.c       | 92 --------------------------------------------
>   s390x/cpumodel.c     |  4 +-
>   s390x/mvpg.c         |  4 +-
>   7 files changed, 114 insertions(+), 112 deletions(-)
>   create mode 100644 lib/s390x/hardware.h
>   delete mode 100644 lib/s390x/vm.h
>   create mode 100644 lib/s390x/hardware.c
>   delete mode 100644 lib/s390x/vm.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 53b0fe04..9d3a1fd7 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -78,7 +78,7 @@ cflatobjs += lib/s390x/sclp-console.o
>   cflatobjs += lib/s390x/interrupt.o
>   cflatobjs += lib/s390x/mmu.o
>   cflatobjs += lib/s390x/smp.o
> -cflatobjs += lib/s390x/vm.o
> +cflatobjs += lib/s390x/hardware.o
>   cflatobjs += lib/s390x/css_dump.o
>   cflatobjs += lib/s390x/css_lib.o
>   cflatobjs += lib/s390x/malloc_io.o
> diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h
> new file mode 100644
> index 00000000..e5910ea5
> --- /dev/null
> +++ b/lib/s390x/hardware.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Functions to retrieve information about the host system.
> + *
> + * Copyright (c) 2020 Red Hat Inc
> + * Copyright 2022 IBM Corp.
> + *
> + * Authors:
> + *  Claudio Imbrenda <imbrenda@linux.ibm.com>
> + */
> +
> +#ifndef _S390X_HARDWARE_H_
> +#define _S390X_HARDWARE_H_
> +#include <asm/arch_def.h>
> +
> +enum s390_host {
> +	HOST_IS_UNKNOWN,
> +	HOST_IS_LPAR,
> +	HOST_IS_KVM,
> +	HOST_IS_TCG
> +};
> +
> +enum s390_host detect_host(void);
> +
> +static inline bool host_is_tcg(void)
> +{
> +	return detect_host() == HOST_IS_TCG;
> +}
> +
> +static inline bool host_is_kvm(void)
> +{
> +	return detect_host() == HOST_IS_KVM;
> +}
> +
> +static inline bool host_is_lpar(void)
> +{
> +	return detect_host() == HOST_IS_LPAR;
> +}
> +
> +#endif  /* _S390X_HARDWARE_H_ */
> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
> deleted file mode 100644
> index 4456b48c..00000000
> --- a/lib/s390x/vm.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * Functions to retrieve VM-specific information
> - *
> - * Copyright (c) 2020 Red Hat Inc
> - */
> -
> -#ifndef _S390X_VM_H_
> -#define _S390X_VM_H_
> -
> -bool vm_is_tcg(void);
> -bool vm_is_kvm(void);
> -bool vm_is_lpar(void);
> -
> -#endif  /* _S390X_VM_H_ */
> diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c
> new file mode 100644
> index 00000000..2bcf9c4c
> --- /dev/null
> +++ b/lib/s390x/hardware.c
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Functions to retrieve information about the host system.
> + *
> + * Copyright (c) 2020 Red Hat Inc
> + * Copyright 2022 IBM Corp.
> + *
> + * Authors:
> + *  Thomas Huth <thuth@redhat.com>
> + *  Claudio Imbrenda <imbrenda@linux.ibm.com>
> + */
> +
> +#include <libcflat.h>
> +#include <alloc_page.h>
> +#include <asm/arch_def.h>
> +#include "hardware.h"
> +#include "stsi.h"
> +
> +/* The string "QEMU" in EBCDIC */
> +static const uint8_t qemu_ebcdic[] = { 0xd8, 0xc5, 0xd4, 0xe4 };
> +/* The string "KVM/" in EBCDIC */
> +static const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
> +
> +static enum s390_host do_detect_host(void *buf)
> +{
> +	struct sysinfo_3_2_2 *stsi_322 = buf;
> +
> +	if (stsi_get_fc() == 2)
> +		return HOST_IS_LPAR;
> +
> +	if (stsi_get_fc() != 3)
> +		return HOST_IS_UNKNOWN;
> +
> +	if (!stsi(buf, 1, 1, 1)) {
> +		/*
> +		 * If the manufacturer string is "QEMU" in EBCDIC, then we
> +		 * are on TCG (otherwise the string is "IBM" in EBCDIC)
> +		 */
> +		if (!memcmp((char *)buf + 32, qemu_ebcdic, sizeof(qemu_ebcdic)))
> +			return HOST_IS_TCG;
> +	}
> +
> +	if (!stsi(buf, 3, 2, 2)) {
> +		/*
> +		 * If the manufacturer string is "KVM/" in EBCDIC, then we
> +		 * are on KVM.
> +		 */
> +		if (!memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)))
> +			return HOST_IS_KVM;
> +	}
> +
> +	return HOST_IS_UNKNOWN;
> +}
> +
> +enum s390_host detect_host(void)
> +{
> +	static enum s390_host host = HOST_IS_UNKNOWN;
> +	static bool initialized = false;
> +	void *buf;
> +
> +	if (initialized)
> +		return host;
> +
> +	buf = alloc_page();
> +	host = do_detect_host(buf);
> +	free_page(buf);
> +	initialized = true;
> +	return host;
> +}
> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
> deleted file mode 100644
> index 33fb1c45..00000000
> --- a/lib/s390x/vm.c
> +++ /dev/null
> @@ -1,92 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * Functions to retrieve VM-specific information
> - *
> - * Copyright (c) 2020 Red Hat Inc
> - *
> - * Authors:
> - *  Thomas Huth <thuth@redhat.com>
> - */
> -
> -#include <libcflat.h>
> -#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)
> - */
> -bool vm_is_tcg(void)
> -{
> -	const char qemu_ebcdic[] = { 0xd8, 0xc5, 0xd4, 0xe4 };
> -	static bool initialized = false;
> -	static bool is_tcg = false;
> -	uint8_t *buf;
> -
> -	if (initialized)
> -		return is_tcg;
> -
> -	if (stsi_get_fc() != 3) {
> -		initialized = true;
> -		return is_tcg;
> -	}
> -
> -	buf = alloc_page();
> -	assert(buf);
> -
> -	if (stsi(buf, 1, 1, 1))
> -		goto out;
> -
> -	/*
> -	 * If the manufacturer string is "QEMU" in EBCDIC, then we
> -	 * are on TCG (otherwise the string is "IBM" in EBCDIC)
> -	 */
> -	is_tcg = !memcmp(&buf[32], qemu_ebcdic, sizeof(qemu_ebcdic));
> -	initialized = true;
> -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 || vm_is_tcg()) {
> -		initialized = true;
> -		return is_kvm;
> -	}
> -
> -	stsi_322 = alloc_page();
> -	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.
> -	 */
> -	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));
> -	initialized = true;
> -out:
> -	free_page(stsi_322);
> -	return is_kvm;
> -}
> -
> -bool vm_is_lpar(void)
> -{
> -	return stsi_get_fc() == 2;
> -}
> -
> diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c
> index 23ccf842..5c0b73e0 100644
> --- a/s390x/cpumodel.c
> +++ b/s390x/cpumodel.c
> @@ -10,7 +10,7 @@
>    */
>   
>   #include <asm/facility.h>
> -#include <vm.h>
> +#include <hardware.h>
>   #include <sclp.h>
>   #include <uv.h>
>   #include <asm/uv.h>
> @@ -118,7 +118,7 @@ int main(void)
>   	for (i = 0; i < ARRAY_SIZE(dep); i++) {
>   		report_prefix_pushf("%d implies %d", dep[i].facility, dep[i].implied);
>   		if (test_facility(dep[i].facility)) {
> -			report_xfail(dep[i].expected_tcg_fail && vm_is_tcg(),
> +			report_xfail(dep[i].expected_tcg_fail && host_is_tcg(),
>   				     test_facility(dep[i].implied),
>   				     "implication not correct");
>   		} else {
> diff --git a/s390x/mvpg.c b/s390x/mvpg.c
> index 2b7c6cc9..62f0fc5a 100644
> --- a/s390x/mvpg.c
> +++ b/s390x/mvpg.c
> @@ -20,7 +20,7 @@
>   #include <smp.h>
>   #include <alloc_page.h>
>   #include <bitops.h>
> -#include <vm.h>
> +#include <hardware.h>
>   
>   /* Used to build the appropriate test values for register 0 */
>   #define KFC(x) ((x) << 10)
> @@ -251,7 +251,7 @@ static void test_mmu_prot(void)
>   	fresh += PAGE_SIZE;
>   
>   	/* Known issue in TCG: CCO flag is not honoured */
> -	if (vm_is_tcg()) {
> +	if (host_is_tcg()) {
>   		report_prefix_push("TCG");
>   		report_skip("destination invalid");
>   		report_skip("source invalid");