Re: [PATCH] libxc: Add xc_domain_hvm_get_mtrr_type() call

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 19/12/2012 15:00, Ian Campbell wrote:
> On Wed, 2012-12-19 at 14:57 +0000, Razvan Cojocaru wrote:
>>>>      m->overlapped = is_var_mtrr_overlapped(m);
>>>>
>>>> Looks like that function contains the necessary logic.
>>> You're right, but what happens there is that that function depends on
>>> the get_mtrr_range() function, which in turn depends on the size_or_mask
>>> global variable, which is initialized in hvm_mtrr_pat_init(), which then
>>> depends on a global table, and so on. Putting that into libxc is pretty
>>> much putting the whole mtrr.c file there.
>> This is where it gets tricky:
>>
>> static void get_mtrr_range(uint64_t base_msr, uint64_t mask_msr,
>>                             uint64_t *base, uint64_t *end)
>> {
>>      [...]
>>      phys_addr = 36;
>>
>>      if ( cpuid_eax(0x80000000) >= 0x80000008 )
>>          phys_addr = (uint8_t)cpuid_eax(0x80000008);
>>
>>      size_or_mask = ~((1 << (phys_addr - PAGE_SHIFT)) - 1);
>>      [...]
>> }
>>
>> specifically, in the cpuid_eax() call, which doesn't make much sense in 
>> dom0 userspace.
> The fact that get_mtrr_range is querying the underlying physical CPUID
> suggests it has something to do with the translation from virtual to
> physical MTRR and is therefore not something userspace needs to worry
> about, but I'm only speculating.

CPUID 0x80000008.EAX is the physical address size supported by the
processor (in bits).  Typical values on modern hardware are 40 or 48.

You need to know this information to work out which bits in the MTRR are
valid.  It is a variable scale of which bits are reserved.

Having said the above, this information is never going to change on the
same CPU, so should be cached once to save repeated emulates of cpuid.

~Andrew

>
>> I did manage to take 'enabled' into account with what appears to be 
>> success, but if I've read the situation correctly, there's not much to 
>> do about 'overlap', unless we save it in hvm_save_mtrr_msr() like it's 
>> done with 'enabled'. What do you think?
> It's not an architectural thing so I don't think it belongs in there.
>
> TBH until someone figures out or explains what overlap actually is I
> don't know if it even needs exporting or taking into account in
> userspace.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel