Re: [RFC V0 PATCH 1/1] Replace handle_mmio calls in svm/vmx

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/08/14 02:15, Mukesh Rathor wrote:
> svm/vmx IO and CR intercepts call handle_mmio to emulate which
> is slightly inappropriate.  Create hvm_emulate() and replace those
> calls with hvm_emulate().
>
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>

For cr/dr operations, this looks plausible (although I feel
"hvm_emulate()" is too generic for a name).

However, for ins/outs, this will break HVM.  ins/outs need to get to the
pio handler in Xen, and for HVM, also to Qemu.  Amongst other things,
our windows PV drivers use outs for efficient logging to the qemu debug
port.

~Andrew

> ---
>  xen/arch/x86/hvm/emulate.c        | 26 ++++++++++++++++++++++++++
>  xen/arch/x86/hvm/svm/svm.c        | 12 ++++++------
>  xen/arch/x86/hvm/vmx/vmx.c        | 10 +++-------
>  xen/include/asm-x86/hvm/emulate.h |  1 +
>  4 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index eac159f..c568196 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1252,6 +1252,32 @@ void hvm_emulate_prepare(
>      hvmemul_get_seg_reg(x86_seg_ss, hvmemul_ctxt);
>  }
>  
> +void hvm_emulate(struct cpu_user_regs *regs)
> +{
> +    int rc;
> +    struct hvm_emulate_ctxt ctxt;
> +    
> +    hvm_emulate_prepare(&ctxt, regs);
> +    rc = hvm_emulate_one(&ctxt);
> +    
> +    switch ( rc )
> +    {
> +    case X86EMUL_UNHANDLEABLE:
> +        hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        break;
> +    case X86EMUL_EXCEPTION:
> +    {
> +        uint8_t vector = ctxt.exn_pending ? ctxt.exn_vector : TRAP_gp_fault;
> +        int32_t errcode = ctxt.exn_pending ? ctxt.exn_error_code : 0;
> +        hvm_inject_hw_exception(vector, errcode);
> +        /* fall thru */
> +    }
> +    default:
> +        hvm_emulate_writeback(&ctxt);
> +        break;
> +    }
> +}       
> +
>  void hvm_emulate_writeback(
>      struct hvm_emulate_ctxt *hvmemul_ctxt)
>  {
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 89df9f7..6c24ad2 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2475,16 +2475,16 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>              if ( handle_pio(port, bytes, dir) )
>                  __update_guest_eip(regs, vmcb->exitinfo2 - vmcb->rip);
>          }
> -        else if ( !handle_mmio() )
> -            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        else 
> +            hvm_emulate(regs);
>          break;
>  
>      case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
>      case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
>          if ( cpu_has_svm_decode && (vmcb->exitinfo1 & (1ULL << 63)) )
>              svm_vmexit_do_cr_access(vmcb, regs);
> -        else if ( !handle_mmio() ) 
> -            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        else
> +            hvm_emulate(regs);
>          break;
>  
>      case VMEXIT_INVLPG:
> @@ -2493,8 +2493,8 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>              svm_invlpg_intercept(vmcb->exitinfo1);
>              __update_guest_eip(regs, vmcb->nextrip - vmcb->rip);
>          }
> -        else if ( !handle_mmio() )
> -            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        else
> +            hvm_emulate(regs);
>          break;
>  
>      case VMEXIT_INVLPGA:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index dd969dc..415b73e 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3008,8 +3008,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>          break;
>  
>      case EXIT_REASON_APIC_ACCESS:
> -        if ( !vmx_handle_eoi_write() && !handle_mmio() )
> -            hvm_inject_hw_exception(TRAP_gp_fault, 0);
> +        if ( !vmx_handle_eoi_write() )
> +            hvm_emulate(regs);
>          break;
>  
>      case EXIT_REASON_EOI_INDUCED:
> @@ -3026,11 +3026,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>      case EXIT_REASON_IO_INSTRUCTION:
>          __vmread(EXIT_QUALIFICATION, &exit_qualification);
>          if ( exit_qualification & 0x10 )
> -        {
> -            /* INS, OUTS */
> -            if ( !handle_mmio() )
> -                hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -        }
> +            hvm_emulate(regs);   /* INS, OUTS */
>          else
>          {
>              /* IN, OUT */
> diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h
> index 00a06cc..bc4a249 100644
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -35,6 +35,7 @@ struct hvm_emulate_ctxt {
>      uint32_t intr_shadow;
>  };
>  
> +void hvm_emulate(struct cpu_user_regs *regs);
>  int hvm_emulate_one(
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
>  void hvm_emulate_prepare(