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

[Thomas Huth <thuth@redhat.com>]

On 05/04/2022 11.33, Janosch Frank wrote:
> 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.

but ...

>>
>>> +        expect_pgm_int();
>>> +        asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
>>> +        check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);

... don't we want to check here whether the diag causes a spec exception if 
we are *under* QEMU here?
/me feels confused now.

  Thomas