Re: Xc_mem_access_enable_emulate() is currently a no-operation

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 12/07/2015 07:55 PM, Lengyel, Tamas wrote:
> 
> 
> On Mon, Dec 7, 2015 at 4:09 AM, Razvan Cojocaru
> <rcojocaru@bitdefender.com <mailto:rcojocaru@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