The shortlog is weird, I get what you're going for with the "()", but it honestly
looks like a typo :-) And add "power management" so that there's a bit more context
in the shortlog? Maybe this?
KVM: x86: Move *_in_guest power management flags to vCPU scope
On Tue, Dec 21, 2021, Kechen Lu wrote:
> For futher extensions on finer-grained control on per-vCPU exits
> disable control, and because VM-scoped restricted to set before
> vCPUs creation, runtime disabled exits flag check could be purely
> vCPU scope.
State what the patch does, not what it "could" do. E.g.
Make the runtime disabled mwait/hlt/pause/cstate exits flags vCPU scope
to allow finer-grained, per-vCPU control. The VM-scoped control is only
allowed before vCPUs are created, thus preserving the existing behavior
is a simple matter of snapshotting the flags at vCPU creation.
A few nits below that aren't even from this path, but otherwise,
Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Kechen Lu <kechenl@xxxxxxxxxx>
> ---
> arch/x86/include/asm/kvm_host.h | 5 +++++
> arch/x86/kvm/cpuid.c | 2 +-
> arch/x86/kvm/lapic.c | 2 +-
> arch/x86/kvm/svm/svm.c | 10 +++++-----
> arch/x86/kvm/vmx/vmx.c | 16 ++++++++--------
> arch/x86/kvm/x86.c | 6 +++++-
> arch/x86/kvm/x86.h | 16 ++++++++--------
> 7 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2164b9f4c7b0..edc5fca4d8c8 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -908,6 +908,11 @@ struct kvm_vcpu_arch {
> #if IS_ENABLED(CONFIG_HYPERV)
> hpa_t hv_root_tdp;
> #endif
> +
> + bool mwait_in_guest;
> + bool hlt_in_guest;
> + bool pause_in_guest;
> + bool cstate_in_guest;
> };
>
> struct kvm_lpage_info {
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 07e9215e911d..6291e15710ba 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -177,7 +177,7 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu)
> best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
>
> best = kvm_find_kvm_cpuid_features(vcpu);
> - if (kvm_hlt_in_guest(vcpu->kvm) && best &&
> + if (kvm_hlt_in_guest(vcpu) && best &&
> (best->eax & (1 << KVM_FEATURE_PV_UNHALT)))
Can you (or Paolo?) opportunistically align this? And maybe even shuffle the
check on "best" to pair the !NULL check with the functional check? E.g.
if (kvm_hlt_in_guest(vcpu) &&
best && (best->eax & (1 << KVM_FEATURE_PV_UNHALT)))
best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);
> best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index f206fc35deff..effb994e6642 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -119,7 +119,7 @@ static bool kvm_can_post_timer_interrupt(struct kvm_vcpu *vcpu)
> bool kvm_can_use_hv_timer(struct kvm_vcpu *vcpu)
> {
> return kvm_x86_ops.set_hv_timer
> - && !(kvm_mwait_in_guest(vcpu->kvm) ||
> + && !(kvm_mwait_in_guest(vcpu) ||
And another opportunistic tweak? I'm been itching for an excuse to "fix" this
particular helper for quite some time :-)
return kvm_x86_ops.set_hv_timer &&
!(kvm_mwait_in_guest(vcpu) || kvm_can_post_timer_interrupt(vcpu));
That overruns the 80 char soft limit, but IMO it's worth it in this case as the
resulting code is more readable.
> kvm_can_post_timer_interrupt(vcpu));
> }
> EXPORT_SYMBOL_GPL(kvm_can_use_hv_timer);
