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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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 <itopan@xxxxxxxxxxxxxxx>

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(&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);
> +            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(&ctxt.ctxt.event);
> +            /* fall through */
> +        default:
> +            hvm_emulate_writeback(&ctxt);
> +            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, &exit_qualification);
> +    __vmread(VMX_INSTRUCTION_INFO, &instr_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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel