Re: [Xen-devel] [PATCH V2 2/2] x86/altp2m: Fixed domain crash with INVALID_ALTP2M EPTP index

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 06/25/2018 03:54 PM, Jan Beulich wrote:
>>>> On 25.06.18 at 14:40, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> Crash:
>>>
>>> (XEN) [ 1924.367273] altp2m_vcpu_initialise()
>>> (XEN) [ 1924.367277] altp2m_vcpu_reset()
>>> (XEN) [ 1924.367278] 1 altp2m_vcpu_update_p2m()
>>> (XEN) [ 1924.367279] vmx_vcpu_update_eptp()
>>> (XEN) [ 1924.367318] HVMOP_altp2m_vcpu_enable_notify
>>> (XEN) [ 1924.367321] vmx_vcpu_update_vmfunc_ve(0),
>>> v->arch.hvm_vmx.secondary_exec_control: 0x1054eb
>>> (XEN) [ 1924.367326] exit vmx_vcpu_update_vmfunc_ve(0),
>>> v->arch.hvm_vmx.secondary_exec_control: 0x1474eb
>>> (XEN) [ 1924.367344] Xen BUG at vmx.c:3407
>>
>> Actually I think this shows us the problem: 65535 (INVALID_ALTP2M) is a
>> stale value from a previous good run. But the EPTP_INDEX value is
>> ignored unless SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS is set. So at the
>> crash point, SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS just got set, the
>> "live" index is 0, and the stale INVALID_ALTP2M value is being read from
>> the previous run (and compared to 0 and MAX_ALTP2M).
> 
> So perhaps the writing of EPTP_INDEX should be done earlier?

And indeed I can confirm this: I've added a sleep() in my test between
xc_altp2m_set_vcpu_enable_notify() and xc_altp2m_set_domain_state(xci,
domid, 0), and it _always_ crashes Xen on the second run.

Quite right, that's exactly what I've been doing: a satisfactory fix
appears to be to simply reverse the order of altp2m_vcpu_update_p2m(v)
and altp2m_vcpu_update_vmfunc_ve(v) in altp2m_vcpu_destroy().

I'll send out a patch ASAP.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel