From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43762) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKLtB-0005Ya-1m for qemu-devel@nongnu.org; Tue, 05 Jul 2016 04:39:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKLt7-00056h-AS for qemu-devel@nongnu.org; Tue, 05 Jul 2016 04:39:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46312) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKLt7-00056d-23 for qemu-devel@nongnu.org; Tue, 05 Jul 2016 04:39:25 -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 7DB2660AE for ; Tue, 5 Jul 2016 08:39:24 +0000 (UTC) Date: Tue, 5 Jul 2016 09:39:20 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20160705083919.GA2118@work-vm> References: <1467659769-15900-1-git-send-email-dgilbert@redhat.com> <1467659769-15900-4-git-send-email-dgilbert@redhat.com> <20160704202148.GI4131@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160704202148.GI4131@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH v2 3/6] x86: fill high bits of mtrr mask List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, marcel@redhat.com, mst@redhat.com, kraxel@redhat.com * Eduardo Habkost (ehabkost@redhat.com) wrote: > On Mon, Jul 04, 2016 at 08:16:06PM +0100, Dr. David Alan Gilbert (git) wrote: > [...] > > @@ -2084,6 +2085,27 @@ static int kvm_get_msrs(X86CPU *cpu) > > } > > > > assert(ret == cpu->kvm_msr_buf->nmsrs); > > + /* > > + * MTRR masks: Each mask consists of 5 parts > > + * a 10..0: must be zero > > + * b 11 : valid bit > > + * c n-1.12: actual mask bits > > + * d 51..n: reserved must be zero > > + * e 63.52: reserved must be zero > > + * > > + * 'n' is the number of physical bits supported by the CPU and is > > + * apparently always <= 52. We know our 'n' but don't know what > > + * the destinations 'n' is; it might be smaller, in which case > > + * it masks (c) on loading. It might be larger, in which case > > + * we fill 'd' so that d..c is consistent irrespetive of the 'n' > > + * we're migrating to. > > + */ > > + if (cpu->fill_mtrr_mask && cpu->phys_bits < 52) { > > + mtrr_top_bits = MAKE_64BIT_MASK(cpu->phys_bits, 52 - cpu->phys_bits); > > + } else { > > + mtrr_top_bits = 0; > > How/where did you find this 52-bit limit? Is it documented > somewhere? It seems to come from AMDs original specification of AMD64; but you're right we could do with a constant rather than the magical 52 everywhere. Looking in AMD doc 24593 Rev 3.26 (AMD64 Arch Programmer's manual vol. 2) p.191 Fig 7.6 MTRRphysBasen Register it shows it as PhysBase running from 51:32 and 63:52 being Reserved/MBZ; The corresponding Intel doc (Intel 64 & IA-32 Architectures Dev manual 3A 11-25 fig 11-7) doesn't have that limit shown; however it does talk about 52-bit physical addresses in a few places; e.g. 4.4 PAE paging talks about 'paging translates 32-bit linear addresses to 52-bit physical addresses' I think the most relevant place in the Intel doc is 5.13.3 'Reserved Bit Checking' which has a Table 5-8 'IA-32e Mode Page Level Protection Matrix with Execute-Disable Bit Capability Enabled' this is a table of reserved bit fields and for each of the page tables it shows bits checked as [51:MAXPHYADDR]. Any suggestions for a name for the 52 constant? I guess MaxMaxPhyAddress? I guess someone decided that 4PB ought to be enough for anyone. Dave > > > + } > > + > > for (i = 0; i < ret; i++) { > > uint32_t index = msrs[i].index; > > switch (index) { > > @@ -2279,7 +2301,8 @@ static int kvm_get_msrs(X86CPU *cpu) > > break; > > case MSR_MTRRphysBase(0) ... MSR_MTRRphysMask(MSR_MTRRcap_VCNT - 1): > > if (index & 1) { > > - env->mtrr_var[MSR_MTRRphysIndex(index)].mask = msrs[i].data; > > + env->mtrr_var[MSR_MTRRphysIndex(index)].mask = msrs[i].data | > > + mtrr_top_bits; > > } else { > > env->mtrr_var[MSR_MTRRphysIndex(index)].base = msrs[i].data; > > } > > -- > > 2.7.4 > > > > -- > Eduardo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK