On 12/07/2015 07:55 PM, Lengyel, Tamas wrote: > > > On Mon, Dec 7, 2015 at 4:09 AM, Razvan Cojocaru > <rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com>> wrote: > > Hello, > > While looking at some code with Tamas these past few days, we discovered > that xc_mem_access_enable_emulate() doesn't actually do anything other > that setting the mem_access_emulate_enabled flag. > > Fixing that would likely be trivial (if the flag is not set, > p2m_mem_access_emulate_check() should just return). However, my question > is: do we really need that function? Are there cases where it would make > sense to use the vm_event subsystem but disable emulation support? After > all, if the client code wants to use emulation support it can call > xc_mem_access_enable_emulate(), and if not it can simply not set the > EMULATE flags in the vm_event replies. > > As far as I'm concerned, the libxc function can go (along with the > per-domain flag), but then again we're emulation-intensive so it's quite > possible that I'm not seeing the proper use case for this. > > Thoughts? > > > It may make sense to gate the allocation of struct arch_vm_event to only > happen when emulation has been enabled beforehand for the domain (the > struct is used only for this purpose). Most places where it is used its > checked for by unlikely(v->arch.vm_event) checks anyway so I feel like > this was the original intention of having this flag to begin with. It's > not checked everywhere like this though so we need to verify that all > places that use it do check before using it.
I think the vm_event cleanup series (which introduced xc_mem_access_enable_emulate()) hit staging some months before I've changed the code to dynamically allocate that struct. Also, the struct is not only used for emulation. Please see the struct monitor_write_data write_data; member, that regulates CR and MSR writes. Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
