[PATCH 00/7 Add support for running in VM guests to intel_idle

From: Arjan van de Ven <[email protected]>

Now that the logic for state_update_enter_method() is in its own function, the long if .. else if .. else if .. else if chain can be simplified by just returning from the function at the various places. This does not change functionality, but it makes the logic much simpler to read or modify later.

Signed-off-by Arjan van de Ven <[email protected]>

---
 drivers/idle/intel_idle.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/idle/intel_idle.c +12 -4

index c351b21c0875..256c2d42e350 100644
@@ -1849,7 +1849,10 @@ static void state_update_enter_method(struct cpuidle_state *state, int cstate)
                WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IBRS);
                WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
                state->enter = intel_idle_xstate;
-       } else if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
+               return;
+       }
+
+       if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
                           state->flags & CPUIDLE_FLAG_IBRS) {
                /*
                 * IBRS mitigation requires that C-states are entered

@@ -1857,9 +1860,15 @@ static void state_update_enter_method(struct cpuidle_state *state, int cstate)
                 */
                WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
                state->enter = intel_idle_ibrs;
-       } else if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
+               return;
+       }
+
+       if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
                state->enter = intel_idle_irq;
-       } else if (force_irq_on) {
+               return;
+       }
+
+       if (force_irq_on) {
                pr_info("forced intel_idle_irq for state %d\n", cstate);
                state->enter = intel_idle_irq;
        }
--

2.40.1

This and the [1/7] can be applied without the rest of the series, so please let me know if you want me to do that.

From: Arjan van de Ven <[email protected]>

The state_update_enter_method function updates a state's enter function pointer, but does so assuming that the current function is "intel_idle" or "intel_idle_irq".

In the code currently that's basically the case, but soon this will change. Add a sanity check early in the function to make the assumption explicit, and return early if the precondition is not met.

Signed-off-by Arjan van de Ven <[email protected]>

---
 drivers/idle/intel_idle.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/idle/intel_idle.c +0 -0

index 256c2d42e350..8415965372c7 100644

drivers/idle/intel_idle.c +1 -1

+
        if (state->flags & CPUIDLE_FLAG_INIT_XSTATE) {
                /*
                 * Combining with XSTATE with IBRS or IRQ_ENABLE flags
--

2.40.1

From: Arjan van de Ven <[email protected]>

Currently the intel_idle driver has a family of functions called intel_idle/intel_idle_irq/intel_idle_xsave/... that use the mwait instruction to enter into a low power state.

x86 cpus can also use the legacy "hlt" instruction for this, and in some cases (VM guests for example) the mwait instruction might not be available.

Because of this, add the basic helpers to allow 'hlt' to be used to enter a low power state (will be used in later patches), both in the regular and the _irq enabled variant.

Signed-off-by Arjan van de Ven <[email protected]>

---
 drivers/idle/intel_idle.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

drivers/idle/intel_idle.c +37 -1

index 8415965372c7..66d262fd267e 100644
@@ -199,6 +199,43 @@ static __cpuidle int intel_idle_xstate(struct cpuidle_device *dev,
        return __intel_idle(dev, drv, index);
 }
 
+static __always_inline int __intel_idle_hlt(struct cpuidle_device *dev,
+                                       struct cpuidle_driver *drv, int index)
+{
+       raw_safe_halt();
+       raw_local_irq_disable();
+       return index;
+}
+
+/**
+ * intel_idle_hlt - Ask the processor to enter the given idle state using hlt.
+ * @dev: cpuidle device of the target CPU.
+ * @drv: cpuidle driver (assumed to point to intel_idle_driver).
+ * @index: Target idle state index.
+ *
+ * Use the HLT instruction to notify the processor that the CPU represented by
+ * @dev is idle and it can try to enter the idle state corresponding to @index.
+ *
+ * Must be called under local_irq_disable().
+ */
+static __cpuidle int intel_idle_hlt(struct cpuidle_device *dev,
+                               struct cpuidle_driver *drv, int index)
+{
+       return __intel_idle_hlt(dev, drv, index);
+}
+
+static __cpuidle int intel_idle_hlt_irq(struct cpuidle_device *dev,
+                                   struct cpuidle_driver *drv, int index)
+{
+       int ret;
+
+       raw_local_irq_enable();
+       ret = __intel_idle_hlt(dev, drv, index);
+       raw_local_irq_disable();
+
+       return ret;
+}
+
 /**
  * intel_idle_s2idle - Ask the processor to enter the given idle state.
  * @dev: cpuidle device of the target CPU.
--

2.40.1

From: Arjan van de Ven <[email protected]>

Currently, intel_idle verifies that the cpuid instruction enumerates that the mwait value for a state is actually supported by the CPU.

Going forward, when running in a VM guest, that check will not work and we're going to need a way to turn it off.

Add a global bool for this, and uses this in the check function to short circuit this cpuid check.

Signed-off-by Arjan van de Ven <[email protected]>

---
 drivers/idle/intel_idle.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/idle/intel_idle.c +1 -0

index 66d262fd267e..55c3e6ece3dd 100644
@@ -69,6 +69,7 @@ static int max_cstate = CPUIDLE_STATE_MAX - 1;
 static unsigned int disabled_states_mask __read_mostly;
 static unsigned int preferred_states_mask __read_mostly;
 static bool force_irq_on __read_mostly;
+static bool skip_mwait_check __read_mostly;
 
 static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
 

drivers/idle/intel_idle.c +0 -1

        /* Ignore the C-state if there are NO sub-states in CPUID for it. */
        if (num_substates == 0)
                return false;
--

I don't think that the static variable is needed here.

This function is only called from intel_idle_init_cstates_icpu() which in turn is only called by intel_idle_cpuidle_driver_init().

The latter is called directly by intel_idle_vminit() in the next patch and so it can be passed a bool arg indicating whether or not to skip the mwait checks.

From: Arjan van de Ven <[email protected]>

In a typical VM guest, the mwait instruction is not available, leaving only the 'hlt' instruction (which causes a VMEXIT to the host).

So currently, intel_idle will detect the lack of mwait, and fail to initialize (after which another idle method would step in which will just use hlt always).

By providing capability to do this with the intel_idle driver, we can do better than this fallback. While this current change only gets us parity to the existing behavior, later patches in this series will add new capabilities.

In order to do this, a simplified version of the initialization function for VM guests is created, and this will be called if the CPU is recognized, but mwait is not supported, and we're in a VM guest.

One thing to note is that the latency (and break even) of this C1 state is higher than the typical bare metal C1 state. Because hlt causes a vmexit, and the cost of vmexit + hypervisor overhead + vmenter is typically in the order of upto 5 microseconds... even if the hypervisor does not actually goes into a hardware power saving state.

Signed-off-by Arjan van de Ven <[email protected]>

---
 drivers/idle/intel_idle.c | 54 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

drivers/idle/intel_idle.c +54 -1

index 55c3e6ece3dd..c4929d8a35a4 100644
@@ -1280,6 +1280,18 @@ static struct cpuidle_state snr_cstates[] __initdata = {
                .enter = NULL }
 };
 
+static struct cpuidle_state vmguest_cstates[] __initdata = {
+       {
+               .name = "C1",
+               .desc = "HLT",
+               .flags = MWAIT2flg(0x00) | CPUIDLE_FLAG_IRQ_ENABLE,
+               .exit_latency = 5,
+               .target_residency = 10,
+               .enter = &intel_idle_hlt_irq, },
+       {
+               .enter = NULL }
+};
+
 static const struct idle_cpu idle_cpu_nehalem __initconst = {
        .state_table = nehalem_cstates,
        .auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,

@@ -2105,6 +2117,46 @@ static void __init intel_idle_cpuidle_devices_uninit(void)
                cpuidle_unregister_device(per_cpu_ptr(intel_idle_cpuidle_devices, i));
 }
 
