Re: [PATCH RFC v3 2/3] KVM: x86: add KVM_CAP_CSTATE_POLICY for per-VM C-state enforcement

[Anthony Harivel]

> Sorry, NAK, this doesn't belong in KVM.  Given that the only way this can work
> is if vCPU are pinned 1:1 to pCPUs, then it should be very doable for the cpuidle
> subystem to provide an interface to let (privileged?) userspace restrict the
> maximum C-state on a per-CPU basis.
>
> My apologies for not responding to v1 or v2, I am guilty of Jim's axiom that
> upstream doesn't respond to RFCs without code.
>

Thanks for the review Sean, and no worries about v1/v2, I understand
the "no code, no review" reality.

I agree this belongs in cpuidle rather than KVM. A per-CPU userspace
interface is the right level of abstraction; it works for our pinned
vCPU use case and is useful beyond KVM (DPDK, bare-metal NFV, etc.).


> +      Option B: Use forced_idle_latency_limit_ns on the pCPU.
> +      Same per-CPU limitation, and latency-based rather than
> +      state-index-based, less precise.
>
> Conceptually, (b) seems like the right approach.  Per-task will be a mess because
> similar to a KVM-based interface, it can probably only work if tasks are pinned
> to pCPUs.
>
> And isn't abstracting away the exact C-state via forced_idle_latency_limit_ns a
> *good* thing?  Without that, userspace will need to tune its configuration for
> each individual uarch based on the properties of various C-states for a given CPU.

I also agree that latency-based is better than C-state-index-based. I
mean "Max 10us
wakeup latency" is portable across Intel/AMD/ARM without needing to
know the C-state table for each uarch and that's a real plus.

I looked at forced_idle_latency_limit_ns as a starting point, but it
doesn't quite fit as-is. Currently it is:

  1. Kernel-internal only, set by cpuidle_use_deepest_state(), called
     exclusively from play_idle_precise() (idle injection for thermal).

  2. A governor bypass, not a governor constraint, when set, the idle
     path skips the normal governor select() entirely and calls
     cpuidle_find_deepest_state() directly (in kernel/sched/idle.c).
     This forces the deepest state *within* the limit rather than
     letting the governor choose while *capping* at the limit.

  3. Not exposed via sysfs. No userspace interface exists.

For our use case, I want the governor to keep making its normal
decisions (considering load, predicted idle duration, etc.) but with
an upper bound on the exit latency it can select. More of a ceiling
than a "force deepest" override.

I see two paths forward:

  (a) Add a new per-CPU sysfs attribute under cpuidle, e.g.
      /sys/devices/system/cpu/cpuN/cpuidle/latency_limit_ns
      that sets a governor-respected ceiling. The governor's select()
      would filter out states with exit_latency_ns > limit. This is
      similar to how PM QoS cpu_latency works, but per-CPU rather
      than system-wide.

  (b) Reuse forced_idle_latency_limit_ns but change its semantics
      to be a governor ceiling rather than a governor bypass. This
      would affect the idle injection path though, so might not be
      desirable.

I tend to choose (a). I've already done a PoC that is working as expected.
I'll send a patch queue to the cpuidle maintainers (Rafael Wysocki,
Daniel Lezcano).

Thanks,
Anthony