From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48161) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKA61-00082F-Eu for qemu-devel@nongnu.org; Mon, 04 Jul 2016 16:03:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKA60-0002pm-Hd for qemu-devel@nongnu.org; Mon, 04 Jul 2016 16:03:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKA60-0002pW-A4 for qemu-devel@nongnu.org; Mon, 04 Jul 2016 16:03:56 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6FEDA46266 for ; Mon, 4 Jul 2016 20:03:55 +0000 (UTC) Date: Mon, 4 Jul 2016 17:03:53 -0300 From: Eduardo Habkost Message-ID: <20160704200353.GF4131@thinpad.lan.raisama.net> References: <1467659769-15900-1-git-send-email-dgilbert@redhat.com> <1467659769-15900-3-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467659769-15900-3-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/6] x86: Mask mtrr mask based on CPU physical address limits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, marcel@redhat.com, mst@redhat.com, kraxel@redhat.com On Mon, Jul 04, 2016 at 08:16:05PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > The CPU GPs if we try and set a bit in a variable MTRR mask above > the limit of physical address bits on the host. We hit this > when loading a migration from a host with a larger physical > address limit than our destination (e.g. a Xeon->i7 of same > generation) but previously used to get away with it > until 48e1a45 started checking that msr writes actually worked. > > It seems in our case the GP probably comes from KVM emulating > that GP. > > Signed-off-by: Dr. David Alan Gilbert > --- > target-i386/kvm.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index f3698f1..6429205 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -1677,6 +1677,8 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > } > } > if (has_msr_mtrr) { > + uint64_t phys_mask = MAKE_64BIT_MASK(0, cpu->phys_bits); > + > kvm_msr_entry_add(cpu, MSR_MTRRdefType, env->mtrr_deftype); > kvm_msr_entry_add(cpu, MSR_MTRRfix64K_00000, env->mtrr_fixed[0]); > kvm_msr_entry_add(cpu, MSR_MTRRfix16K_80000, env->mtrr_fixed[1]); > @@ -1690,10 +1692,15 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > kvm_msr_entry_add(cpu, MSR_MTRRfix4K_F0000, env->mtrr_fixed[9]); > kvm_msr_entry_add(cpu, MSR_MTRRfix4K_F8000, env->mtrr_fixed[10]); > for (i = 0; i < MSR_MTRRcap_VCNT; i++) { > + /* The CPU GPs if we write to a bit above the physical limit of > + * the host CPU (and KVM emulates that) > + */ > + uint64_t mask = env->mtrr_var[i].mask; > + mask &= phys_mask; > + I was arguing that we should print warnings when we really had to change the MTRR values on migration. But if we are already inside kvm_put_msrs() with a different phys_bits value, the best we can do is to trim the bits so they still make sense. If we want to print a warning, this can be done somewhere else. Reviewed-by: Eduardo Habkost > kvm_msr_entry_add(cpu, MSR_MTRRphysBase(i), > env->mtrr_var[i].base); > - kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i), > - env->mtrr_var[i].mask); > + kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i), mask); > } > } > > -- > 2.7.4 > -- Eduardo