On 01/29/2016 08:52 AM, Razvan Cojocaru wrote:
> On 01/28/2016 11:47 PM, Tamas K Lengyel wrote:
>>
>>
>> On Thu, Jan 28, 2016 at 2:05 PM, Razvan Cojocaru
>> <rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com>> wrote:
>>
>>     On 01/28/2016 10:09 PM, Tamas K Lengyel wrote:
>>     > On Thu, Jan 28, 2016 at 6:52 AM, Razvan Cojocaru
>>     > <rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com>
>>     <mailto:rcojoc...@bitdefender.com
>>     <mailto:rcojoc...@bitdefender.com>>> wrote:
>>     >
>>     >     This patch pauses the domain for all writes through the 'ad'
>>     >     pointer in monitor_domctl(), defers a domain_unpause() call until
>>     >     after the CRs are updated for the MONITOR_EVENT_WRITE_CTRLREG
>>     >     case, and makes sure that the domain is paused for both vm_event
>>     >     enable and disable cases in vm_event_domctl().
>>     >     Thanks go to Andrew Cooper for his review and suggestions.
>>     >
>>     >
>>     > For vm_event_enable the domain is already paused by libxc before the
>>     > domctl is issued. I don't see a problem in doing another pause in Xen,
>>     > but given we have XSA-99, just doing this pause in Xen would not be
>>     > enough. So is it really necessary/fixes anything?
>>
>>     This isn't about XSA-99, the problem here is related to my previous
>>     patch "x86 vm_event: reset monitor in vm_event_cleanup_domain()". While
>>     that improves matters and greatly reduces the chances of crashes due to
>>     hvm_msr_write_intercept() or hvm_set_crX() dereferencing a NULL
>>     v->arch.vm_event that's assumed to be OK, when the corresponding
>>     v->domain->arch.monitor is non-zero, the foolproof way is to make sure
>>     that functions such as vm_event_cleanup_domain() are always being called
>>     only while the domain has been paused. So there should be a
>>     domain_pause() call somewhere on the call path before that.
>>
>>
>> Sure, but that's the disable case. I was only wondering about the enable
>> case where the domain is already paused.
> 
> Yes, for the disable case it is functionally redundant.

Sorry, I obviously meant "for the enable case".


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

Reply via email to