Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-28 Thread Vlad-Ioan TOPAN 
On Tue, 21 Mar 2017 12:04:02 +
Andrew Cooper  wrote:

> On 10/03/17 15:50, Vlad Ioan Topan wrote:
> > Adds monitor support for descriptor access events (reads & writes of
> > IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM).
> >
> > Signed-off-by: Vlad Ioan Topan 
> 
> How much extra overhead does this typically give?  (I am curious, more
> than anything else)

Without a monitor attached, none at all; with a monitor attached and
with exits activated, there are probably a few hundred exits during
the OS startup process; during normal OS runtime there should be no
descriptor register access.

The other suggestions will be incorporated by my colleagues who will
take over the patch(es).

Thank you,
--
Vlad-Ioan TOPAN
Linux Kernel Development Lead
Bitdefender

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-21 Thread Andrew Cooper 
On 10/03/17 15:50, Vlad Ioan Topan wrote:
> Adds monitor support for descriptor access events (reads & writes of
> IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM).
>
> Signed-off-by: Vlad Ioan Topan 

How much extra overhead does this typically give?  (I am curious, more
than anything else)

> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ccfae4f..cfe5aa2 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3645,6 +3645,41 @@ gp_fault:
>  return X86EMUL_EXCEPTION;
>  }
>  
> +int hvm_descriptor_access_intercept(uint64_t exit_info, uint64_t 
> exit_qualification, 
> +uint8_t descriptor, bool_t is_write)
> +{
> +struct vcpu *v = current;
> +struct domain *d = v->domain;
> +struct hvm_emulate_ctxt ctxt = {};
> +int rc;
> +
> +if ( d->arch.monitor.descriptor_access_enabled )
> +{
> +ASSERT(v->arch.vm_event);
> +hvm_monitor_descriptor_access(exit_info, exit_qualification, 
> descriptor, is_write);
> +}
> +else
> +{
> +hvm_emulate_init_once(, NULL, guest_cpu_user_regs());
> +rc = hvm_emulate_one();
> +switch ( rc )
> +{
> +case X86EMUL_UNHANDLEABLE:
> +hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
> +break;
> +case X86EMUL_EXCEPTION:
> +if ( ctxt.ctxt.event_pending )

You can drop this if().  The expected behaviour of x86_emulate() makes
this true, and we now have an assertion to catch it being wrong.  (I
should update other areas of code).

> +hvm_inject_event();
> +/* fall through */
> +default:
> +hvm_emulate_writeback();
> +break;
> +}
> +}
> +
> +return X86EMUL_OKAY;
> +}
> +
>  static bool is_cross_vendor(const struct x86_emulate_state *state,
>  const struct x86_emulate_ctxt *ctxt)
>  {
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 894d7d4..2b2d193 100644
> @@ -3369,6 +3384,33 @@ static void vmx_handle_xrstors(void)
>  domain_crash(current->domain);
>  }
>  
> +static void vmx_handle_descriptor_access(uint32_t exit_reason)
> +{
> +uint8_t instr_id;
> +uint64_t instr_info;
> +uint64_t exit_qualification;
> +uint8_t descriptor = VM_EVENT_DESC_INVALID;
> +
> +__vmread(EXIT_QUALIFICATION, _qualification);
> +__vmread(VMX_INSTRUCTION_INFO, _info);

Rather than all this hand decoding, can I ask you to introduce a
structure like ept_qual_t?

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-21 Thread Jan Beulich 
>>> On 20.03.17 at 20:21,  wrote:
> On Fri, 17 Mar 2017 05:03:44 -0600
> "Jan Beulich"  wrote:
>> >>> On 10.03.17 at 16:50,  wrote:
>> > +else
>> > +{
>> > +hvm_emulate_init_once(, NULL, guest_cpu_user_regs());
>> > +rc = hvm_emulate_one();
>> > +switch ( rc )
>> > +{
>> > +case X86EMUL_UNHANDLEABLE:
>> > +hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
>> 
>> Is #UD the right exception here? In fact, is delivering an exception
>> sensible in this case at all?
> 
> Possibly not; it's how that case is handled elsewhere. For the given
> set of instructions at least, X86EMUL_UNHANDLEABLE should only be
> returnable due to some internal bug/error (e.g. invalid/unknown
> HVMCOPY_* constant returned by hvm_fetch_from_guest_linear() or an
> invalid selector passed to ->write_segment() etc.); what would be the
> best way to handle that case?

I think crashing the domain in such cases is better - injecting
whatever non-architectural exception is only going to defer
problems, making later analysis more difficult.

>> > +hvm_descriptor_access_intercept(instr_info, exit_qualification, 
>> > descriptor,
>> > +instr_id >= 2);
>> 
>> instr_id & 2 (to be consistent with the other code. But anyway, even
>> better would be to use manifest constants instead of all these literal
>> numbers.
> 
> (instr_id & 2) makes sense, but the 1 & 2 literals are unnamed criteria
> inferred from the encoding to simplify the code, they don't have an
> explicit meaning (at least in the Intel docs).

I don't follow - if there are no names, you're free to give them
names. I'm sure you and the VMX maintainers can agree on
something suitable.

>> > @@ -259,6 +261,24 @@ struct vm_event_mov_to_msr {
>> >  uint64_t value;
>> >  };
>> >  
>> > +#define VM_EVENT_DESC_INVALID0
>> 
>> What is this good for?
> 
> The default (uninitialized) value is given a semantic of "invalid" to
> make potential problems due to incorrectly / incompletely initialized
> or corrupted data more obvious

No need to give it a name though, afaics - just start valid values
at 1.

> (I assume the .pad* fields are checked to
> be 0 for the same reason).

No, they're being checked so that later assigning them a meaning
will be possible without breaking backwards compatibility.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-20 Thread Vlad-Ioan TOPAN 
On Fri, 17 Mar 2017 05:03:44 -0600
"Jan Beulich"  wrote:

> >>> On 10.03.17 at 16:50,  wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -3645,6 +3645,41 @@ gp_fault:
> >  return X86EMUL_EXCEPTION;
> >  }
> >  
> > +int hvm_descriptor_access_intercept(uint64_t exit_info, uint64_t 
> > exit_qualification, 
> > +uint8_t descriptor, bool_t is_write)
> 
> bool (also elsewhere)

Ok.

> > +{
> > +struct vcpu *v = current;
> > +struct domain *d = v->domain;
> 
> curr and currd respectively.

Ok.

> > +struct hvm_emulate_ctxt ctxt = {};
> 
> Please move this into the more narrow scope it's needed in.

Ok.

> > +int rc;
> > +
> > +if ( d->arch.monitor.descriptor_access_enabled )
> > +{
> > +ASSERT(v->arch.vm_event);
> > +hvm_monitor_descriptor_access(exit_info, exit_qualification, 
> > descriptor, is_write);
> > +}
> > +else
> > +{
> > +hvm_emulate_init_once(, NULL, guest_cpu_user_regs());
> > +rc = hvm_emulate_one();
> > +switch ( rc )
> > +{
> > +case X86EMUL_UNHANDLEABLE:
> > +hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
> 
> Is #UD the right exception here? In fact, is delivering an exception
> sensible in this case at all?

Possibly not; it's how that case is handled elsewhere. For the given
set of instructions at least, X86EMUL_UNHANDLEABLE should only be
returnable due to some internal bug/error (e.g. invalid/unknown
HVMCOPY_* constant returned by hvm_fetch_from_guest_linear() or an
invalid selector passed to ->write_segment() etc.); what would be the
best way to handle that case?

> > @@ -3369,6 +3384,33 @@ static void vmx_handle_xrstors(void)
> >  domain_crash(current->domain);
> >  }
> >  
> > +static void vmx_handle_descriptor_access(uint32_t exit_reason)
> > +{
> > +uint8_t instr_id;
> > +uint64_t instr_info;
> > +uint64_t exit_qualification;
> > +uint8_t descriptor = VM_EVENT_DESC_INVALID;
> > +
> > +__vmread(EXIT_QUALIFICATION, _qualification);
> > +__vmread(VMX_INSTRUCTION_INFO, _info);
> > +
> > +instr_id = (instr_info >> 28) & 3; /* Instruction identity: bits 29:28 
> > */
> > +
> > +if ( exit_reason == EXIT_REASON_ACCESS_GDTR_OR_IDTR )
> > +if ( instr_id & 1 )
> > +descriptor = VM_EVENT_DESC_IDTR;
> > +else
> > +descriptor = VM_EVENT_DESC_GDTR;
> > +else
> > +if ( instr_id & 1 )
> > +descriptor = VM_EVENT_DESC_TR;
> > +else
> > +descriptor = VM_EVENT_DESC_LDTR;
> 
> Please use conditional expressions instead of if/else in such cases.

Ok.

> > +hvm_descriptor_access_intercept(instr_info, exit_qualification, 
> > descriptor,
> > +instr_id >= 2);
> 
> instr_id & 2 (to be consistent with the other code. But anyway, even
> better would be to use manifest constants instead of all these literal
> numbers.

(instr_id & 2) makes sense, but the 1 & 2 literals are unnamed criteria
inferred from the encoding to simplify the code, they don't have an
explicit meaning (at least in the Intel docs).

> > @@ -444,6 +445,7 @@ int hvm_event_needs_reinjection(uint8_t type, uint8_t 
> > vector);
> >  uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2);
> >  
> >  void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
> > +void hvm_set_descriptor_access_exiting(struct domain *d, bool_t enable);
> 
> Dead declaration.

It somehow got away from a previous (internal) version of the patch; it
will be removed.

> > --- a/xen/include/asm-x86/monitor.h
> > +++ b/xen/include/asm-x86/monitor.h
> > @@ -77,6 +77,7 @@ static inline uint32_t 
> > arch_monitor_get_capabilities(struct domain *d)
> > (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
> > (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> > (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> > +   (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS) |
> > (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT);
> 
> If I was the maintainer of this code, I'd ask for the elements to
> remain ordered numerically, i.e. matching ...
> 
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -1087,6 +1087,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
> >  #define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
> >  #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL   7
> >  #define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
> > +#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS   9
> 
> ... the sequence here.

Ok.

> > @@ -259,6 +261,24 @@ struct vm_event_mov_to_msr {
> >  uint64_t value;
> >  };
> >  
> > +#define VM_EVENT_DESC_INVALID0
> 
> What is this good for?

The default (uninitialized) value is given a semantic of "invalid" to
make potential

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-17 Thread Jan Beulich 
>>> On 10.03.17 at 16:50,  wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3645,6 +3645,41 @@ gp_fault:
>  return X86EMUL_EXCEPTION;
>  }
>  
> +int hvm_descriptor_access_intercept(uint64_t exit_info, uint64_t 
> exit_qualification, 
> +uint8_t descriptor, bool_t is_write)

bool (also elsewhere)

> +{
> +struct vcpu *v = current;
> +struct domain *d = v->domain;

curr and currd respectively.

> +struct hvm_emulate_ctxt ctxt = {};

Please move this into the more narrow scope it's needed in.

> +int rc;
> +
> +if ( d->arch.monitor.descriptor_access_enabled )
> +{
> +ASSERT(v->arch.vm_event);
> +hvm_monitor_descriptor_access(exit_info, exit_qualification, 
> descriptor, is_write);
> +}
> +else
> +{
> +hvm_emulate_init_once(, NULL, guest_cpu_user_regs());
> +rc = hvm_emulate_one();
> +switch ( rc )
> +{
> +case X86EMUL_UNHANDLEABLE:
> +hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);

Is #UD the right exception here? In fact, is delivering an exception
sensible in this case at all?

> @@ -3369,6 +3384,33 @@ static void vmx_handle_xrstors(void)
>  domain_crash(current->domain);
>  }
>  
> +static void vmx_handle_descriptor_access(uint32_t exit_reason)
> +{
> +uint8_t instr_id;
> +uint64_t instr_info;
> +uint64_t exit_qualification;
> +uint8_t descriptor = VM_EVENT_DESC_INVALID;
> +
> +__vmread(EXIT_QUALIFICATION, _qualification);
> +__vmread(VMX_INSTRUCTION_INFO, _info);
> +
> +instr_id = (instr_info >> 28) & 3; /* Instruction identity: bits 29:28 */
> +
> +if ( exit_reason == EXIT_REASON_ACCESS_GDTR_OR_IDTR )
> +if ( instr_id & 1 )
> +descriptor = VM_EVENT_DESC_IDTR;
> +else
> +descriptor = VM_EVENT_DESC_GDTR;
> +else
> +if ( instr_id & 1 )
> +descriptor = VM_EVENT_DESC_TR;
> +else
> +descriptor = VM_EVENT_DESC_LDTR;

Please use conditional expressions instead of if/else in such cases.

> +hvm_descriptor_access_intercept(instr_info, exit_qualification, 
> descriptor,
> +instr_id >= 2);

instr_id & 2 (to be consistent with the other code. But anyway, even
better would be to use manifest constants instead of all these literal
numbers.

> @@ -444,6 +445,7 @@ int hvm_event_needs_reinjection(uint8_t type, uint8_t 
> vector);
>  uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2);
>  
>  void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
> +void hvm_set_descriptor_access_exiting(struct domain *d, bool_t enable);

Dead declaration.

> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -77,6 +77,7 @@ static inline uint32_t arch_monitor_get_capabilities(struct 
> domain *d)
> (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> +   (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS) |
> (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT);

If I was the maintainer of this code, I'd ask for the elements to
remain ordered numerically, i.e. matching ...

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1087,6 +1087,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
>  #define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
>  #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL   7
>  #define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8
> +#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS   9

... the sequence here.

> @@ -259,6 +261,24 @@ struct vm_event_mov_to_msr {
>  uint64_t value;
>  };
>  
> +#define VM_EVENT_DESC_INVALID0

What is this good for?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-16 Thread Vlad-Ioan TOPAN 
On Tue, 14 Mar 2017 09:15:04 -0400
Boris Ostrovsky  wrote:

> 
> 
> On 03/14/2017 08:50 AM, Razvan Cojocaru wrote:
> > On 03/14/2017 02:15 PM, Vlad-Ioan TOPAN wrote:
>  @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs 
>  *regs)
>   case VMEXIT_PAUSE:
>   svm_vmexit_do_pause(regs);
>   break;
>  +
>  +case VMEXIT_IDTR_READ:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_IDTR, 0);
>  +break;
>  +
>  +case VMEXIT_GDTR_READ:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_GDTR, 0);
>  +break;
>  +
>  +case VMEXIT_LDTR_READ:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_LDTR, 0);
>  +break;
>  +
>  +case VMEXIT_TR_READ:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_TR, 0);
>  +break;
>  +
>  +case VMEXIT_IDTR_WRITE:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_IDTR, 1);
>  +break;
>  +
>  +case VMEXIT_GDTR_WRITE:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_GDTR, 1);
>  +break;
>  +
>  +case VMEXIT_LDTR_WRITE:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_LDTR, 1);
>  +break;
>  +
>  +case VMEXIT_TR_WRITE:
>  +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>  VM_EVENT_DESC_TR, 1);
>  +break;
> >>>
> >>> I think this can be halved in size by having
> >>> 'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'
> >>>
> >>> And maybe even collapse completely by having a lookup table mapping exit
> >>> reason to event.
> >>
> >> The problem with both ideas is that they depend on assumptions about the
> >> values of the VMEXIT_* constants to make the code shorter and still
> >> keep it readable, which in my opinion would be bad. Although they will
> >> most likely stay sequential and keep their current numeric values, it's
> >> not something I'd hardcode. Without those assumptions, it's either
> >> another switch or a very long if, which would mean roughly the same
> >> amount of code, but less readable (it's the way I've written it
> >> initally before coming to this version).
> >
> > I'm reading Boris' suggestion to mean:
> >
> > case VMEXIT_IDTR_READ:
> > case VMEXIT_IDTR_WRITE:
> > hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0,
> > VM_EVENT_DESC_IDTR, exit_reason == VMEXIT_IDTR_WRITE);
> > break;
> >
> > I could be wrong.
>
> Right, that's exactly what I meant, thanks.

That's the "roughly same amount of code, but less readable" option, but
it works for me if that's the consensus.

> As for getting rid of all but one cases --- yes, it may be a a bit 
> tricky to do it in a reasonably compact manner.
> 
> -boris

Okay then, I'll post v2 of the patch with the two suggested changes.

--
Vlad-Ioan TOPAN
Linux Kernel Development Lead
Bitdefender

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Jan Beulich 
>>> On 14.03.17 at 13:15,  wrote:
>> > @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>> >  case VMEXIT_PAUSE:
>> >  svm_vmexit_do_pause(regs);
>> >  break;
>> > +
>> > +case VMEXIT_IDTR_READ:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_IDTR, 0);
>> > +break;
>> > +
>> > +case VMEXIT_GDTR_READ:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_GDTR, 0);
>> > +break;
>> > +
>> > +case VMEXIT_LDTR_READ:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_LDTR, 0);
>> > +break;
>> > +
>> > +case VMEXIT_TR_READ:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_TR, 0);
>> > +break;
>> > +
>> > +case VMEXIT_IDTR_WRITE:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_IDTR, 1);
>> > +break;
>> > +
>> > +case VMEXIT_GDTR_WRITE:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_GDTR, 1);
>> > +break;
>> > +
>> > +case VMEXIT_LDTR_WRITE:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_LDTR, 1);
>> > +break;
>> > +
>> > +case VMEXIT_TR_WRITE:
>> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_TR, 1);
>> > +break;
>> 
>> I think this can be halved in size by having
>> 'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'
>> 
>> And maybe even collapse completely by having a lookup table mapping exit
>> reason to event.
> 
> The problem with both ideas is that they depend on assumptions about the
> values of the VMEXIT_* constants to make the code shorter and still
> keep it readable, which in my opinion would be bad.

