Re: [Xen-devel] [PATCH V3] x86/vm_event: block interrupt injection for sync vm_events

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.01.19 at 08:10, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 1/14/19 4:42 PM, Juergen Gross wrote:
>> On 14/01/2019 11:56, Razvan Cojocaru wrote:
>>> On 1/14/19 11:53 AM, Jan Beulich wrote:
>>>>>>> On 14.01.19 at 10:34, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> On 1/12/19 12:04 AM, Boris Ostrovsky wrote:
>>>>>> On 12/14/18 6:49 AM, Razvan Cojocaru wrote:
>>>>>>> Block interrupts (in vmx_intr_assist()) for the duration of
>>>>>>> processing a sync vm_event (similarly to the strategy
>>>>>>> currently used for single-stepping). Otherwise, attempting
>>>>>>> to emulate an instruction when requested by a vm_event
>>>>>>> reply may legitimately need to call e.g.
>>>>>>> hvm_inject_page_fault(), which then overwrites the active
>>>>>>> interrupt in the VMCS.
>>>>>>>
>>>>>>> The sync vm_event handling path on x86/VMX is (roughly):
>>>>>>> monitor_traps() -> process vm_event -> vmx_intr_assist()
>>>>>>> (possibly writing VM_ENTRY_INTR_INFO) ->
>>>>>>> hvm_vm_event_do_resume() -> hvm_emulate_one_vm_event()
>>>>>>> (possibly overwriting the VM_ENTRY_INTR_INFO value).
>>>>>>>
>>>>>>> This patch may also be helpful for the future removal
>>>>>>> of may_defer in hvm_set_cr{0,3,4} and hvm_set_msr().
>>>>>>>
>>>>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>>>>>>
>>>>>>
>>>>>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>>>>>
>>>>> Thanks! So now we have three reviewed-bys, if I'm not mistaken all we
>>>>> need is Tamas' (for the vm_event part) and Julien / Stefano's (for ARM)
>>>>> acks (or otherwise).
>>>>
>>>> And you'd need to talk Jürgen into allowing this in, now that we're
>>>> past the freeze point.
>>>
>>> (Adding Jürgen to the conversation.)
>>>
>>> Right, that would be ideal if possible - the code has absolutely no
>>> impact on anything unless vm_event is active on the domain, which is
>>> never the case for the use-cases considered for a Xen release.
>>>
>>> So the case I'm making for the patch to go in 4.12 is that:
>>>
>>> 1. It's perfectly harmless (it affects nothing, except for introspection).
>>>
>>> 2. It's trivial and thus very easy to see that it's correct.
>>>
>>> 3. It fixes a major headache for us, and thus it is a great improvement
>>> from an introspection standpoint (fixes OS crashes / hangs which we'd
>>> otherwise need to work around in rather painful ways).
>>>
>>> 4. V3 of the patch has been sent out on Dec 14th - it's just that
>>> reviewers have had other priorities and it did not gather all acks in time.
>>>
>>> However, if it's not possible or desirable to allow this in the next
>>> best thing is to at least have all the acks necessary for it to go in
>>> first thing once the freeze is over.
>>>
>>> From Julien's reply I understand that the last ack necessary is Tamas'.
>> 
>> With that ack just arrived:
>> 
>> Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
> AFAICT this is fine to apply to staging now, am I incorrect?

Yes, but may I ask that you be a little more patient?

Jan


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