Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test

[Thomas Huth <thuth@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --]

On 8/20/19 5:21 PM, Janosch Frank wrote:
> On 8/20/19 2:55 PM, Thomas Huth wrote:
>> On 8/20/19 2:25 PM, Janosch Frank wrote:
>>> On 8/20/19 1:59 PM, Thomas Huth wrote:
>>>> On 8/20/19 12:55 PM, Janosch Frank wrote:
> [...]
>>>> ... maybe we could also introduce such a variable as a global variable
>>>> in lib/s390x/ since this is already the third or fourth time that we use
>>>> it in the kvm-unit-tests...
>>>
>>> Sure I also thought about that, any particular place?
>>
>> No clue. Maybe lib/s390x/mmu.c ? Or a new file called lowcore.c ?
>>
>>>>> +static inline void diag288_uneven(void)
>>>>> +{
>>>>> +	register unsigned long fc asm("1") = 0;
>>>>> +	register unsigned long time asm("1") = 15;
>>>>
>>>> So you're setting register 1 twice? And "time" is not really used in the
>>>> inline assembly below? How's that supposed to work? Looks like a bug to
>>>> me... if not, please explain with a comment in the code here.
>>>
>>> Well I'm waiting for a spec exception here, so it doesn't have to work.> I'll probably just remove the register variables and do a:
>>>
>>> "diag %r1,%r2,0x288"
>>
>> Yes, I think that's easier to understand.
>>
>> BTW, is there another documentation of diag 288 beside the "CP
>> programming services" manual? At least my version of that specification
>> does not say that the fc register has to be even...
> 
> I used the non-public lpar documentation...

Ok, if it's specified there, then the check is fine with me.

>>>>> +static void test_bite(void)
>>>>> +{
>>>>> +	if (lc->restart_old_psw.addr) {
>>>>> +		report("restart", true);
>>>>> +		return;
>>>>> +	}
>>>>> +	lc->restart_new_psw.addr = (uint64_t)test_bite;
>>>>> +	diag288(CODE_INIT, 15, ACTION_RESTART);
>>>>> +	while(1) {};
>>>>
>>>> Should this maybe timeout after a minute or so?
>>>
>>> Well run_tests.sh does timeout externally.
>>> Do you need it backed into the test?
>>
>> I sometimes also run the tests without the wrapper script, so in that
>> case it would be convenient ... but I can also quit QEMU manually in
>> that case, so it's not a big issue.
> 
> How about setting the clock comparator, that should trigger an
> unexpected external interrupt?

Sounds like an idea (if this is not getting too complicated... otherwise
just leave it as it is).

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]