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

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Fri, Jan 30, 2015 at 8:58 AM, Razvan Cojocaru
<rcojocaru@xxxxxxxxxxxxxxx> wrote:
> 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 ...

Introspection_enabled is getting deprecated in this patch and is moved
into the monitor_op domctl.

>
>> 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?

It is moved into the monitor_op domctl when mov_to_msr trapping is
enabled. The reason of having introspection_enabled AFAIU was to
reenable trapping MSR's that were disabled shortly after boot. Thus,
an option field is present in the monitor_op when enabling mov_to_msr
trapping: extended_capture. Let me know if this still achieves the
same effect as before!

>
>> 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?

It still doesn't but the information on what settings are available
for each event are now better described in public/domclt.h.

>
> Thanks,
> Razvan

Cheers,
Tamas

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