I see nothing bad here, we make similar assumptions elsewhere.

> Although they will
> most likely stay sequential and keep their current numeric values,

They're sure to stay the same forever, or else everyone's code
will break.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Boris Ostrovsky 



On 03/14/2017 08:50 AM, Razvan Cojocaru wrote:

On 03/14/2017 02:15 PM, Vlad-Ioan TOPAN wrote:

@@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
 case VMEXIT_PAUSE:
 svm_vmexit_do_pause(regs);
 break;
+
+case VMEXIT_IDTR_READ:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_IDTR, 0);
+break;
+
+case VMEXIT_GDTR_READ:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_GDTR, 0);
+break;
+
+case VMEXIT_LDTR_READ:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_LDTR, 0);
+break;
+
+case VMEXIT_TR_READ:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_TR, 0);
+break;
+
+case VMEXIT_IDTR_WRITE:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_IDTR, 1);
+break;
+
+case VMEXIT_GDTR_WRITE:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_GDTR, 1);
+break;
+
+case VMEXIT_LDTR_WRITE:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_LDTR, 1);
+break;
+
+case VMEXIT_TR_WRITE:
+hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
VM_EVENT_DESC_TR, 1);
+break;


I think this can be halved in size by having
'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'

And maybe even collapse completely by having a lookup table mapping exit
reason to event.


