Re: [kvm-unit-tests PATCH 2/8] s390x: diag308: Only test subcode 2 under QEMU

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

On 4/5/22 11:18, Thomas Huth wrote:
> On 05/04/2022 09.52, Janosch Frank wrote:
>> Other hypervisors might implement it and therefore not send a
>> specification exception.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>    s390x/diag308.c | 15 ++++++++++++++-
>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/s390x/diag308.c b/s390x/diag308.c
>> index c9d6c499..9614f9a9 100644
>> --- a/s390x/diag308.c
>> +++ b/s390x/diag308.c
>> @@ -8,6 +8,7 @@
>>    #include <libcflat.h>
>>    #include <asm/asm-offsets.h>
>>    #include <asm/interrupt.h>
>> +#include <hardware.h>
>>    
>>    /* The diagnose calls should be blocked in problem state */
>>    static void test_priv(void)
>> @@ -75,7 +76,7 @@ static void test_subcode6(void)
>>    /* Unsupported subcodes should generate a specification exception */
>>    static void test_unsupported_subcode(void)
>>    {
>> -	int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
>> +	int subcodes[] = { 0x101, 0xffff, 0x10001, -1 };
>>    	int idx;
>>    
>>    	for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
>> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void)
>>    		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>>    		report_prefix_pop();
>>    	}
>> +
>> +	/*
>> +	 * Subcode 2 is not available under QEMU but might be on other
>> +	 * hypervisors.
>> +	 */
>> +	if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) {
> 
> Shouldn't this be rather the other way round instead?
> 
> 	if (detect_host() == HOST_IS_TCG || detect_host() == HOST_IS_KVM)
> 
> ?

The css if checks if we are under QEMU, this one checks if we're not 
under QEMU.

> 
> ... anyway, since you already used a similar if-clause in your first patch,
> it might make sense to add a helper function a la host_is_qemu() to check
> whether we're running on QEMU or not.

Will do

> 
>> +		report_prefix_pushf("0x%04x", 2);
> 
> 	report_prefix_pushf("0x02") ?
> 
>> +		expect_pgm_int();
>> +		asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
>> +		check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>> +		report_prefix_pop();
>> +	}
>>    }
> 
>    Thomas
>