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

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 6 Sep 2017 16:00:04 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     KVM <kvm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Janakarajan Natarajan <Janakarajan.Natarajan@....com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>
Subject: Re: [PATCH] KVM: SVM: Do not issue virtual VMLOAD/VMSAVE
 supported-message

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@...e.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@...e.de>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: Janakarajan Natarajan <Janakarajan.Natarajan@....com>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Radim Krčmář <rkrcmar@...hat.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@...hat.com>

-- 

Thanks,

David