Re: [PATCH] KVM: SVM: Do not issue virtual VMLOAD/VMSAVE supported-message

[David Hildenbrand <david@redhat.com>]

On 06.09.2017 15:17, Borislav Petkov wrote:
> On Wed, Sep 06, 2017 at 02:54:05PM +0200, David Hildenbrand wrote:
>> Maybe go one step further and incorporate everything (+vls) into a
>> single if statement?
> 
> Or maybe simplify it even more by not even looking at vls. If the user
> disables it, fine, if she enables it but the hw doesn't support it, it
> will be set to false automatically.
> 
> Or am I missing a case?
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Tue, 5 Sep 2017 18:59:55 +0200
> Subject: [PATCH] KVM: SVM: Do not issue virtual VMLOAD/VMSAVE
>  supported-message
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> There's no need to issue that everytime during boot - we have the
> /proc/cpuinfo flag for people and software to query.
> 
> Also, simplify logic which verifies the vls chicken bit setting.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Janakarajan Natarajan <Janakarajan.Natarajan@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  arch/x86/kvm/svm.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 8dbd8dbc83eb..d3c481778d9c 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1098,15 +1098,10 @@ static __init int svm_hardware_setup(void)
>  		}
>  	}
>  
> -	if (vls) {
> -		if (!npt_enabled ||
> -		    !boot_cpu_has(X86_FEATURE_V_VMSAVE_VMLOAD) ||
> -		    !IS_ENABLED(CONFIG_X86_64)) {
> -			vls = false;
> -		} else {
> -			pr_info("Virtual VMLOAD VMSAVE supported\n");
> -		}
> -	}
> +	if (!npt_enabled ||
> +	    !boot_cpu_has(X86_FEATURE_V_VMSAVE_VMLOAD) ||
> +	    !IS_ENABLED(CONFIG_X86_64))
> +		vls = false;
>  
>  	return 0;
>  
> 

had the same idea but was worried about runtime. but  this is really
only executed once, so

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David