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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.03.17 at 16:50, <itopan@xxxxxxxxxxxxxxx> 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(&ctxt, NULL, guest_cpu_user_regs());
> +        rc = hvm_emulate_one(&ctxt);
> +        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, &exit_qualification);
> +    __vmread(VMX_INSTRUCTION_INFO, &instr_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_INVALID        0

What is this good for?

Jan


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