Re: [Xen-devel] [PATCH 7/8] x86/emul: Support CPUID fauilting via a speculative MSR read

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 05 December 2016 10:10
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: [PATCH 7/8] x86/emul: Support CPUID fauilting via a speculative
> MSR read
> 
> Use the new speculative read support to query MSR_INTEL_MISC_FEATURES
> for
> active CPUID faulting, without raising #GP as a side effect.
> 
> This removes the need for every cpuid() emulation hook to individually
> support
> CPUID faulting.
> 
> 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>

> 
> Jan: Probably worth waiting to rebase over your changes moving the
> architectrual defines elsewhere
> ---
>  xen/arch/x86/hvm/emulate.c             |  9 ---------
>  xen/arch/x86/x86_emulate/x86_emulate.c | 16 +++++++++++++---
>  xen/arch/x86/x86_emulate/x86_emulate.h |  7 +------
>  3 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index bce0b00..1a132e7 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1567,15 +1567,6 @@ int hvmemul_cpuid(
>      unsigned int *edx,
>      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;
> -
>      hvm_funcs.cpuid_intercept(eax, ebx, ecx, edx);
>      return X86EMUL_OKAY;
>  }
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 5cba7ec..67495eb 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -406,6 +406,8 @@ typedef union {
> 
>  /* MSRs. */
>  #define MSR_TSC          0x00000010
> +#define MSR_INTEL_MISC_FEATURES 0x00000140
> +#define MISC_FEATURES_CPUID_FAULTING (1 << 0)
>  #define MSR_SYSENTER_CS  0x00000174
>  #define MSR_SYSENTER_ESP 0x00000175
>  #define MSR_SYSENTER_EIP 0x00000176
> @@ -5044,13 +5046,21 @@ x86_emulate(
>          src.val = x86_seg_fs;
>          goto pop_seg;
> 
> -    case X86EMUL_OPC(0x0f, 0xa2): /* cpuid */ {
> +    case X86EMUL_OPC(0x0f, 0xa2): /* cpuid */
> +    {
>          unsigned int eax = _regs.eax, ebx = _regs.ebx;
>          unsigned int ecx = _regs.ecx, edx = _regs.edx;
> +        uint64_t misc_features;
> +
>          fail_if(ops->cpuid == NULL);
> +        generate_exception_if(
> +            ops->read_msr &&
> +            ops->read_msr(MSR_INTEL_MISC_FEATURES,
> +                          &misc_features, true, ctxt) == X86EMUL_OKAY &&
> +            (misc_features & MISC_FEATURES_CPUID_FAULTING) &&
> +            !mode_ring0(), EXC_GP, 0); /* CPUID Faulting? */
> +
>          rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt);
> -        generate_exception_if(rc == X86EMUL_EXCEPTION,
> -                              EXC_GP, 0); /* CPUID Faulting? */
>          if ( rc != X86EMUL_OKAY )
>              goto done;
>          _regs.eax = eax; _regs.ebx = ebx;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index 89cf20d..46c863f 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -395,12 +395,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)(
>          unsigned int *eax,
>          unsigned int *ebx,
> --
> 2.1.4


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