The problem with both ideas is that they depend on assumptions about the
values of the VMEXIT_* constants to make the code shorter and still
keep it readable, which in my opinion would be bad. Although they will
most likely stay sequential and keep their current numeric values, it's
not something I'd hardcode. Without those assumptions, it's either
another switch or a very long if, which would mean roughly the same
amount of code, but less readable (it's the way I've written it
initally before coming to this version).


I'm reading Boris' suggestion to mean:

case VMEXIT_IDTR_READ:
case VMEXIT_IDTR_WRITE:
hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0,
VM_EVENT_DESC_IDTR, exit_reason == VMEXIT_IDTR_WRITE);
break;

I could be wrong.



Right, that's exactly what I meant, thanks.

As for getting rid of all but one cases --- yes, it may be a a bit 
tricky to do it in a reasonably compact manner.


-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Razvan Cojocaru 
On 03/14/2017 02:15 PM, Vlad-Ioan TOPAN wrote:
>>> @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>>>  case VMEXIT_PAUSE:
>>>  svm_vmexit_do_pause(regs);
>>>  break;
>>> +
>>> +case VMEXIT_IDTR_READ:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_IDTR, 0);
>>> +break;
>>> +
>>> +case VMEXIT_GDTR_READ:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_GDTR, 0);
>>> +break;
>>> +
>>> +case VMEXIT_LDTR_READ:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_LDTR, 0);
>>> +break;
>>> +
>>> +case VMEXIT_TR_READ:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_TR, 0);
>>> +break;
>>> +
>>> +case VMEXIT_IDTR_WRITE:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_IDTR, 1);
>>> +break;
>>> +
>>> +case VMEXIT_GDTR_WRITE:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_GDTR, 1);
>>> +break;
>>> +
>>> +case VMEXIT_LDTR_WRITE:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_LDTR, 1);
>>> +break;
>>> +
>>> +case VMEXIT_TR_WRITE:
>>> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_TR, 1);
>>> +break;
>>
>> I think this can be halved in size by having
>> 'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'
>>
>> And maybe even collapse completely by having a lookup table mapping exit
>> reason to event.
> 
> The problem with both ideas is that they depend on assumptions about the
> values of the VMEXIT_* constants to make the code shorter and still
> keep it readable, which in my opinion would be bad. Although they will
> most likely stay sequential and keep their current numeric values, it's
> not something I'd hardcode. Without those assumptions, it's either
> another switch or a very long if, which would mean roughly the same
> amount of code, but less readable (it's the way I've written it
> initally before coming to this version).

