[PATCH] xen: Filter out MSR write events

[PATCH] xen: Filter out MSR write events

[Razvan Cojocaru]

This patch only allows introspection-related MSR write events to
be sent out, improving performance. Should additional events be
required, they can then simply be added to the list of
vmx_introspection_force_enabled_msrs[].

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/x86/hvm/hvm.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24126d..21ba611 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3688,6 +3688,17 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
     goto out;
 }
 
+static bool_t is_introspection_msr(unsigned int msr)
+{
+    unsigned int i;
+
+    for ( i = 0; i < vmx_introspection_force_enabled_msrs_size; i++ )
+        if ( msr == vmx_introspection_force_enabled_msrs[i] )
+            return 1;
+
+    return 0;
+}
+
 int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
                             bool_t may_defer)
 {
@@ -3703,7 +3714,8 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
     hvm_cpuid(1, NULL, NULL, NULL, &edx);
     mtrr = !!(edx & cpufeat_mask(X86_FEATURE_MTRR));
 
-    if ( may_defer && unlikely(currad->monitor.mov_to_msr_enabled) )
+    if ( may_defer && unlikely(currad->monitor.mov_to_msr_enabled) &&
+         is_introspection_msr(msr) )
     {
         ASSERT(v->arch.vm_event);
 
-- 
2.8.0


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

Re: [PATCH] xen: Filter out MSR write events

[Konrad Rzeszutek Wilk]

On Mon, Apr 11, 2016 at 07:41:54PM +0300, Razvan Cojocaru wrote:
> This patch only allows introspection-related MSR write events to
> be sent out, improving performance. Should additional events be
> required, they can then simply be added to the list of
> vmx_introspection_force_enabled_msrs[].
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thought should there be some .. dynamic mechanism to update
the MSR list? Or remove entries (or temporarily blacklist
the built-one ins), etc?

> ---
>  xen/arch/x86/hvm/hvm.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index f24126d..21ba611 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3688,6 +3688,17 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
>      goto out;
>  }
>  
> +static bool_t is_introspection_msr(unsigned int msr)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; i < vmx_introspection_force_enabled_msrs_size; i++ )
> +        if ( msr == vmx_introspection_force_enabled_msrs[i] )
> +            return 1;
> +
> +    return 0;
> +}
> +
>  int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
>                              bool_t may_defer)
>  {
> @@ -3703,7 +3714,8 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
>      hvm_cpuid(1, NULL, NULL, NULL, &edx);
>      mtrr = !!(edx & cpufeat_mask(X86_FEATURE_MTRR));
>  
> -    if ( may_defer && unlikely(currad->monitor.mov_to_msr_enabled) )
> +    if ( may_defer && unlikely(currad->monitor.mov_to_msr_enabled) &&
> +         is_introspection_msr(msr) )
>      {
>          ASSERT(v->arch.vm_event);
>  
> -- 
> 2.8.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [PATCH] xen: Filter out MSR write events

[Razvan Cojocaru]

On 04/11/16 22:18, Konrad Rzeszutek Wilk wrote:
> On Mon, Apr 11, 2016 at 07:41:54PM +0300, Razvan Cojocaru wrote:
>> This patch only allows introspection-related MSR write events to
>> be sent out, improving performance. Should additional events be
>> required, they can then simply be added to the list of
>> vmx_introspection_force_enabled_msrs[].
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Thought should there be some .. dynamic mechanism to update
> the MSR list? Or remove entries (or temporarily blacklist
> the built-one ins), etc?

While this should be enough for now (the introspection MSR list is very
small, and we're probably the only consumers), that's indeed the way for
the future, I think. We should have a set-like container of unique
elements that can grow and shrink and can be searched quickly, and add /
remove MSRs we're interested in there.

The CR write events bitmap approach clearly doesn't work, as there are
too many MSRs to be able to select them all with a bitmap in the future.


Thanks,
Razvan

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

Re: [PATCH] xen: Filter out MSR write events

[Andrew Cooper]

On 12/04/16 05:26, Razvan Cojocaru wrote:
> On 04/11/16 22:18, Konrad Rzeszutek Wilk wrote:
>> On Mon, Apr 11, 2016 at 07:41:54PM +0300, Razvan Cojocaru wrote:
>>> This patch only allows introspection-related MSR write events to
>>> be sent out, improving performance. Should additional events be
>>> required, they can then simply be added to the list of
>>> vmx_introspection_force_enabled_msrs[].
>>>
>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>> Thought should there be some .. dynamic mechanism to update
>> the MSR list? Or remove entries (or temporarily blacklist
>> the built-one ins), etc?
> While this should be enough for now (the introspection MSR list is very
> small, and we're probably the only consumers), that's indeed the way for
> the future, I think. We should have a set-like container of unique
> elements that can grow and shrink and can be searched quickly, and add /
> remove MSRs we're interested in there.
>
> The CR write events bitmap approach clearly doesn't work, as there are
> too many MSRs to be able to select them all with a bitmap in the future.

The current intercept bitmaps have 4x1K bitmaps, a read and a write bit
for the low and high MSR indices.

For introspection purposes, you will also want a bitmap covering the
hypervisor range.  For now, 3x1K bitmaps should be plenty to cover all
potentially interesting MSRs.

~Andrew

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

Re: [PATCH] xen: Filter out MSR write events

[Razvan Cojocaru]

On 04/12/2016 12:38 PM, Andrew Cooper wrote:
> On 12/04/16 05:26, Razvan Cojocaru wrote:
>> On 04/11/16 22:18, Konrad Rzeszutek Wilk wrote:
>>> On Mon, Apr 11, 2016 at 07:41:54PM +0300, Razvan Cojocaru wrote:
>>>> This patch only allows introspection-related MSR write events to
>>>> be sent out, improving performance. Should additional events be
>>>> required, they can then simply be added to the list of
>>>> vmx_introspection_force_enabled_msrs[].
>>>>
>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>
>>> Thought should there be some .. dynamic mechanism to update
>>> the MSR list? Or remove entries (or temporarily blacklist
>>> the built-one ins), etc?
>> While this should be enough for now (the introspection MSR list is very
>> small, and we're probably the only consumers), that's indeed the way for
>> the future, I think. We should have a set-like container of unique
>> elements that can grow and shrink and can be searched quickly, and add /
>> remove MSRs we're interested in there.
>>
>> The CR write events bitmap approach clearly doesn't work, as there are
>> too many MSRs to be able to select them all with a bitmap in the future.
> 
> The current intercept bitmaps have 4x1K bitmaps, a read and a write bit
> for the low and high MSR indices.
> 
> For introspection purposes, you will also want a bitmap covering the
> hypervisor range.  For now, 3x1K bitmaps should be plenty to cover all
> potentially interesting MSRs.

I see, so for future reference, add a pointer member allocated with
alloc_xenheap_page() and memset() to 0 to struct arch_domain, maybe
named monitor_msrs_bitmap (while removing mov_to_msr_enabled and
mov_to_msr_extended from struct monitor)? This could get allocated on
vm_event_init_domain() and de-allocated on vm_event_cleanup_domain().


Thanks,
Razvan

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

Re: [PATCH] xen: Filter out MSR write events

[Andrew Cooper]

On 12/04/16 11:17, Razvan Cojocaru wrote:
> On 04/12/2016 12:38 PM, Andrew Cooper wrote:
>> On 12/04/16 05:26, Razvan Cojocaru wrote:
>>> On 04/11/16 22:18, Konrad Rzeszutek Wilk wrote:
>>>> On Mon, Apr 11, 2016 at 07:41:54PM +0300, Razvan Cojocaru wrote:
>>>>> This patch only allows introspection-related MSR write events to
>>>>> be sent out, improving performance. Should additional events be
>>>>> required, they can then simply be added to the list of
>>>>> vmx_introspection_force_enabled_msrs[].
>>>>>
>>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>>
>>>> Thought should there be some .. dynamic mechanism to update
>>>> the MSR list? Or remove entries (or temporarily blacklist
>>>> the built-one ins), etc?
>>> While this should be enough for now (the introspection MSR list is very
>>> small, and we're probably the only consumers), that's indeed the way for
>>> the future, I think. We should have a set-like container of unique
>>> elements that can grow and shrink and can be searched quickly, and add /
>>> remove MSRs we're interested in there.
>>>
>>> The CR write events bitmap approach clearly doesn't work, as there are
>>> too many MSRs to be able to select them all with a bitmap in the future.
>> The current intercept bitmaps have 4x1K bitmaps, a read and a write bit
>> for the low and high MSR indices.
>>
>> For introspection purposes, you will also want a bitmap covering the
>> hypervisor range.  For now, 3x1K bitmaps should be plenty to cover all
>> potentially interesting MSRs.
> I see, so for future reference, add a pointer member allocated with
> alloc_xenheap_page() and memset() to 0 to struct arch_domain, maybe
> named monitor_msrs_bitmap (while removing mov_to_msr_enabled and
> mov_to_msr_extended from struct monitor)? This could get allocated on
> vm_event_init_domain() and de-allocated on vm_event_cleanup_domain().

Looks suitable.

~Andrew

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