From patchwork Wed Oct 7 12:47:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joerg Roedel X-Patchwork-Id: 7345081 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3293BBEEA4 for ; Wed, 7 Oct 2015 12:47:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2548F20304 for ; Wed, 7 Oct 2015 12:47:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 575EB20272 for ; Wed, 7 Oct 2015 12:47:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753817AbbJGMrE (ORCPT ); Wed, 7 Oct 2015 08:47:04 -0400 Received: from 8bytes.org ([81.169.241.247]:46399 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394AbbJGMrC (ORCPT ); Wed, 7 Oct 2015 08:47:02 -0400 Received: by theia.8bytes.org (Postfix, from userid 1000) id 3D9203C5; Wed, 7 Oct 2015 14:47:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=8bytes.org; s=mail-1; t=1444222020; bh=UmnuNgmmO3sTcu8zV6s0JOYuh3WYdvY/3soyDzDEvJA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rb5b4KTlFJYWSdreFTydn6gyvN2MNmtCluYCOIsW2NHjEvH5lNJK5nFlgFsldnm7s schyCe1/O5WU0frGJWvhdoCkrNnkI78xW6o0FRP2WrWcJ1aA9qIlFMo/sKmwHjbIIN 2JEUw3obZL0Vo5ia0iaQqCiQ+jpKZdKCWKPmk5la1vhFL0dJTQERebKx0sxIdFpTNH pRO+u0pNbVXMBGn6hJPTrgutVhjzvqEGUsT0UTqQKPjj7cPxRP81BMJKPd4PfZGpYa 5XL0pMAbiScxTM4WDyZ2K1ZYnRriBXFCnDiPMTPGmnDGS7FvtuVjeDs0Z8jIEla0sP /VQTDAfQ2G36A== Date: Wed, 7 Oct 2015 14:47:00 +0200 From: Joerg Roedel To: Bandan Das Cc: Paolo Bonzini , kvm@vger.kernel.org, Dirk =?iso-8859-1?Q?M=FCller?= Subject: [PATCH] kvm: svm: Only propagate next_rip when guest supports it Message-ID: <20151007124700.GE28811@8bytes.org> References: <560D1C6E.2060803@suse.com> <560D278F.10801@redhat.com> <20151006102838.GD20886@8bytes.org> <20151007110335.GA28811@8bytes.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20151007110335.GA28811@8bytes.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Oct 07, 2015 at 01:03:35PM +0200, Joerg Roedel wrote: > But we don't care if L1 writes something into its own next_rip, as we > never read this value from its VMCB. We only copy the next_rip value we > get from our shadow-vmcb to it on an emulated vmexit. So I still don't > understand what triggers the reported problem or why the WARN_ON is > necessary. Okay, I think I have an idea now. I talked a bit with Dirk and the WARN_ON triggers in the guest, and not on the host. This makes a lot more sense. In nested-svm we always copy the next_rip from the shadow-vmcb to the guests vmcb, even when the nrips bit in cpuid is not set for the guest. This obviously triggers the WARN_ON() in the L1 KVM (I still don't understand why the WARN_ON was introduced in the first place). So the right fix is to only copy next_rip to the guests vmcb when its cpuid indicates that next_rip is supported there, like in this patch: From 019afc60507618b8e44e0c67d5ea2d850d88c9dd Mon Sep 17 00:00:00 2001 From: Joerg Roedel Date: Wed, 7 Oct 2015 13:38:19 +0200 Subject: [PATCH] kvm: svm: Only propagate next_rip when guest supports it Currently we always write the next_rip of the shadow vmcb to the guests vmcb when we emulate a vmexit. This could confuse the guest when its cpuid indicated no support for the next_rip feature. Fix this by only propagating next_rip if the guest actually supports it. Signed-off-by: Joerg Roedel Tested-By: Dirk Mueller --- arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++ arch/x86/kvm/svm.c | 7 ++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h index dd05b9c..effca1f 100644 --- a/arch/x86/kvm/cpuid.h +++ b/arch/x86/kvm/cpuid.h @@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) best = kvm_find_cpuid_entry(vcpu, 7, 0); return best && (best->ebx & bit(X86_FEATURE_MPX)); } + +/* + * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3 + */ +#define BIT_NRIPS 3 + +static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *best; + + best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0); + + /* + * NRIPS is a scattered cpuid feature, so we can't use + * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit + * position 8, not 3). + */ + return best && (best->edx & bit(BIT_NRIPS)); +} +#undef BIT_NRIPS + #endif diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 94b7d15..e1a8824 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2459,7 +2459,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm) nested_vmcb->control.exit_info_2 = vmcb->control.exit_info_2; nested_vmcb->control.exit_int_info = vmcb->control.exit_int_info; nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err; - nested_vmcb->control.next_rip = vmcb->control.next_rip; + + if (guest_cpuid_has_nrips(vcpu)) + nested_vmcb->control.next_rip = vmcb->control.next_rip; /* * If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have @@ -2714,6 +2716,9 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) svm->vmcb->control.event_inj = nested_vmcb->control.event_inj; svm->vmcb->control.event_inj_err = nested_vmcb->control.event_inj_err; + /* Clear next_rip, as real hardware would do */ + nested_vmcb->control.next_rip = 0; + nested_svm_unmap(page); /* Enter Guest-Mode */