I'm reading Boris' suggestion to mean:

case VMEXIT_IDTR_READ:
case VMEXIT_IDTR_WRITE:
hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0,
VM_EVENT_DESC_IDTR, exit_reason == VMEXIT_IDTR_WRITE);
break;

I could be wrong.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Wei Liu 
On Fri, Mar 10, 2017 at 05:50:34PM +0200, Vlad Ioan Topan wrote:
> Adds monitor support for descriptor access events (reads & writes of
> IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM).
> 
> Signed-off-by: Vlad Ioan Topan 
> ---
>  tools/libxc/include/xenctrl.h   |  2 ++
>  tools/libxc/xc_monitor.c| 14 +++
>  tools/tests/xen-access/xen-access.c | 29 -

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Vlad-Ioan TOPAN 
> > @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
> >  case VMEXIT_PAUSE:
> >  svm_vmexit_do_pause(regs);
> >  break;
> > +
> > +case VMEXIT_IDTR_READ:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_IDTR, 0);
> > +break;
> > +
> > +case VMEXIT_GDTR_READ:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_GDTR, 0);
> > +break;
> > +
> > +case VMEXIT_LDTR_READ:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_LDTR, 0);
> > +break;
> > +
> > +case VMEXIT_TR_READ:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_TR, 0);
> > +break;
> > +
> > +case VMEXIT_IDTR_WRITE:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_IDTR, 1);
> > +break;
> > +
> > +case VMEXIT_GDTR_WRITE:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_GDTR, 1);
> > +break;
> > +
> > +case VMEXIT_LDTR_WRITE:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_LDTR, 1);
> > +break;
> > +
> > +case VMEXIT_TR_WRITE:
> > +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> > VM_EVENT_DESC_TR, 1);
> > +break;
> 
> I think this can be halved in size by having
> 'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'
> 
> And maybe even collapse completely by having a lookup table mapping exit
> reason to event.

