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

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

On 19/12/2012 15:54, Ian Campbell wrote:
> On Wed, 2012-12-19 at 15:27 +0000, Andrew Cooper wrote:
>> 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.
> I know what the bit is. This code seems to be leaking physical CPU
> parameters into the virtual CPU state and the question is if userspace
> needs to care about that. I suspect the answer is no.
>
> What should matter for the guest state is the virtualised CPUID
> 0x80000008.EAX which, at least in theory, could be different (e.g. a
> migrated guest?). 
>
> Ian.
>

Ah - I see your concern.  Yes - it might well be different.  Is this
information passed in an HVM save record?  It does not appear to be
associated with the MTRR HVM save record.

An HVM domain, booted on processor with 48bit physical addressing, which
makes use of the upper 8 bits when setting up the variable MTRRs, which
is subsequently migrated to a different server with only 40bit physical
addressing is going to have problems.  Whether a protection fault occurs
or the top bits are simply ignored, stuff will break.

As a result, I think the virtualised CPUID value needs feature levelling
across hosts, or restrictions applied to where you can migrate a started
VM to.

~Andrew