On 13.10.2022 14:03, Roger Pau Monné wrote:
> On Thu, Aug 18, 2022 at 03:04:51PM +0200, Jan Beulich wrote:
>> From: Peter Zijlstra <[email protected]>
>>
>> Having IBRS enabled while the SMT sibling is idle unnecessarily slows
>> down the running sibling. OTOH, disabling IBRS around idle takes two
>> MSR writes, which will increase the idle latency.
>>
>> Therefore, only disable IBRS around deeper idle states. Shallow idle
>> states are bounded by the tick in duration, since NOHZ is not allowed
>> for them by virtue of their short target residency.
>>
>> Only do this for mwait-driven idle, since that keeps interrupts disabled
>> across idle, which makes disabling IBRS vs IRQ-entry a non-issue.
>>
>> Note: C6 is a random threshold, most importantly C1 probably shouldn't
>> disable IBRS, benchmarking needed.
>>
>> Suggested-by: Tim Chen <[email protected]>
>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>> Signed-off-by: Borislav Petkov <[email protected]>
>> Reviewed-by: Josh Poimboeuf <[email protected]>
>> Signed-off-by: Borislav Petkov <[email protected]>
>> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> bf5835bcdb96
>> Signed-off-by: Jan Beulich <[email protected]>
>
> Acked-by: Roger Pau Monné <[email protected]>
Thanks.
> One unrelated comment below.
> [...]
>> @@ -932,8 +939,6 @@ static void cf_check mwait_idle(void)
>> pm_idle_save();
>> else
>> {
>> - struct cpu_info *info = get_cpu_info();
>> -
>> spec_ctrl_enter_idle(info);
>> safe_halt();
>> spec_ctrl_exit_idle(info);
>
> Do we need to disable speculation just for the hlt if there's no
> C state change?
>
> It would seem to me like the MSR writes could add a lot of latency
> when there's no C state change.
HLT enters (at least) C1, so is a C-state change to me as well. Plus
we may remain there for a while, and during that time we'd like to
not unduly impact the other thread.
Jan