The problem with both ideas is that they depend on assumptions about the
values of the VMEXIT_* constants to make the code shorter and still
keep it readable, which in my opinion would be bad. Although they will
most likely stay sequential and keep their current numeric values, it's
not something I'd hardcode. Without those assumptions, it's either
another switch or a very long if, which would mean roughly the same
amount of code, but less readable (it's the way I've written it
initally before coming to this version).

--
Vlad-Ioan TOPAN
Linux Kernel Development Lead
Bitdefender

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-14 Thread Vlad-Ioan TOPAN 
> > +struct vm_event_desc_access {
> > +union {
> > +uint32_t vmx_instr_info;/* VMX: VMCS Instruction-Information 
> > Field */
> > +uint64_t svm_exitinfo;  /* SVM: VMCB EXITINFO Field */
> > +uint64_t info;
> > +} vmexit;
> > +uint64_t exit_qualification;/* VMX only: VMCS Exit Qualification 
> > Field */
> 
> IMHO it might be a bit cleaner if we had two sub-structs here, one for
> vmx and one for svm, and have them in a union. The vmx struct would
> have vmx_instr_info and exit_qualification, while the svm would only
> have svm_exitinfo.

Makes sense, will do.

--
Vlad-Ioan TOPAN
Linux Kernel Development Lead
Bitdefender

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-13 Thread Boris Ostrovsky 

> @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>  case VMEXIT_PAUSE:
>  svm_vmexit_do_pause(regs);
>  break;
> +
> +case VMEXIT_IDTR_READ:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_IDTR, 0);
> +break;
> +
> +case VMEXIT_GDTR_READ:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_GDTR, 0);
> +break;
> +
> +case VMEXIT_LDTR_READ:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_LDTR, 0);
> +break;
> +
> +case VMEXIT_TR_READ:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_TR, 0);
> +break;
> +
> +case VMEXIT_IDTR_WRITE:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_IDTR, 1);
> +break;
> +
> +case VMEXIT_GDTR_WRITE:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_GDTR, 1);
> +break;
> +
> +case VMEXIT_LDTR_WRITE:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_LDTR, 1);
> +break;
> +
> +case VMEXIT_TR_WRITE:
> +hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
> VM_EVENT_DESC_TR, 1);
> +break;

