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:50, Thomas Huth wrote:
> 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.

Seems like it's me who's confused today
Anyway, I'll clean that up.

> 
>    Thomas
>