Re: [PATCH 3/6] x86/hvm: Don't discard the SW/HW event distinction from the emulator

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 09/23/2014 11:03 AM, Andrew Cooper wrote:
> Injecting emulator software events as hardware exceptions results in a bypass
> of DPL checks.  As the emulator doesn't perform DPL checks itself, guest
> userspace is capable of bypassing DPL checks and injecting arbitrary events.
>
> Propagating software event information from the emulator allows VMX to now
> properly inject software events, including DPL and presence checks, as well
> correct fault/trap frames.
>
> Reported-by: Andrei LUTAS <vlutas@bitdefender.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Tested-by: Andrei LUTAS <vlutas@bitdefender.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Eddie Dong <eddie.dong@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

> ---
>   xen/arch/x86/hvm/emulate.c        |   41 ++++++++++++++++++++++++++++---------
>   xen/arch/x86/hvm/io.c             |    2 +-
>   xen/arch/x86/hvm/svm/svm.c        |    2 +-
>   xen/arch/x86/hvm/vmx/realmode.c   |   14 ++++++-------
>   xen/arch/x86/hvm/vmx/vmx.c        |    2 +-
>   xen/include/asm-x86/hvm/emulate.h |    5 ++---
>   6 files changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 5d5d765..7ee146b 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -441,9 +441,10 @@ static int hvmemul_virtual_to_linear(
>   
>       /* This is a singleton operation: fail it with an exception. */
>       hvmemul_ctxt->exn_pending = 1;
> -    hvmemul_ctxt->exn_vector = TRAP_gp_fault;
> -    hvmemul_ctxt->exn_error_code = 0;
> -    hvmemul_ctxt->exn_insn_len = 0;
> +    hvmemul_ctxt->trap.vector = TRAP_gp_fault;
> +    hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> +    hvmemul_ctxt->trap.error_code = 0;
> +    hvmemul_ctxt->trap.insn_len = 0;
>       return X86EMUL_EXCEPTION;
>   }
>   
> @@ -1111,9 +1112,10 @@ static int hvmemul_inject_hw_exception(
>           container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>   
>       hvmemul_ctxt->exn_pending = 1;
> -    hvmemul_ctxt->exn_vector = vector;
> -    hvmemul_ctxt->exn_error_code = error_code;
> -    hvmemul_ctxt->exn_insn_len = 0;
> +    hvmemul_ctxt->trap.vector = vector;
> +    hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> +    hvmemul_ctxt->trap.error_code = error_code;
> +    hvmemul_ctxt->trap.insn_len = 0;
>   
>       return X86EMUL_OKAY;
>   }
> @@ -1127,10 +1129,29 @@ static int hvmemul_inject_sw_interrupt(
>       struct hvm_emulate_ctxt *hvmemul_ctxt =
>           container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>   
> +    switch ( type )
> +    {
> +    case x86_swint_icebp:
> +        hvmemul_ctxt->trap.type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
> +        break;
> +
> +    case x86_swint_int3:
> +    case x86_swint_into:
> +        hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_EXCEPTION;
> +        break;
> +
> +    case x86_swint_int:
> +        hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_INTERRUPT;
> +        break;
> +
> +    default:
> +        return X86EMUL_UNHANDLEABLE;
> +    }
> +
>       hvmemul_ctxt->exn_pending = 1;
> -    hvmemul_ctxt->exn_vector = vector;
> -    hvmemul_ctxt->exn_error_code = -1;
> -    hvmemul_ctxt->exn_insn_len = insn_len;
> +    hvmemul_ctxt->trap.vector = vector;
> +    hvmemul_ctxt->trap.error_code = HVM_DELIVER_NO_ERROR_CODE;
> +    hvmemul_ctxt->trap.insn_len = insn_len;
>   
>       return X86EMUL_OKAY;
>   }
> @@ -1404,7 +1425,7 @@ void hvm_mem_event_emulate_one(bool_t nowrite, unsigned int trapnr,
>           break;
>       case X86EMUL_EXCEPTION:
>           if ( ctx.exn_pending )
> -            hvm_inject_hw_exception(ctx.exn_vector, ctx.exn_error_code);
> +            hvm_inject_trap(&ctx.trap);
>           break;
>       }
>   
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 9f565d6..e5d5e79 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -113,7 +113,7 @@ int handle_mmio(void)
>           return 0;
>       case X86EMUL_EXCEPTION:
>           if ( ctxt.exn_pending )
> -            hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code);
> +            hvm_inject_trap(&ctxt.trap);
>           break;
>       default:
>           break;
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 5d404ce..de982fd 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2080,7 +2080,7 @@ static void svm_vmexit_ud_intercept(struct cpu_user_regs *regs)
>           break;
>       case X86EMUL_EXCEPTION:
>           if ( ctxt.exn_pending )
> -            hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code);
> +            hvm_inject_trap(&ctxt.trap);
>           /* fall through */
>       default:
>           hvm_emulate_writeback(&ctxt);
> diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
> index 45066b2..9a6de6c 100644
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -129,27 +129,27 @@ static void realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt)
>                   gdprintk(XENLOG_ERR, "Exception pending but no info.\n");
>                   goto fail;
>               }
> -            hvmemul_ctxt->exn_vector = (uint8_t)intr_info;
> -            hvmemul_ctxt->exn_insn_len = 0;
> +            hvmemul_ctxt->trap.vector = (uint8_t)intr_info;
> +            hvmemul_ctxt->trap.insn_len = 0;
>           }
>   
>           if ( unlikely(curr->domain->debugger_attached) &&
> -             ((hvmemul_ctxt->exn_vector == TRAP_debug) ||
> -              (hvmemul_ctxt->exn_vector == TRAP_int3)) )
> +             ((hvmemul_ctxt->trap.vector == TRAP_debug) ||
> +              (hvmemul_ctxt->trap.vector == TRAP_int3)) )
>           {
>               domain_pause_for_debugger();
>           }
>           else if ( curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE )
>           {
>               gdprintk(XENLOG_ERR, "Exception %02x in protected mode.\n",
> -                     hvmemul_ctxt->exn_vector);
> +                     hvmemul_ctxt->trap.vector);
>               goto fail;
>           }
>           else
>           {
>               realmode_deliver_exception(
> -                hvmemul_ctxt->exn_vector,
> -                hvmemul_ctxt->exn_insn_len,
> +                hvmemul_ctxt->trap.vector,
> +                hvmemul_ctxt->trap.insn_len,
>                   hvmemul_ctxt);
>           }
>       }
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 84119ed..addaa81 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2510,7 +2510,7 @@ static void vmx_vmexit_ud_intercept(struct cpu_user_regs *regs)
>           break;
>       case X86EMUL_EXCEPTION:
>           if ( ctxt.exn_pending )
> -            hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code);
> +            hvm_inject_trap(&ctxt.trap);
>           /* fall through */
>       default:
>           hvm_emulate_writeback(&ctxt);
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
> index efff97e..6cdc57b 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -13,6 +13,7 @@
>   #define __ASM_X86_HVM_EMULATE_H__
>   
>   #include <xen/config.h>
> +#include <asm/hvm/hvm.h>
>   #include <asm/x86_emulate.h>
>   
>   struct hvm_emulate_ctxt {
> @@ -28,9 +29,7 @@ struct hvm_emulate_ctxt {
>       unsigned long seg_reg_dirty;
>   
>       bool_t exn_pending;
> -    uint8_t exn_vector;
> -    uint8_t exn_insn_len;
> -    int32_t exn_error_code;
> +    struct hvm_trap trap;
>   
>       uint32_t intr_shadow;
>   };