I think this can be halved in size by having
'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'

And maybe even collapse completely by having a lookup table mapping exit
reason to event.

-boris



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-10 Thread Tamas K Lengyel 
On Fri, Mar 10, 2017 at 8:50 AM, Vlad Ioan Topan  wrote:
> Adds monitor support for descriptor access events (reads & writes of
> IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM).
>
> Signed-off-by: Vlad Ioan Topan 
> ---
>  tools/libxc/include/xenctrl.h   |  2 ++
>  tools/libxc/xc_monitor.c| 14 +++
>  tools/tests/xen-access/xen-access.c | 29 -
>  xen/arch/x86/hvm/hvm.c  | 35 ++
>  xen/arch/x86/hvm/monitor.c  | 16 
>  xen/arch/x86/hvm/svm/svm.c  | 50 
> +
>  xen/arch/x86/hvm/vmx/vmx.c  | 45 +
>  xen/arch/x86/monitor.c  | 18 +
>  xen/arch/x86/vm_event.c |  6 +
>  xen/include/asm-x86/domain.h|  1 +
>  xen/include/asm-x86/hvm/hvm.h   |  2 ++
>  xen/include/asm-x86/hvm/monitor.h   |  3 +++
>  xen/include/asm-x86/hvm/support.h   |  2 ++
>  xen/include/asm-x86/monitor.h   |  1 +
>  xen/include/public/domctl.h |  1 +
>  xen/include/public/vm_event.h   | 21 
>  16 files changed, 245 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index a48981a..2de6c61 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1984,6 +1984,8 @@ int xc_monitor_mov_to_msr(xc_interface *xch, domid_t 
> domain_id, uint32_t msr,
>  int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable);
>  int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
> bool enable);
> +int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
> + bool enable);
>  int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
>   bool enable, bool sync);
>  int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index 15a7c32..f99b6e3 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -129,6 +129,20 @@ int xc_monitor_singlestep(xc_interface *xch, domid_t 
> domain_id,
>  return do_domctl(xch, );
>  }
>
> +int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
> + bool enable)
> +{
> +DECLARE_DOMCTL;
> +
> +domctl.cmd = XEN_DOMCTL_monitor_op;
> +domctl.domain = domain_id;
> +domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
> +: XEN_DOMCTL_MONITOR_OP_DISABLE;
> +domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS;
> +
> +return do_domctl(xch, );
> +}
> +
>  int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool 
> enable,
>   bool sync)
>  {
> diff --git a/tools/tests/xen-access/xen-access.c 
> b/tools/tests/xen-access/xen-access.c
> index 9d4f957..c567cc0 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -337,7 +337,7 @@ void usage(char* progname)
>  {
>  fprintf(stderr, "Usage: %s [-m]  write|exec", progname);
>  #if defined(__i386__) || defined(__x86_64__)
> -fprintf(stderr, 
> "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid");
> +fprintf(stderr, 
> "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access");
>  #elif defined(__arm__) || defined(__aarch64__)
>  fprintf(stderr, "|privcall");
>  #endif
> @@ -368,6 +368,7 @@ int main(int argc, char *argv[])
>  int altp2m = 0;
>  int debug = 0;
>  int cpuid = 0;
> +int desc_access = 0;
>  uint16_t altp2m_view_id = 0;
>
>  char* progname = argv[0];
> @@ -434,6 +435,10 @@ int main(int argc, char *argv[])
>  {
>  cpuid = 1;
>  }
> +else if ( !strcmp(argv[0], "desc_access") )
> +{
> +desc_access = 1;
> +}
>  #elif defined(__arm__) || defined(__aarch64__)
>  else if ( !strcmp(argv[0], "privcall") )
>  {
> @@ -570,6 +575,16 @@ int main(int argc, char *argv[])
>  goto exit;
>  }
>  }
> +
> +if ( desc_access )
> +{
> +rc = xc_monitor_descriptor_access(xch, domain_id, 1);
> +if ( rc < 0 )
> +{
> +ERROR("Error %d setting descriptor access listener with 
> vm_event\n", rc);
> +goto exit;
> +}
> +}
>
>  if ( privcall )
>  {
> @@ -595,6 +610,8 @@ int main(int argc, char *argv[])
>  rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
>  if ( cpuid )
>  rc = xc_monitor_cpuid(xch, domain_id, 0);
> +if ( desc_access )
> +rc = xc_monitor_descriptor_access(xch, domain_id, 0);
>
>  if ( privcall )
>  rc =

[Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

2017-03-10 Thread Vlad Ioan Topan 
Adds monitor support for descriptor access events (reads & writes of
IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM).

Signed-off-by: Vlad Ioan Topan 
---
 tools/libxc/include/xenctrl.h   |  2 ++
 tools/libxc/xc_monitor.c| 14 +++
 tools/tests/xen-access/xen-access.c | 29 -
 xen/arch/x86/hvm/hvm.c  | 35 ++
 xen/arch/x86/hvm/monitor.c  | 16 
 xen/arch/x86/hvm/svm/svm.c  | 50 +
 xen/arch/x86/hvm/vmx/vmx.c  | 45 +
 xen/arch/x86/monitor.c  | 18 +
 xen/arch/x86/vm_event.c |  6 +
 xen/include/asm-x86/domain.h|  1 +
 xen/include/asm-x86/hvm/hvm.h   |  2 ++
 xen/include/asm-x86/hvm/monitor.h   |  3 +++
 xen/include/asm-x86/hvm/support.h   |  2 ++
 xen/include/asm-x86/monitor.h   |  1 +
 xen/include/public/domctl.h |  1 +
 xen/include/public/vm_event.h   | 21 
 16 files changed, 245 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a48981a..2de6c61 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1984,6 +1984,8 @@ int xc_monitor_mov_to_msr(xc_interface *xch, domid_t 
domain_id, uint32_t msr,
 int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable);
 int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
bool enable);
+int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
+ bool enable);
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
  bool enable, bool sync);
 int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 15a7c32..f99b6e3 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -129,6 +129,20 @@ int xc_monitor_singlestep(xc_interface *xch, domid_t 
domain_id,
 return do_domctl(xch, );
 }
 
+int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
+ bool enable)
+{
+DECLARE_DOMCTL;
+
+domctl.cmd = XEN_DOMCTL_monitor_op;
+domctl.domain = domain_id;
+domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+: XEN_DOMCTL_MONITOR_OP_DISABLE;
+domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS;
+
+return do_domctl(xch, );
+}
+
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
  bool sync)
 {
diff --git a/tools/tests/xen-access/xen-access.c 
b/tools/tests/xen-access/xen-access.c
index 9d4f957..c567cc0 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -337,7 +337,7 @@ void usage(char* progname)
 {
 fprintf(stderr, "Usage: %s [-m]  write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-fprintf(stderr, 
"|breakpoint|altp2m_write|altp2m_exec|debug|cpuid");
+fprintf(stderr, 
"|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access");
 #elif defined(__arm__) || defined(__aarch64__)
 fprintf(stderr, "|privcall");
 #endif
@@ -368,6 +368,7 @@ int main(int argc, char *argv[])
 int altp2m = 0;
 int debug = 0;
 int cpuid = 0;
+int desc_access = 0;
 uint16_t altp2m_view_id = 0;
 
 char* progname = argv[0];
@@ -434,6 +435,10 @@ int main(int argc, char *argv[])
 {
 cpuid = 1;
 }
+else if ( !strcmp(argv[0], "desc_access") )
+{
+desc_access = 1;
+}
 #elif defined(__arm__) || defined(__aarch64__)
 else if ( !strcmp(argv[0], "privcall") )
 {
@@ -570,6 +575,16 @@ int main(int argc, char *argv[])
 goto exit;
 }
 }
+
+if ( desc_access )
+{
+rc = xc_monitor_descriptor_access(xch, domain_id, 1);
+if ( rc < 0 )
+{
+ERROR("Error %d setting descriptor access listener with 
vm_event\n", rc);
+goto exit;
+}
+}
 
 if ( privcall )
 {
@@ -595,6 +610,8 @@ int main(int argc, char *argv[])
 rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
 if ( cpuid )
 rc = xc_monitor_cpuid(xch, domain_id, 0);
+if ( desc_access )
+rc = xc_monitor_descriptor_access(xch, domain_id, 0);
 
 if ( privcall )
 rc = xc_monitor_privileged_call(xch, domain_id, 0);
@@ -779,6 +796,16 @@ int main(int argc, char *argv[])
 rsp.data = req.data;
 rsp.data.regs.x86.rip += req.u.cpuid.insn_length;
 break;
+case VM_EVENT_REASON_DESCRIPTOR_ACCESS:
+printf("Descriptor access: