From patchwork Sun May 19 04:52:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nakajima, Jun" X-Patchwork-Id: 2589671 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id BADC840B0D for ; Sun, 19 May 2013 04:52:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751724Ab3ESEww (ORCPT ); Sun, 19 May 2013 00:52:52 -0400 Received: from mail-da0-f42.google.com ([209.85.210.42]:33458 "EHLO mail-da0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751219Ab3ESEwu (ORCPT ); Sun, 19 May 2013 00:52:50 -0400 Received: by mail-da0-f42.google.com with SMTP id r6so1922508dad.29 for ; Sat, 18 May 2013 21:52:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=4uiUzDdC726uEtuMDyYzRiiqxH9+zZ4MmEaT6i7k5Iw=; b=k8wViMxffCglUHcb3Ih5Y8S9gOfYSpWeJ4opz8haGeFtK6LhcVM0cVC24gbRmbTKon Z4I+Hgsnlh99MBDZgvbnxqxkJt/VgcDF45Tkk+Ht3Jr0Fih5buhmLyAzgdI3gLuewFwq IjS9JIj47SbYTK+VFkQqe5RJTFLtB3cY8U26qtfFzJ0oSJ4HMOmdGhlNxLBcwRiCn7bV GUZ6yXj2HDXGQknFc9+jt+FlGqk5nL3k1i92wQoY5PDPTVuif9fcd+DcvsUEic2o+BG0 mezuOrbD9yf7XUEJ+x6si+XLYi0OJVkp+Q+n5g7FkXbdPtp1ilx7UpsoFbCHWLZirNHO 1xpw== X-Received: by 10.66.148.168 with SMTP id tt8mr44048670pab.66.1368939170253; Sat, 18 May 2013 21:52:50 -0700 (PDT) Received: from localhost (c-98-207-34-191.hsd1.ca.comcast.net. [98.207.34.191]) by mx.google.com with ESMTPSA id zs12sm8967016pab.0.2013.05.18.21.52.48 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 18 May 2013 21:52:49 -0700 (PDT) From: Jun Nakajima To: kvm@vger.kernel.org Cc: Gleb Natapov , Paolo Bonzini Subject: [PATCH v3 07/13] nEPT: Fix wrong test in kvm_set_cr3 Date: Sat, 18 May 2013 21:52:26 -0700 Message-Id: <1368939152-11406-7-git-send-email-jun.nakajima@intel.com> X-Mailer: git-send-email 1.8.2.1.610.g562af5b In-Reply-To: <1368939152-11406-1-git-send-email-jun.nakajima@intel.com> References: <1368939152-11406-1-git-send-email-jun.nakajima@intel.com> X-Gm-Message-State: ALoCoQmIeYMBk28MtHk1ZUbUHBEgUKiUjDExuxT6ORoNyKuOOmnFpapxUQZs9Fg9XOxLKkxjyWm2 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org From: Nadav Har'El kvm_set_cr3() attempts to check if the new cr3 is a valid guest physical address. The problem is that with nested EPT, cr3 is an *L2* physical address, not an L1 physical address as this test expects. As the comment above this test explains, it isn't necessary, and doesn't correspond to anything a real processor would do. So this patch removes it. Note that this wrong test could have also theoretically caused problems in nested NPT, not just in nested EPT. However, in practice, the problem was avoided: nested_svm_vmexit()/vmrun() do not call kvm_set_cr3 in the nested NPT case, and instead set the vmcb (and arch.cr3) directly, thus circumventing the problem. Additional potential calls to the buggy function are avoided in that we don't trap cr3 modifications when nested NPT is enabled. However, because in nested VMX we did want to use kvm_set_cr3() (as requested in Avi Kivity's review of the original nested VMX patches), we can't avoid this problem and need to fix it. Signed-off-by: Nadav Har'El Signed-off-by: Jun Nakajima Signed-off-by: Xinhao Xu --- arch/x86/kvm/x86.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 094b5d9..7b36ec6 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -683,17 +683,6 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) */ } - /* - * Does the new cr3 value map to physical memory? (Note, we - * catch an invalid cr3 even in real-mode, because it would - * cause trouble later on when we turn on paging anyway.) - * - * A real CPU would silently accept an invalid cr3 and would - * attempt to use it - with largely undefined (and often hard - * to debug) behavior on the guest side. - */ - if (unlikely(!gfn_to_memslot(vcpu->kvm, cr3 >> PAGE_SHIFT))) - return 1; vcpu->arch.cr3 = cr3; __set_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail); vcpu->arch.mmu.new_cr3(vcpu);