Re: [Xen-devel] [PATCH v2 4/4] x86/HVM: don't uniformly report "MMIO" for various forms of failed emulation

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 18 April 2017 11:34
> To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>;
> Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jun Nakajima
> <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; Boris
> Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Subject: [PATCH v2 4/4] x86/HVM: don't uniformly report "MMIO" for
> various forms of failed emulation
> 
> This helps distinguishing the call paths leading there.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> 

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2033,7 +2033,7 @@ int hvm_emulate_one_mmio(unsigned long m
>      switch ( rc )
>      {
>      case X86EMUL_UNHANDLEABLE:
> -        hvm_dump_emulation_state(XENLOG_G_WARNING "MMCFG", &ctxt);
> +        hvm_dump_emulation_state(XENLOG_G_WARNING, "MMCFG",
> &ctxt);
>          break;
>      case X86EMUL_EXCEPTION:
>          if ( ctxt.ctxt.event_pending )
> @@ -2092,7 +2092,7 @@ void hvm_emulate_one_vm_event(enum emul_
>           */
>          return;
>      case X86EMUL_UNHANDLEABLE:
> -        hvm_dump_emulation_state(XENLOG_G_DEBUG "Mem event", &ctx);
> +        hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event",
> &ctx);
>          hvm_inject_hw_exception(trapnr, errcode);
>          break;
>      case X86EMUL_EXCEPTION:
> @@ -2220,7 +2220,7 @@ static const char *guest_x86_mode_to_str
>      }
>  }
> 
> -void hvm_dump_emulation_state(const char *prefix,
> +void hvm_dump_emulation_state(const char *loglvl, const char *prefix,
>                                struct hvm_emulate_ctxt *hvmemul_ctxt)
>  {
>      struct vcpu *curr = current;
> @@ -2228,8 +2228,8 @@ void hvm_dump_emulation_state(const char
>      const struct segment_register *cs =
>          hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt);
> 
> -    printk("%s emulation failed: %pv %s @ %04x:%08lx -> %*ph\n",
> -           prefix, curr, mode_str, cs->sel, hvmemul_ctxt->insn_buf_eip,
> +    printk("%s%s emulation failed: %pv %s @ %04x:%08lx -> %*ph\n",
> +           loglvl, prefix, curr, mode_str, cs->sel, 
> hvmemul_ctxt->insn_buf_eip,
>             hvmemul_ctxt->insn_buf_bytes, hvmemul_ctxt->insn_buf);
>  }
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3633,7 +3633,7 @@ int hvm_descriptor_access_intercept(uint
>          hvm_monitor_descriptor_access(exit_info, vmx_exit_qualification,
>                                        descriptor, is_write);
>      }
> -    else if ( !hvm_emulate_one_insn(is_sysdesc_access) )
> +    else if ( !hvm_emulate_one_insn(is_sysdesc_access, "sysdesc access") )
>          domain_crash(currd);
> 
>      return X86EMUL_OKAY;
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -77,7 +77,7 @@ void send_invalidate_req(void)
>          gprintk(XENLOG_ERR, "Unsuccessful map-cache invalidate\n");
>  }
> 
> -bool hvm_emulate_one_insn(hvm_emulate_validate_t *validate)
> +bool hvm_emulate_one_insn(hvm_emulate_validate_t *validate, const
> char *descr)
>  {
>      struct hvm_emulate_ctxt ctxt;
>      struct vcpu *curr = current;
> @@ -96,7 +96,7 @@ bool hvm_emulate_one_insn(hvm_emulate_va
>      switch ( rc )
>      {
>      case X86EMUL_UNHANDLEABLE:
> -        hvm_dump_emulation_state(XENLOG_G_WARNING "MMIO", &ctxt);
> +        hvm_dump_emulation_state(XENLOG_G_WARNING, descr, &ctxt);
>          return false;
> 
>      case X86EMUL_EXCEPTION:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2676,7 +2676,7 @@ void svm_vmexit_handler(struct cpu_user_
>              if ( handle_pio(port, bytes, dir) )
>                  __update_guest_eip(regs, vmcb->exitinfo2 - vmcb->rip);
>          }
> -        else if ( !hvm_emulate_one_insn(x86_insn_is_portio) )
> +        else if ( !hvm_emulate_one_insn(x86_insn_is_portio, "port I/O") )
>              hvm_inject_hw_exception(TRAP_gp_fault, 0);
>          break;
> 
> @@ -2684,7 +2684,7 @@ void svm_vmexit_handler(struct cpu_user_
>      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 ( !hvm_emulate_one_insn(x86_insn_is_cr_access) )
> +        else if ( !hvm_emulate_one_insn(x86_insn_is_cr_access, "CR access") )
>              hvm_inject_hw_exception(TRAP_gp_fault, 0);
>          break;
> 
> @@ -2694,7 +2694,7 @@ void svm_vmexit_handler(struct cpu_user_
>              svm_invlpg_intercept(vmcb->exitinfo1);
>              __update_guest_eip(regs, vmcb->nextrip - vmcb->rip);
>          }
> -        else if ( !hvm_emulate_one_insn(is_invlpg) )
> +        else if ( !hvm_emulate_one_insn(is_invlpg, "invlpg") )
>              hvm_inject_hw_exception(TRAP_gp_fault, 0);
>          break;
> 
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -153,7 +153,7 @@ void vmx_realmode_emulate_one(struct hvm
>      return;
> 
>   fail:
> -    hvm_dump_emulation_state(XENLOG_G_ERR "Real-mode",
> hvmemul_ctxt);
> +    hvm_dump_emulation_state(XENLOG_G_ERR, "Real-mode",
> hvmemul_ctxt);
>      domain_crash(curr->domain);
>  }
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -4013,7 +4013,7 @@ void vmx_vmexit_handler(struct cpu_user_
>          if ( exit_qualification & 0x10 )
>          {
>              /* INS, OUTS */
> -            if ( !hvm_emulate_one_insn(x86_insn_is_portio) )
> +            if ( !hvm_emulate_one_insn(x86_insn_is_portio, "port I/O") )
>                  hvm_inject_hw_exception(TRAP_gp_fault, 0);
>          }
>          else
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -49,8 +49,9 @@ enum emul_kind {
>      EMUL_KIND_SET_CONTEXT_INSN
>  };
> 
> -bool __nonnull(1) hvm_emulate_one_insn(
> -    hvm_emulate_validate_t *validate);
> +bool __nonnull(1, 2) hvm_emulate_one_insn(
> +    hvm_emulate_validate_t *validate,
> +    const char *descr);
>  int hvm_emulate_one(
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
>  void hvm_emulate_one_vm_event(enum emul_kind kind,
> @@ -77,7 +78,7 @@ int hvm_emulate_one_mmio(unsigned long m
> 
>  static inline bool handle_mmio(void)
>  {
> -    return hvm_emulate_one_insn(x86_insn_is_mem_access);
> +    return hvm_emulate_one_insn(x86_insn_is_mem_access, "MMIO");
>  }
> 
>  int hvmemul_insn_fetch(enum x86_segment seg,
> @@ -90,7 +91,7 @@ int hvmemul_do_pio_buffer(uint16_t port,
>                            uint8_t dir,
>                            void *buffer);
> 
> -void hvm_dump_emulation_state(const char *prefix,
> +void hvm_dump_emulation_state(const char *loglvl, const char *prefix,
>                                struct hvm_emulate_ctxt *hvmemul_ctxt);
> 
>  #endif /* __ASM_X86_HVM_EMULATE_H__ */
> 


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