Re: [kvm-unit-tests PATCH v6 08/10] s390x: define wfi: wait for interrupt

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

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

On 4/28/20 10:44 AM, Pierre Morel wrote:
> 
> 
> On 2020-04-27 14:59, Janosch Frank wrote:
>> On 4/24/20 12:45 PM, Pierre Morel wrote:
>>> wfi(irq_mask) allows the programm to wait for an interrupt.
>>
>> s/programm/program/
> 
> Thx,
> 
>>
>>> The interrupt handler is in charge to remove the WAIT bit
>>> when it finished handling interrupt.
>>
>> ...finished handling the interrupt.
> 
> OK, thx
> 
>>
> 
>>>   }
>>>   
>>> +static inline void wfi(uint64_t irq_mask)
>>
>> enabled_wait()
> 
> 
> I do not like enabled_wait(), we do not know what is enabled and we do 
> not know what we are waiting for.
> 
> What about wait_for_interrupt()

As long as it's not called wfi...

> 
>>
>>> +{
>>> +	uint64_t psw_mask;
>>
>> You can directly initialize this variable.
>>
>>> +
>>> +	psw_mask = extract_psw_mask();
>>> +	load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT);
>>
>> Maybe add a comment here:
>>
>> /*
>>   * After being woken and having processed the interrupt, let's restore
>> the PSW mask.
>> */
>>
>>> +	load_psw_mask(psw_mask);
>>> +}
>>> +
> 
> I can do this, but wasn't it obvious?

It took me a minute, so it will take even longer for developers that are
not yet familiar with s390 kernel development.

> 
> 
> Regards,
> Pierre
> 



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