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. Hence simply a new SCF_
flag would look to be sufficient, for mwait_idle() to convey the necessary
info to spec_ctrl_enter_idle()?
> 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.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
