Re: [Xen-devel] [PATCH 6/8] x86/emul: Support speculative MSR reads

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 05 December 2016 10:09
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: [PATCH 6/8] x86/emul: Support speculative MSR reads
> 
> Update the read_msr() hook to take an additional parameter, indicating that
> there should be no side effects of the read.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>

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

> ---
>  xen/arch/x86/hvm/emulate.c             |  3 ++-
>  xen/arch/x86/x86_emulate/x86_emulate.c | 24 ++++++++++++------------
>  xen/arch/x86/x86_emulate/x86_emulate.h |  6 +++++-
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index b182d57..bce0b00 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1529,11 +1529,12 @@ static int hvmemul_write_cr(
>  static int hvmemul_read_msr(
>      unsigned int reg,
>      uint64_t *val,
> +    bool speculative,
>      struct x86_emulate_ctxt *ctxt)
>  {
>      int rc = hvm_msr_read_intercept(reg, val);
> 
> -    if ( rc == X86EMUL_EXCEPTION )
> +    if ( rc == X86EMUL_EXCEPTION && !speculative )
>          x86_emul_hw_exception(TRAP_gp_fault, 0, ctxt);
> 
>      return rc;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 877023d..5cba7ec 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1319,7 +1319,7 @@ in_longmode(
>      uint64_t efer;
> 
>      if ( !ops->read_msr ||
> -         unlikely(ops->read_msr(MSR_EFER, &efer, ctxt) != X86EMUL_OKAY) )
> +         unlikely(ops->read_msr(MSR_EFER, &efer, false, ctxt) !=
> X86EMUL_OKAY) )
>          return -1;
> 
>      return !!(efer & EFER_LMA);
> @@ -4412,7 +4412,7 @@ x86_emulate(
>          {
>              uint64_t tsc_aux;
>              fail_if(ops->read_msr == NULL);
> -            if ( (rc = ops->read_msr(MSR_TSC_AUX, &tsc_aux, ctxt)) != 0 )
> +            if ( (rc = ops->read_msr(MSR_TSC_AUX, &tsc_aux, false, ctxt)) != 
> 0 )
>                  goto done;
>              _regs.ecx = (uint32_t)tsc_aux;
>              goto rdtsc;
> @@ -4548,11 +4548,11 @@ x86_emulate(
> 
>          /* Inject #UD if syscall/sysret are disabled. */
>          fail_if(ops->read_msr == NULL);
> -        if ( (rc = ops->read_msr(MSR_EFER, &msr_content, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr(MSR_EFER, &msr_content, false, ctxt)) != 0 )
>              goto done;
>          generate_exception_if((msr_content & EFER_SCE) == 0, EXC_UD);
> 
> -        if ( (rc = ops->read_msr(MSR_STAR, &msr_content, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr(MSR_STAR, &msr_content, false, ctxt)) != 0 )
>              goto done;
> 
>          cs.sel = (msr_content >> 32) & ~3; /* SELECTOR_RPL_MASK */
> @@ -4574,11 +4574,11 @@ x86_emulate(
>              _regs.r11 = _regs.eflags & ~EFLG_RF;
> 
>              if ( (rc = ops->read_msr(mode_64bit() ? MSR_LSTAR : MSR_CSTAR,
> -                                     &msr_content, ctxt)) != 0 )
> +                                     &msr_content, false, ctxt)) != 0 )
>                  goto done;
>              _regs.rip = msr_content;
> 
> -            if ( (rc = ops->read_msr(MSR_FMASK, &msr_content, ctxt)) != 0 )
> +            if ( (rc = ops->read_msr(MSR_FMASK, &msr_content, false, ctxt)) 
> != 0
> )
>                  goto done;
>              _regs.eflags &= ~(msr_content | EFLG_RF);
>          }
> @@ -4793,7 +4793,7 @@ x86_emulate(
>              generate_exception_if(cr4 & CR4_TSD, EXC_GP, 0);
>          }
>          fail_if(ops->read_msr == NULL);
> -        if ( (rc = ops->read_msr(MSR_TSC, &val, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr(MSR_TSC, &val, false, ctxt)) != 0 )
>              goto done;
>          _regs.edx = (uint32_t)(val >> 32);
>          _regs.eax = (uint32_t)(val >>  0);
> @@ -4804,7 +4804,7 @@ x86_emulate(
>          uint64_t val;
>          generate_exception_if(!mode_ring0(), EXC_GP, 0);
>          fail_if(ops->read_msr == NULL);
> -        if ( (rc = ops->read_msr((uint32_t)_regs.ecx, &val, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr((uint32_t)_regs.ecx, &val, false, ctxt)) != 
> 0 )
>              goto done;
>          _regs.edx = (uint32_t)(val >> 32);
>          _regs.eax = (uint32_t)(val >>  0);
> @@ -4825,7 +4825,7 @@ x86_emulate(
>          generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
> 
>          fail_if(ops->read_msr == NULL);
> -        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, false,
> ctxt)) != 0 )
>              goto done;
> 
>          generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0);
> @@ -4853,11 +4853,11 @@ x86_emulate(
>               (rc = ops->write_segment(x86_seg_ss, &sreg, ctxt)) != 0 )
>              goto done;
> 
> -        if ( (rc = ops->read_msr(MSR_SYSENTER_EIP, &msr_content, ctxt)) != 0 
> )
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_EIP, &msr_content, false,
> ctxt)) != 0 )
>              goto done;
>          _regs.eip = lm ? msr_content : (uint32_t)msr_content;
> 
> -        if ( (rc = ops->read_msr(MSR_SYSENTER_ESP, &msr_content, ctxt)) != 0
> )
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_ESP, &msr_content, false,
> ctxt)) != 0 )
>              goto done;
>          _regs.esp = lm ? msr_content : (uint32_t)msr_content;
> 
> @@ -4873,7 +4873,7 @@ x86_emulate(
>          generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
> 
>          fail_if(ops->read_msr == NULL);
> -        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) != 0 )
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, false,
> ctxt)) != 0 )
>              goto done;
> 
>          generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0);
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index 164fc24..89cf20d 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -372,15 +372,19 @@ struct x86_emulate_ops
>      /*
>       * read_msr: Read from model-specific register.
>       *  @reg:   [IN ] Register to read.
> +     *  @val:   [OUT] Value read (only valid on X86EMUL_OKAY)
> +     *  @speculative [IN] Speculative read?
>       */
>      int (*read_msr)(
>          unsigned int reg,
>          uint64_t *val,
> +        bool speculative,
>          struct x86_emulate_ctxt *ctxt);
> 
>      /*
> -     * write_dr: Write to model-specific register.
> +     * write_msr: Write to model-specific register.
>       *  @reg:   [IN ] Register to write.
> +     *  @val:   [IN ] Value to write.
>       */
>      int (*write_msr)(
>          unsigned int reg,
> --
> 2.1.4


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