Re: [kvm-unit-tests PATCH v1 2/2] s390x: firq: floating interrupt test

[David Hildenbrand <david@redhat.com>]

On 02.12.21 13:07, Claudio Imbrenda wrote:
> On Thu, 2 Dec 2021 12:13:08 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>>>> +static void wait_for_sclp_int(void)
>>>> +{
>>>> +	/* Enable SCLP interrupts on this CPU only. */
>>>> +	ctl_set_bit(0, CTL0_SERVICE_SIGNAL);
>>>> +
>>>> +	set_flag(1);  
>>>
>>> why not just WRITE_ONCE/READ_ONCE?  
>>
>> Because I shamelessly copied that from s390x/smp.c ;)
>>
>>>> +	set_flag(0);
>>>> +
>>>> +	/* Start CPU #1 and let it wait for the interrupt. */
>>>> +	psw.mask = extract_psw_mask();
>>>> +	psw.addr = (unsigned long)wait_for_sclp_int;
>>>> +	ret = smp_cpu_setup(1, psw);
>>>> +	if (ret) {
>>>> +		report_skip("cpu #1 not found");  
>>>
>>> ...which means that this will hang, and so will all the other report*
>>> functions. maybe you should manually unset the flag before calling the
>>> various report* functions.  
>>
>> Good point, thanks!
>>
>>>   
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	/* Wait until the CPU #1 at least enabled SCLP interrupts. */
>>>> +	wait_for_flag();
>>>> +
>>>> +	/*
>>>> +	 * We'd have to jump trough some hoops to sense e.g., via SIGP
>>>> +	 * CONDITIONAL EMERGENCY SIGNAL if CPU #1 is already in the
>>>> +	 * wait state.
>>>> +	 *
>>>> +	 * Although not completely reliable, use SIGP SENSE RUNNING STATUS
>>>> +	 * until not reported as running -- after all, our SCLP processing
>>>> +	 * will take some time as well and make races very rare.
>>>> +	 */
>>>> +	while(smp_sense_running_status(1));
> 
> if you wait here for CPU1 to be in wait state, then why did you need to
> wait until it has set the flag earlier? can't you just wait here and not
> use the whole wait_for_flag logic? smp_cpu_setup only returns after the
> new CPU has started running.

I use the flag right now as a mechanism to make the race window as short
as possible. But you're right, we might not need the flag logic as
knowing that we processed the setup part and will jump/jumped to the
target code might be good enough.

Thanks!

-- 
Thanks,

David / dhildenb