Re: [Xen-devel] [PATCH 5/8] x86/vm-event/monitor: don't compromise monitor_write_data on domain cleanup

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 07/04/16 16:11, Jan Beulich wrote:
>>>> On 04.07.16 at 15:03, <czuzu@xxxxxxxxxxxxxxx> wrote:
>> On 7/4/2016 3:47 PM, Jan Beulich wrote:
>>>>>> On 30.06.16 at 20:45, <czuzu@xxxxxxxxxxxxxxx> wrote:
>>>> The arch_vm_event structure is dynamically allocated and freed @
>>>> vm_event_cleanup_domain. This cleanup is triggered e.g. when the toolstack 
>>>> user
>>>> disables domain monitoring (xc_monitor_disable), which in turn effectively
>>>> discards any information that was in arch_vm_event.write_data.
>>> Isn't that rather a toolstack user bug, not warranting a relatively
>>> extensive (even if mostly mechanical) hypervisor change like this
>>> one? Sane monitor behavior, after all, is required anyway for the
>>> monitored guest to survive.
>>
>> Sorry but could you please rephrase this, I don't quite understand what 
>> you're saying.
>> The write_data field in arch_vm_event should _not ever_ be invalidated 
>> as a direct result of a toolstack user's action.
> 
> The monitoring app can cause all kinds of problems to the guest it
> monitors. Why would this specific one need taking care of in the
> hypervisor, instead of demanding that the app not disable monitoring
> at the wrong time?

I'm not sure there's a right time here. The problem is that, if I
understand this correctly, a race is possible between the moment the
userspace application responds to the vm_event _and_ call
xc_monitor_disable() and the time hvm_do_resume() gets called.

If xc_monitor_disable() happened before hvm_do_resume() springs into
action, we lose a register write. There's no guaranteed way for this not
to happen as far as I can see, although it's true that the race should
pretty much never happen in practice - at least we've never come across
such a case so far.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel