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

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

On 4/21/23 16:13, Claudio Imbrenda wrote:
> 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

Sure
If someone has a shorter suggestion I'd also be happy to hear it.

> 
>> +{
>> +	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

Could do, but in this case I opted for simplicity.