From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [PATCH] Use WARN_ON_ONCE for missing X86_FEATURE_NRIPS Date: Mon, 5 Oct 2015 11:50:22 +0200 Message-ID: <20151005095022.GX3036@8bytes.org> References: <560D1C6E.2060803@suse.com> <560D278F.10801@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , kvm@vger.kernel.org, Dirk =?iso-8859-1?Q?M=FCller?= To: Bandan Das Return-path: Received: from 8bytes.org ([81.169.241.247]:52468 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752264AbbJEJuY (ORCPT ); Mon, 5 Oct 2015 05:50:24 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Oct 01, 2015 at 06:31:27PM -0400, Bandan Das wrote: > Paolo Bonzini writes: >=20 > > On 01/10/2015 13:43, Dirk M=FCller wrote: > >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > >> index 94b7d15..0a42859 100644 > >> --- a/arch/x86/kvm/svm.c > >> +++ b/arch/x86/kvm/svm.c > >> @@ -514,7 +514,7 @@ static void skip_emulated_instruction(struct k= vm_vcpu *vcpu) > >> struct vcpu_svm *svm =3D to_svm(vcpu); > >> =20 > >> if (svm->vmcb->control.next_rip !=3D 0) { > >> - WARN_ON(!static_cpu_has(X86_FEATURE_NRIPS)); > >> + WARN_ON_ONCE(!static_cpu_has(X86_FEATURE_NRIPS)); > >> svm->next_rip =3D svm->vmcb->control.next_rip; > >> } > >> =20 > > > > Bandan, what was the reason for warning here? >=20 > I added the warning so that we catch if the next_rip field is being w= ritten > to (even if the feature isn't supported) by a buggy L1 hypervisor. Even if the L1 hypervisor writes to the next_rip field in the VMCB, we would never see it in this code path, as we access the shadow VMCB in this statement. We don't even care if the L1 hypervisor writes to its next_rip field because we only write to this field on an emulatated VMEXIT and never read it back. So what's the point in adding a guest-triggerable warning at all? Joerg