Re: [RFC PATCH V3 10/12] xen: Introduce monitor_op domctl

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 01/29/2015 11:46 PM, Tamas K Lengyel wrote:
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index c7a0bde..3b58700 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1445,15 +1445,6 @@ void p2m_vm_event_emulate_check(struct vcpu *v, const vm_event_response_t *rsp)
>      }
>  }
>  
> -void p2m_setup_introspection(struct domain *d)
> -{
> -    if ( hvm_funcs.enable_msr_exit_interception )
> -    {
> -        d->arch.hvm_domain.introspection_enabled = 1;
> -        hvm_funcs.enable_msr_exit_interception(d);
> -    }
> -}
> -
>  bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>                              struct npfec npfec,
>                              vm_event_request_t **req_ptr)

I see that introspection_enabled is no longer assigned here ...

> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 0db899e..0b30750 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -617,16 +617,10 @@ int vm_event_domctl(struct domain *d, xen_domctl_vm_event_op_t *vec,
>          switch( vec->op )
>          {
>          case XEN_DOMCTL_VM_EVENT_OP_MONITOR_ENABLE:
> -        case XEN_DOMCTL_VM_EVENT_OP_MONITOR_ENABLE_INTROSPECTION:
>          {
>              rc = vm_event_enable(d, vec, ved, _VPF_mem_access,
>                                      HVM_PARAM_MONITOR_RING_PFN,
>                                      mem_access_notification);
> -
> -            if ( vec->op == XEN_DOMCTL_VM_EVENT_OP_MONITOR_ENABLE_INTROSPECTION
> -                 && !rc )
> -                p2m_setup_introspection(d);
> -
>          }
>          break;
>  
> @@ -635,7 +629,6 @@ int vm_event_domctl(struct domain *d, xen_domctl_vm_event_op_t *vec,
>              if ( ved->ring_page )
>              {
>                  rc = vm_event_disable(d, ved);
> -                d->arch.hvm_domain.introspection_enabled = 0;
>              }
>          }
>          break;

... nor here. Patch 6/12 checks it but doesn't set it. Patch 5/12 sets
it to 0 (which could account for the removal of the assignment in
vm_event.c) but never to 1. A few important things depend on it being
enabled: it becomes impossible to disable interception for a select set
of MSRs, optimization for RET instructions emulation is disabled, and
othere places in p2m.c makes use of the flag as well.

Is there some place in the code, untouched by this series, where
introspection_enabled is being set to 1?

> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index 8fba3d1b..a057608 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -67,7 +67,7 @@
>  #define VM_EVENT_REASON_MOV_TO_CR3              5
>  /* CR4 was updated */
>  #define VM_EVENT_REASON_MOV_TO_CR4              6
> -/* An MSR was updated. Does NOT honour HVMPME_onchangeonly */
> +/* An MSR was updated. */
>  #define VM_EVENT_REASON_MOV_TO_MSR              9
>  /* Debug operation executed (int3) */
>  #define VM_EVENT_REASON_SOFTWARE_BREAKPOINT     7
> 

Does VM_EVENT_REASON_MOV_TO_MSR now honour HVMPME_onchangeonly?


Thanks,
Razvan