+static int __init intel_idle_vminit(const struct x86_cpu_id *id)
+{
+       int retval;
+
+       cpuidle_state_table = vmguest_cstates;
+       skip_mwait_check = true; /* hypervisor hides mwait from us normally */
+
+       icpu = (const struct idle_cpu *)id->driver_data;
+
+       pr_debug("v" INTEL_IDLE_VERSION " model 0x%X\n",
+                boot_cpu_data.x86_model);
+
+       intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+       if (!intel_idle_cpuidle_devices)
+               return -ENOMEM;
+
+       intel_idle_cpuidle_driver_init(&intel_idle_driver);
+
+       retval = cpuidle_register_driver(&intel_idle_driver);
+       if (retval) {
+               struct cpuidle_driver *drv = cpuidle_get_driver();
+               printk(KERN_DEBUG pr_fmt("intel_idle yielding to %s\n"),
+                      drv ? drv->name : "none");
+               goto init_driver_fail;
+       }
+
+       retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
+                                  intel_idle_cpu_online, NULL);
+       if (retval < 0)
+               goto hp_setup_fail;
+
+       return 0;
+hp_setup_fail:
+       intel_idle_cpuidle_devices_uninit();
+       cpuidle_unregister_driver(&intel_idle_driver);
+init_driver_fail:
+       free_percpu(intel_idle_cpuidle_devices);
+       return retval;
+}
+
 static int __init intel_idle_init(void)
 {
        const struct x86_cpu_id *id;

@@ -2123,6 +2175,8 @@ static int __init intel_idle_init(void)
        id = x86_match_cpu(intel_idle_ids);
        if (id) {
                if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
+                       if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+                               return intel_idle_vminit(id);
                        pr_debug("Please enable MWAIT in BIOS SETUP\n");
                        return -ENODEV;
                }
--

2.40.1

Quoted 5 lines

From: Arjan van de Ven <[email protected]>

In a typical VM guest, the mwait instruction is not available, leaving only the 'hlt' instruction (which causes a VMEXIT to the host).

What if it is available? It can be AFAICS.

Quoted 3 lines

In order to do this, a simplified version of the initialization function for VM guests is created, and this will be called if the CPU is recognized, but mwait is not supported, and we're in a VM guest.

It will cause intel_idle to become the idle driver in some cases in which ACPI idle is used nowadays if I'm not mistaken. Wouldn't that be regarded as a problem?

From: Arjan van de Ven <[email protected]>

intel_idle provides the CPU Idle states (for power saving in idle) to the cpuidle framework, based on per-cpu tables combined with limited hardware enumeration. This combination of cpuidle and intel_idle provides dynamic behavior where power saving and performance impact are dynamically balanced and where a set of generic knobs are provided in sysfs for users to tune the heuristics (and get statistics etc)

However, intel_idle currently does not support running inside VM guests, and the linux kernel falls back to either ACPI based idle (if supported by the hypervisor/virtual bios) or just the default x86 fallback "hlt" based idle method... that was introduced in the 1.2 kernel series... and lacks all the dynamic behavior, user control and statistics that cpuidle brings.

While this is obviously functional, it's not great and we can do better for the user by hooking up intel_idle into the cpuidle framework also for the "in a guest" case.
And not only not great for the user, it's also not optimal and lacks two key capabilities that are supported by the bare metal case:

1) The ability to flush the TLB for very long idle periods, to avoid a costly (and high latency) IPI wakeup later, of an idle vCPU when a process that used to run on the idle vCPU does an munmap or similar operation. Avoiding high latency IPIs helps avoid performance jitter.
2) The ability to use the new Intel C0.2 idle state instead of polling for very short duration idle periods to save power (and carbon footprint)

This patch series adds the basic support to run in a VM guest to the intel_idle driver, and then addresses the first of these shortfalls.

The C0.2 gap will be fixed with a small additional patch after the C0.2 support is merged seperately.

Arjan van de Ven (7):
intel_idle: refactor state->enter manipulation into its own function
intel_idle: clean up the (new) state_update_enter_method function
intel_idle: Add a sanity check in the new state_update_enter_method
function
intel_idle: Add helper functions to support 'hlt' as idle state
intel_idle: Add a way to skip the mwait check on all states
intel_idle: Add support for using intel_idle in a VM guest using just
hlt
intel_idle: Add a "Long HLT" C1 state for the VM guest mode

 drivers/idle/intel_idle.c | 216 ++++++++++++++++++++++++++++++++++----
 1 file changed, 194 insertions(+), 22 deletions(-)

--