From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bandan Das <bsd@redhat.com>
Subject: Re: [PATCH] Use WARN_ON_ONCE for missing X86_FEATURE_NRIPS
Date: Sun, 04 Oct 2015 21:15:02 -0400
Message-ID: <jpg4mi6jeuh.fsf@linux.bootlegged.copy>
References: <560D1C6E.2060803@suse.com> <560D278F.10801@redhat.com>
	<jpgbncixlts.fsf@linux.bootlegged.copy>
	<101C0DE6-0CEA-4F15-9E9F-9D2EDF23ED4C@suse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Joerg Roedel <joro@8bytes.org>
To: Dirk =?utf-8?Q?M=C3=BCller?= <dmueller@suse.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:58414 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751153AbbJEBPG convert rfc822-to-8bit (ORCPT
	<rfc822;kvm@vger.kernel.org>); Sun, 4 Oct 2015 21:15:06 -0400
In-Reply-To: <101C0DE6-0CEA-4F15-9E9F-9D2EDF23ED4C@suse.com> ("Dirk
 \=\?utf-8\?Q\?M\=C3\=BCller\=22's\?\=
	message of "Fri, 2 Oct 2015 08:43:08 +0200")
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Dirk M=C3=BCller <dmueller@suse.com> writes:

>> I added the warning so that we catch if the next_rip field is being =
written
>> to (even if the feature isn't supported) by a buggy L1 hypervisor.
>
> Interesting, so how about this patch?
>
>
> From c5c8ea255d680f972cbdfc835cdf352fa78897ae Mon Sep 17 00:00:00 200=
1
> From: Dirk Mueller <dirk@dmllr.de>
> Date: Fri, 2 Oct 2015 08:35:24 +0200
> Subject: [PATCH] KVM: nSVM: Check for NRIP support before accepting
>  control.next_rip
>
> NRIP support itself depends on cpuid Fn8000_000A_EDX[NRIPS], remove
> a WARN_ON_(once) and check for it directly.
>
> Signed-off-by: Dirk Mueller <dmueller@suse.com>
> ---
>  arch/x86/kvm/svm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 0a42859..33d36da 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -513,8 +513,8 @@ static void skip_emulated_instruction(struct kvm_=
vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm =3D to_svm(vcpu);
> =20
> -	if (svm->vmcb->control.next_rip !=3D 0) {
> -		WARN_ON_ONCE(!static_cpu_has(X86_FEATURE_NRIPS));
> +	if (static_cpu_has(X86_FEATURE_NRIPS) &&
> +	    svm->vmcb->control.next_rip !=3D 0) {
>  		svm->next_rip =3D svm->vmcb->control.next_rip;
>  	}

Ok, looks good to me. Still, probably a good idea to let the user know =
if this condition is
hit.

Bandan