Re: [PATCH RFC V2 3/6] xen: Force-enable relevant MSR events; optimize the number of sent MSR events

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/07/14 19:22, Razvan Cojocaru wrote:
> On 07/11/2014 09:19 PM, Andrew Cooper wrote:
>> On 11/07/14 19:09, Razvan Cojocaru wrote:
>>> On 07/11/2014 08:03 PM, Andrew Cooper wrote:
>>>> On 11/07/14 16:43, Razvan Cojocaru wrote:
>>>>> Vmx_disable_intercept_for_msr() will now refuse to disable interception of
>>>>> MSRs needed for memory introspection. It is not possible to gate this on
>>>>> mem_access being active for the domain, since by the time mem_access does
>>>>> become active the interception for the interesting MSRs has already been
>>>>> disabled (vmx_disable_intercept_for_msr() runs very early on).
>>>>>
>>>>> Changes since V1:
>>>>>  - Replaced printk() with gdprintk(XENLOG_DEBUG, ...).
>>>>>
>>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>>>> ---
>>>>>  xen/arch/x86/hvm/vmx/vmcs.c |   18 ++++++++++++++++++
>>>>>  1 file changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
>>>>> index 8ffc562..35fcfcc 100644
>>>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>>>> @@ -700,6 +700,24 @@ void vmx_disable_intercept_for_msr(struct vcpu *v, u32 msr, int type)
>>>>>      if ( msr_bitmap == NULL )
>>>>>          return;
>>>>>  
>>>>> +    /* Filter out MSR-s needed for memory introspection */
>>>>> +    switch ( msr )
>>>> This absolutely must be gated on mem_events being enabled for the domain.
>>>>
>>>> It is too much of a performance penalty to apply to domains which are
>>>> not being introspected.
>>> I understand, but it really runs very early on, and the mem_event part
>>> comes in after the MSR interception is disabled. This effectively
>>> renders a lot of memory introspection functionality useless.
>> In which case the hypercall which enables mem_event needs to prod the
>> vmcs state an explicitly enable intercepts for these MSRs. (and
>> conversly re-disables intercepts if mem_events are disabled)
>>
>> You can probably get away with hvm_funcs to enable and disable mem events.
> That makes sense. Would Aravindh's solution also be acceptable to you
> (using a Xen command line parameter and gate it based on that rather
> than a mem_event listener being present)?
>
>
> Thanks,
> Razvan Cojocaru

Its not a question of acceptable to me.  I am not a maintainer or
committer.  I am merely offering an opinion as a member of the Xen
community.

The people you ultimately need to convince are the Intel VT-x
maintainers (who I notice are still missed off the CC list - please use
/scripts/get_maintainers.pl to identify all the people you need to CC
based on code areas your patches touch) and the general x86 maintainers.

In this case, it would be better to dynamically change the interceptions
based on the use of mem_event, rather than having someone wanting to use
mem_events have to apply the same performance penalty to all their other
VMs if they want it to actually work.

~Andrew