On 03.07.2025 14:10, Andrew Cooper wrote:
> On 03/07/2025 10:43 am, Jan Beulich wrote:
>> On 02.07.2025 16:41, Andrew Cooper wrote:
>>> @@ -461,12 +468,19 @@ void mwait_idle_with_hints(unsigned int eax, unsigned
>>> int ecx)
>>>
>>> monitor(this_softirq_pending, 0, 0);
>>>
>>> + ASSERT(!local_irq_is_enabled());
>>> +
>>> if ( !*this_softirq_pending )
>>> {
>>> struct cpu_info *info = get_cpu_info();
>>>
>>> spec_ctrl_enter_idle(info);
>>> - mwait(eax, ecx);
>>> +
>>> + if ( ecx & MWAIT_ECX_INTERRUPT_BREAK )
>>> + mwait(eax, ecx);
>>> + else
>>> + sti_mwait_cli(eax, ecx);
>> Actually, I'm curious: It seems quite likely that you did consider an
>> alternative resulting in assembly code like this:
>>
>> test $MWAIT_ECX_INTERRUPT_BREAK, %cl
>> jz 0f
>> sti
>> 0:
>> monitor
>> cli
>>
>> CLI being a relatively cheap operation aiui, is there anything wrong or
>> undesirable with this (smaller overall) alternative?
>
> Other than it needing to be mwait?
Oops.
> The overheads aren't interesting;
> they're nothing compared to going idle.
>
> What does matter is that such a transformation cannot exist in mwait()
> itself, as that breaks acpi_dead_idle(), and if we turn this mwait()
> into inline asm, there's only a single caller of mwait() left.
I was rather think of it living in something derived from sti_mwait_cli(),
which could then be called unconditionally from mwait_idle_with_hints().
Jan
