linux-kernel - Re: [PATCH v3 4/7] KVM: x86: nSVM: support PAUSE filter threshold and count when cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a235df4f-ba06-1b01-c588-06f12d8341b7@redhat.com>
Date:   Wed, 9 Mar 2022 14:12:52 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org
Cc:     Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jim Mattson <jmattson@...gle.com>, x86@...nel.org,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>, linux-kernel@...r.kernel.org,
        Wanpeng Li <wanpengli@...cent.com>
Subject: Re: [PATCH v3 4/7] KVM: x86: nSVM: support PAUSE filter threshold and
 count when cpu_pm=on

On 3/1/22 15:36, Maxim Levitsky wrote:
> Allow L1 to use these settings if L0 disables PAUSE interception
> (AKA cpu_pm=on)
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
>   arch/x86/kvm/svm/nested.c |  6 ++++++
>   arch/x86/kvm/svm/svm.c    | 17 +++++++++++++++++
>   arch/x86/kvm/svm/svm.h    |  2 ++
>   3 files changed, 25 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 37510cb206190..4cb0bc49986d5 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -664,6 +664,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
>   	if (!nested_vmcb_needs_vls_intercept(svm))
>   		svm->vmcb->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;
>   
> +	if (svm->pause_filter_enabled)
> +		svm->vmcb->control.pause_filter_count = svm->nested.ctl.pause_filter_count;
> +
> +	if (svm->pause_threshold_enabled)
> +		svm->vmcb->control.pause_filter_thresh = svm->nested.ctl.pause_filter_thresh;

I think this should be

	if (kvm_pause_in_guest(vcpu->kvm)) {
		/* copy from VMCB12 if guest has CPUID, else set to 0 */
	} else {
		/* copy from VMCB01, unconditionally */
	}

and likewise it should be copied back to VMCB01 unconditionally on 
vmexit if !kvm_pause_in_guest(vcpu->kvm).

>   	nested_svm_transition_tlb_flush(vcpu);
>   
>   	/* Enter Guest-Mode */
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 6a571eed32ef4..52198e63c5fc4 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4008,6 +4008,17 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
>   
>   	svm->v_vmload_vmsave_enabled = vls && guest_cpuid_has(vcpu, X86_FEATURE_V_VMSAVE_VMLOAD);
>   
> +	if (kvm_pause_in_guest(vcpu->kvm)) {
> +		svm->pause_filter_enabled = pause_filter_count > 0 &&
> +					    guest_cpuid_has(vcpu, X86_FEATURE_PAUSEFILTER);
> +
> +		svm->pause_threshold_enabled = pause_filter_thresh > 0 &&
> +					    guest_cpuid_has(vcpu, X86_FEATURE_PFTHRESHOLD);

Why only if the module parameters are >0?  The module parameter is 
unused if pause-in-guest is active.

> +	} else {
> +		svm->pause_filter_enabled = false;
> +		svm->pause_threshold_enabled = false;
> +	}
> +
>   	svm_recalc_instruction_intercepts(vcpu, svm);
>   
>   	/* For sev guests, the memory encryption bit is not reserved in CR3.  */
> @@ -4763,6 +4774,12 @@ static __init void svm_set_cpu_caps(void)
>   		if (vls)
>   			kvm_cpu_cap_set(X86_FEATURE_V_VMSAVE_VMLOAD);
>   
> +		if (pause_filter_count)
> +			kvm_cpu_cap_set(X86_FEATURE_PAUSEFILTER);
> +
> +		if (pause_filter_thresh)
> +			kvm_cpu_cap_set(X86_FEATURE_PFTHRESHOLD);

Likewise, this should be set using just boot_cpu_has, not the module 
parameters.

Paolo

>   		/* Nested VM can receive #VMEXIT instead of triggering #GP */
>   		kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK);
>   	}
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index a3c93f9c02847..6fa81eb3ffb78 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -234,6 +234,8 @@ struct vcpu_svm {
>   	bool tsc_scaling_enabled          : 1;
>   	bool lbrv_enabled                 : 1;
>   	bool v_vmload_vmsave_enabled      : 1;
> +	bool pause_filter_enabled         : 1;
> +	bool pause_threshold_enabled      : 1;
>   
>   	u32 ldr_reg;
>   	u32 dfr_reg;