Re: [Qemu-devel] [PATCH v2 3/6] x86: fill high bits of mtrr mask

[Dr. David Alan Gilbert]

* Eduardo Habkost (address@hidden) 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 / address@hidden / Manchester, UK