[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.  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>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mwait-idle.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index 9c16cc166a14..5c16f5ad3a82 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -875,7 +875,6 @@ static const struct cpuidle_state snr_cstates[] = {
 static void cf_check mwait_idle(void)
 {
        unsigned int cpu = smp_processor_id();
-       struct cpu_info *info = get_cpu_info();
        struct acpi_processor_power *power = processor_powers[cpu];
        struct acpi_processor_cx *cx = NULL;
        unsigned int next_state;
@@ -902,6 +901,8 @@ 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);
@@ -928,11 +929,6 @@ static void cf_check mwait_idle(void)
        if ((cx->type >= 3) && errata_c6_workaround())
                cx = power->safe_state;
 
-       if (cx->ibrs_disable) {
-               ASSERT(!cx->irq_enable_early);
-               spec_ctrl_enter_idle(info);
-       }
-
 #if 0 /* XXX Can we/do we need to do something similar on Xen? */
        /*
         * leave_mm() to avoid costly and often unnecessary wakeups
@@ -964,10 +960,6 @@ static void cf_check mwait_idle(void)
 
        /* Now back in C0. */
        update_idle_stats(power, cx, before, after);
-
-       if (cx->ibrs_disable)
-               spec_ctrl_exit_idle(info);
-
        local_irq_enable();
 
        TRACE_TIME(TRC_PM_IDLE_EXIT, cx->type, after,
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.