Re: [Xen-devel] [PATCH V4] x86/altp2m: Fix crash with INVALID_ALTP2M EPTP index

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.06.18 at 17:25, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 06/27/2018 06:04 PM, Jan Beulich wrote:
>>>>> On 27.06.18 at 15:12, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> xc_altp2m_set_vcpu_enable_notify() ends up calling
>>> altp2m_vcpu_update_vmfunc_ve(), which sets the
>>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS bit on
>>> vmx_secondary_exec_control. A subsequent call to
>>> xc_altp2m_set_domain_state(..., false) (i.e. disabling altp2m
>>> for the domain) ends up calling altp2m_vcpu_destroy(), which
>>> calls (in this order) altp2m_vcpu_reset() (which sets the
>>> current EPTP index to INVALID_ALTP2M), altp2m_vcpu_update_p2m()
>>> (which __vmwrite()s EPTP_INDEX as INVALID_ALTP2M if
>>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS is set), and
>>> altp2m_vcpu_update_vmfunc_ve() (which finally clears
>>> SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS).
>> 
>> I continue to consider this paragraph unreadable, but perhaps it's
>> just me. The rest of the description looks reasonable to me now.
> 
> If you (or anyone else) have something specific in mind I'd be happy to
> change it to that, otherwise I can also try again (the only concern is
> that I might unwantedly continue to be unable to guess correctly at the
> desired balance between technical clarity (detail) and general readability).
> 
> Is "A VM entry handler executed immediately after enabling #VE might
> find a stale __vmsave()d EPTP_INDEX, stored by calling
> altp2m_vcpu_destroy() when SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS had
> been set by altp2m_vcpu_update_vmfunc_ve()." a better first paragraph
> perhaps?

To me personally - yes, very much.

Jan



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