Re: [PATCH v2 3/3] x86/emul: Support CPUID faulting via a speculative MSR read

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: 20 February 2017 10:29
> To: Xen-devel <xen-devel@lists.xen.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich
> <JBeulich@suse.com>; Paul Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH v2 3/3] x86/emul: Support CPUID faulting via a speculative
> MSR read
> 
> This removes the need for every cpuid() emulation hook to individually
> support
> CPUID faulting.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> 

hvm/emulate.c change...

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

> v2:
>  * Substantial rebase
>  * Reimplement speculative reading by squashing the exception rather than
>    trying to prevent it by using an extra function parameter.
> ---
>  xen/arch/x86/hvm/emulate.c             |  9 ---------
>  xen/arch/x86/traps.c                   | 18 +-----------------
>  xen/arch/x86/x86_emulate/x86_emulate.c | 19 +++++++++++++++++--
>  xen/arch/x86/x86_emulate/x86_emulate.h |  7 +------
>  4 files changed, 19 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index edcae5e..f24d289 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1575,15 +1575,6 @@ static int hvmemul_wbinvd(
>  int hvmemul_cpuid(uint32_t leaf, uint32_t subleaf,
>                    struct cpuid_leaf *res, struct x86_emulate_ctxt *ctxt)
>  {
> -    /*
> -     * x86_emulate uses this function to query CPU features for its own
> internal
> -     * use. Make sure we're actually emulating CPUID before emulating CPUID
> -     * faulting.
> -     */
> -    if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) &&
> -         hvm_check_cpuid_faulting(current) )
> -        return X86EMUL_EXCEPTION;
> -
>      guest_cpuid(current, leaf, subleaf, res);
>      return X86EMUL_OKAY;
>  }
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index ec8b002..75c89eb 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2884,23 +2884,7 @@ static int priv_op_wbinvd(struct x86_emulate_ctxt
> *ctxt)
>  int pv_emul_cpuid(uint32_t leaf, uint32_t subleaf,
>                    struct cpuid_leaf *res, struct x86_emulate_ctxt *ctxt)
>  {
> -    const struct vcpu *curr = current;
> -
> -    /*
> -     * x86_emulate uses this function to query CPU features for its own
> -     * internal use. Make sure we're actually emulating CPUID before checking
> -     * for emulated CPUID faulting.
> -     */
> -    if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) )
> -    {
> -
> -        /* If cpuid faulting is enabled and CPL>0 leave the #GP untouched. */
> -        if ( curr->arch.cpuid_faulting &&
> -             !guest_kernel_mode(curr, ctxt->regs) )
> -            return X86EMUL_EXCEPTION;
> -    }
> -
> -    guest_cpuid(curr, leaf, subleaf, res);
> +    guest_cpuid(current, leaf, subleaf, res);
> 
>      return X86EMUL_OKAY;
>  }
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index f339d36..c3fc26a 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5424,10 +5424,25 @@ x86_emulate(
>          break;
> 
>      case X86EMUL_OPC(0x0f, 0xa2): /* cpuid */
> +        msr_val = 0;
>          fail_if(ops->cpuid == NULL);
> +
> +        /* Speculatively read MSR_INTEL_MISC_FEATURES_ENABLES. */
> +        if ( ops->read_msr &&
> +             (rc = ops->read_msr(MSR_INTEL_MISC_FEATURES_ENABLES,
> +                                 &msr_val, ctxt)) == X86EMUL_EXCEPTION )
> +        {
> +            /* Not implemented.  Squash the exception and proceed normally. */
> +            x86_emul_reset_event(ctxt);
> +            rc = X86EMUL_OKAY;
> +        }
> +        if ( rc != X86EMUL_OKAY )
> +            goto done;
> +
> +        generate_exception_if((msr_val &
> MSR_MISC_FEATURES_CPUID_FAULTING) &&
> +                              !mode_ring0(), EXC_GP, 0); /* Faulting active? */
> +
>          rc = ops->cpuid(_regs._eax, _regs._ecx, &cpuid_leaf, ctxt);
> -        generate_exception_if(rc == X86EMUL_EXCEPTION,
> -                              EXC_GP, 0); /* CPUID Faulting? */
>          if ( rc != X86EMUL_OKAY )
>              goto done;
>          _regs.r(ax) = cpuid_leaf.a;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index 071668d..c35873e 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -413,12 +413,7 @@ struct x86_emulate_ops
>      int (*wbinvd)(
>          struct x86_emulate_ctxt *ctxt);
> 
> -    /*
> -     * cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs.
> -     *
> -     * May return X86EMUL_EXCEPTION, which causes the emulator to inject
> -     * #GP[0].  Used to implement CPUID faulting.
> -     */
> +    /* cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs. */
>      int (*cpuid)(
>          uint32_t leaf,
>          uint32_t subleaf,
> --
> 2.1.4


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