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

Janosch Frank
2020-04-28T09:20:11Z

[-- 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 --]