Re: [kvm-unit-tests PATCH v3 4/7] lib: s390x: uv: Add pv guest requirement check function

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Fri, 21 Apr 2023 11:36:44 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> When running PV guests some of the UV memory needs to be allocated
> with > 31 bit addresses which means tests with PV guests will always
> need a lot more memory than other tests.
> Additionally facilities nr 158 and sclp.sief2 need to be available.
> 
> Let's add a function that checks for these requirements and prints a
> helpful skip message.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/snippet.h |  7 +++++++
>  lib/s390x/uv.c      | 20 ++++++++++++++++++++
>  lib/s390x/uv.h      |  1 +
>  s390x/pv-diags.c    |  8 +-------
>  4 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
> index 57045994..11ec54c3 100644
> --- a/lib/s390x/snippet.h
> +++ b/lib/s390x/snippet.h
> @@ -30,6 +30,13 @@
>  #define SNIPPET_HDR_LEN(type, file) \
>  	((uintptr_t)SNIPPET_HDR_END(type, file) - (uintptr_t)SNIPPET_HDR_START(type, file))
>  
> +/*
> + * Some of the UV memory needs to be allocated with >31 bit
> + * addresses which means we need a lot more memory than other
> + * tests.
> + */
> +#define SNIPPET_PV_MIN_MEM_SIZE	(SZ_1M * 2200UL)
> +
>  #define SNIPPET_PV_TWEAK0	0x42UL
>  #define SNIPPET_PV_TWEAK1	0UL
>  #define SNIPPET_UNPACK_OFF	0
> diff --git a/lib/s390x/uv.c b/lib/s390x/uv.c
> index 383271a5..db47536c 100644
> --- a/lib/s390x/uv.c
> +++ b/lib/s390x/uv.c
> @@ -18,6 +18,7 @@
>  #include <asm/uv.h>
>  #include <uv.h>
>  #include <sie.h>
> +#include <snippet.h>
>  
>  static struct uv_cb_qui uvcb_qui = {
>  	.header.cmd = UVC_CMD_QUI,
> @@ -38,6 +39,25 @@ bool uv_os_is_host(void)
>  	return test_facility(158) && uv_query_test_call(BIT_UVC_CMD_INIT_UV);
>  }
>  
> +bool uv_guest_requirement_checks(void)

I would call it uv_host_requirement_checks since it will run on the
host to check if the host meets certain requirements

> +{
> +	if (!test_facility(158)) {
> +		report_skip("UV Call facility unavailable");
> +		return false;
> +	}
> +	if (!sclp_facilities.has_sief2) {
> +		report_skip("SIEF2 facility unavailable");
> +		return false;
> +	}
> +	if (get_ram_size() < SNIPPET_PV_MIN_MEM_SIZE) {
> +		report_skip("Not enough memory. This test needs about %ld MB of memory",
> +			    SNIPPET_PV_MIN_MEM_SIZE / 1024 / 1024);

a better way to do this would be to check the amount of memory needed
by the Ultravisor and check if that size + 2GB is available

of course in that case unittest.cfg would also need to be adjusted

> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  bool uv_query_test_call(unsigned int nr)
>  {
>  	/* Query needs to be called first */
> diff --git a/lib/s390x/uv.h b/lib/s390x/uv.h
> index 78b979b7..d9af691a 100644
> --- a/lib/s390x/uv.h
> +++ b/lib/s390x/uv.h
> @@ -7,6 +7,7 @@
>  
>  bool uv_os_is_guest(void);
>  bool uv_os_is_host(void);
> +bool uv_guest_requirement_checks(void);
>  bool uv_query_test_call(unsigned int nr);
>  const struct uv_cb_qui *uv_get_query_data(void);
>  void uv_init(void);
> diff --git a/s390x/pv-diags.c b/s390x/pv-diags.c
> index fa4e5532..1289a571 100644
> --- a/s390x/pv-diags.c
> +++ b/s390x/pv-diags.c
> @@ -149,14 +149,8 @@ static void test_diag_yield(void)
>  int main(void)
>  {
>  	report_prefix_push("pv-diags");
> -	if (!test_facility(158)) {
> -		report_skip("UV Call facility unavailable");
> +	if (!uv_guest_requirement_checks())
>  		goto done;
> -	}
> -	if (!sclp_facilities.has_sief2) {
> -		report_skip("SIEF2 facility unavailable");
> -		goto done;
> -	}
>  
>  	uv_setup_asces();
>  	snippet_setup_guest(&vm, true);