Re: [PATCH RFC v13 06/20] pvh: vmx-specific changes

[George Dunlap <george.dunlap@eu.citrix.com>]

On 07/10/13 17:12, Tim Deegan wrote:
> At 17:06 +0100 on 07 Oct (1381165615), George Dunlap wrote:
>> On 07/10/13 16:55, Roger Pau Monné wrote:
>>> On 23/09/13 18:49, George Dunlap wrote:
>>>> @@ -1028,12 +1129,28 @@ static int construct_vmcs(struct vcpu *v)
>>>>                 | (1U << TRAP_no_device);
>>>>       vmx_update_exception_bitmap(v);
>>>>   
>>>> +    /* In HVM domains, this happens on the realmode->paging
>>>> +     * transition.  Since PVH never goes through this transition, we
>>>> +     * need to do it at start-of-day. */
>>>> +    if ( is_pvh_domain(d) )
>>>> +        vmx_update_debug_state(v);
>>>> +
>>>>       v->arch.hvm_vcpu.guest_cr[0] = X86_CR0_PE | X86_CR0_ET;
>>>> +
>>>> +    /* PVH domains always start in paging mode */
>>>> +    if ( is_pvh_domain(d) )
>>>> +        v->arch.hvm_vcpu.guest_cr[0] |= X86_CR0_PG | X86_CR0_NE |
>>>> X86_CR0_WP;
>>>> +
>>>>       hvm_update_guest_cr(v, 0);
>>>>   
>>>> -    v->arch.hvm_vcpu.guest_cr[4] = 0;
>>>> +    v->arch.hvm_vcpu.guest_cr[4] = is_pvh_domain(d) ?
>>>> +        real_cr4_to_pv_guest_cr4(mmu_cr4_features)
>>> Here we need to mask the bits in CR4 that the guest isn't allowed to
>>> set. Right now Xen is setting the VMXE bit by default, which the guest
>>> is not able to modify, so if the guests tries to update CR4 based on the
>>> previous value Xen is going to complain:
>>>
>>> +        real_cr4_to_pv_guest_cr4(mmu_cr4_features) &
>>> +        ~HVM_CR4_GUEST_RESERVED_BITS(v)
>> Thanks for testing that -- I'll include it in the next spin-up.
> <harping on> This is the sort of thing that would be easier if PVH guests
> were just HVM ones with extra hypercalls. <harping off>

I'm not sure this would have helped -- the issue here is that we start 
the guest with long mode and paging enabled, which I think is something 
we want to do (rather than having to start in 16-bit mode and level-up 
through 32-bit to 64-bit mode).  To do that, Mukesh very reasonably 
began by setting the guest cr4 to what a PV guest would see of the cr4, 
and for him it worked -- apparently because his PVH Linux doesn't touch 
cr4.  It's possible that his version of the cr4 exit handler tolerated 
the VMXE bit being set, in which case it was the reworking I did that 
introduced the bug.  (Although that demonstrates why having duplicate 
code is a really bad idea.)

  -George