Re: [PATCH 3/4] Revert part of "x86/mwait-idle: disable IBRS during long idle"

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/06/2025 10:58 am, Jan Beulich wrote:
> On 24.06.2025 18:39, Andrew Cooper wrote:
>> Most of the patch (handling of CPUIDLE_FLAG_IBRS) is fine, but the
>> adjustements to mwait_idle() are not.
>>
>> spec_ctrl_{enter,exit}_idle() do more than just alter MSR_SPEC_CTRL.IBRS.  
>> The
>> VERW and RSB stuff are **unsafe** to omit.
>>
>> The only reason this doesn't need an XSA is because no changes were made to
>> the lower level mwait_idle_with_hints(), and thus it remained properly
>> protected.
>>
>> I.e. This change only served to double the expensive operations in the case 
>> it
>> was trying to optimise.
>>
>> I have an idea of how to plumb this more nicely, but it requires larger
>> changes to legacy IBRS handling to not make spec_ctrl_enter_idle() vulnerable
>> in other ways.
> What are the concerns here? As it looks skipping the MSR write would look
> to require checking some (per-CPU) conditional. Conditional branches can't
> really be of concern, or the "if (cx->ibrs_disable)" that you're now
> removing again would have been of concern, too.

The conditional branches are what set off alarm bells in the first place.

A conditional branch in enter should be ok; HLT and MWAIT should be
serialising enough.

A conditional branch in exit is not ok without extra safety measures.

I can expand on this in the commit message if you'd like.  I was trying
to not be overly critical...

>  Hence simply a new SCF_
> flag would look to be sufficient, for mwait_idle() to convey the necessary
> info to spec_ctrl_enter_idle()?

I've got a local patch going that route, but it needs more than just an
SCF flag.  This is the "requires larger changes".

>
>>  In the short term, simply take out the perf hit.
>>
>> Fixes: 08acdf9a2615 ("x86/mwait-idle: disable IBRS during long idle")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>

Thanks.

~Andrew