| Message ID | 1480932571-23547-8-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 05 December 2016 10:10 > 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 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@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > > 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
>>> On 05.12.16 at 11:09, <andrew.cooper3@citrix.com> wrote: > Jan: Probably worth waiting to rebase over your changes moving the > architectrual defines elsewhere I have no such changes yet; I only recall us discussing this would be a good thing at some point. > --- 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; With this gone, is hvm_check_cpuid_faulting() as useful function to retain, considering it then has only one caller in VMX code? Other than that Reviewed-by: Jan Beulich <jbeulich@suse.com> Would allow me to drop half a hunk from the pv privop patch, so I'd be glad to see this go in soon. Jan
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,
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@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Paul Durrant <paul.durrant@citrix.com> 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(-)