* [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept @ 2016-12-19 16:37 Andrew Cooper 2016-12-19 16:37 ` [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation Andrew Cooper 2016-12-20 8:09 ` [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Jan Beulich 0 siblings, 2 replies; 6+ messages in thread From: Andrew Cooper @ 2016-12-19 16:37 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich Having the instruction emulator fill in all #UDs when using FEP is unhelpful when trying to test emulation behaviour against hardware. Restrict emulation from the #UD intercept to the cross-vendor case, and when a postive Forced Emulation Prefix has been identified. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/arch/x86/hvm/hvm.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 73d24df..12a6f46 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4002,13 +4002,15 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content, void hvm_ud_intercept(struct cpu_user_regs *regs) { + struct vcpu *cur = current; + bool should_emulate = + cur->domain->arch.x86_vendor != boot_cpu_data.x86_vendor; struct hvm_emulate_ctxt ctxt; hvm_emulate_init_once(&ctxt, regs); if ( opt_hvm_fep ) { - struct vcpu *cur = current; const struct segment_register *cs = &ctxt.seg_reg[x86_seg_cs]; uint32_t walk = (ctxt.seg_reg[x86_seg_ss].attr.fields.dpl == 3) ? PFEC_user_mode : 0; @@ -4032,9 +4034,17 @@ void hvm_ud_intercept(struct cpu_user_regs *regs) regs->eip = regs->_eip; add_taint(TAINT_HVM_FEP); + + should_emulate = true; } } + if ( !should_emulate ) + { + hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC); + return; + } + switch ( hvm_emulate_one(&ctxt) ) { case X86EMUL_UNHANDLEABLE: -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation 2016-12-19 16:37 [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Andrew Cooper @ 2016-12-19 16:37 ` Andrew Cooper 2016-12-20 8:22 ` Jan Beulich 2016-12-20 8:09 ` [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Jan Beulich 1 sibling, 1 reply; 6+ messages in thread From: Andrew Cooper @ 2016-12-19 16:37 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich Introduce vendor_is() to allow emulation to have vendor-specific behaviour. Adjust the SYSCALL behaviour on Intel to raise #UD when executed outside of 64bit mode. in_longmode() has different return semantics from rc, so use a separate integer for the purpose. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/arch/x86/x86_emulate/x86_emulate.c | 54 ++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 165eebb..419c2da 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1327,6 +1327,45 @@ static bool vcpu_has( #define host_and_vcpu_must_have(feat) vcpu_must_have(feat) #endif +#define X86_VENDOR_INTEL 0 +#define X86_VENDOR_AMD 2 + +static bool vendor_is( + struct x86_emulate_ctxt *ctxt, + const struct x86_emulate_ops *ops, + int vendor) +{ + unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0; + int rc = X86EMUL_OKAY; + + fail_if(!ops->cpuid); + rc = ops->cpuid(&eax, &ebx, &ecx, &edx, ctxt); + if ( rc == X86EMUL_OKAY ) + { + switch ( vendor ) + { + case X86_VENDOR_INTEL: + return (ebx == 0x756e6547u && /* "GenuineIntel" */ + ecx == 0x6c65746eu && + edx == 0x49656e69u); + + case X86_VENDOR_AMD: + return (ebx == 0x68747541u && /* "AuthenticAMD" */ + ecx == 0x444d4163u && + edx == 0x69746e65u); + default: + rc = ~X86EMUL_OKAY; + break; + } + } + + done: + return rc == X86EMUL_OKAY; +} + +#define vendor_is_intel() vendor_is(ctxt, ops, X86_VENDOR_INTEL) +#define vendor_is_amd() vendor_is(ctxt, ops, X86_VENDOR_AMD) + static int in_longmode( struct x86_emulate_ctxt *ctxt, @@ -4623,10 +4662,15 @@ x86_emulate( break; } - case X86EMUL_OPC(0x0f, 0x05): /* syscall */ { + case X86EMUL_OPC(0x0f, 0x05): /* syscall */ + { uint64_t msr_content; +#ifdef __x86_64__ + int lm; +#endif - generate_exception_if(!in_protmode(ctxt, ops), EXC_UD); + generate_exception_if(!in_protmode(ctxt, ops) || + (vendor_is_intel() && !mode_64bit()), EXC_UD); /* Inject #UD if syscall/sysret are disabled. */ fail_if(ops->read_msr == NULL); @@ -4645,10 +4689,10 @@ x86_emulate( sreg.attr.bytes = 0xc93; /* G+DB+P+S+Data */ #ifdef __x86_64__ - rc = in_longmode(ctxt, ops); - if ( rc < 0 ) + lm = in_longmode(ctxt, ops); + if ( lm < 0 ) goto cannot_emulate; - if ( rc ) + if ( lm ) { cs.attr.bytes = 0xa9b; /* L+DB+P+S+Code */ -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation 2016-12-19 16:37 ` [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation Andrew Cooper @ 2016-12-20 8:22 ` Jan Beulich 2016-12-20 12:14 ` Andrew Cooper 0 siblings, 1 reply; 6+ messages in thread From: Jan Beulich @ 2016-12-20 8:22 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 19.12.16 at 17:37, <andrew.cooper3@citrix.com> wrote: > Introduce vendor_is() to allow emulation to have vendor-specific > behaviour. Adjust the SYSCALL behaviour on Intel to raise #UD when > executed outside of 64bit mode. I'd rather not see us go this route. I've been carrying a patch making the vendor an input (not submitted so far due to other at least contextual prereqs missing when I last check, but sent out just a minute ago). What do you think? > in_longmode() has different return semantics from rc, so use a separate > integer for the purpose. The ugliness of the additional #ifdef doesn't seem to warrant this. What I've been thinking the other day though is: Why don't we put the whole SYSCALL emulation into a __XEN__ conditional (implying __x86_64__, i.e. allowing the inner ones to be removed)? That would likely also limit the impact of the pending register name changes which I hope to be able to submit soon (once enough prereqs have gone in). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation 2016-12-20 8:22 ` Jan Beulich @ 2016-12-20 12:14 ` Andrew Cooper 2016-12-20 13:00 ` Jan Beulich 0 siblings, 1 reply; 6+ messages in thread From: Andrew Cooper @ 2016-12-20 12:14 UTC (permalink / raw) To: Jan Beulich; +Cc: Xen-devel On 20/12/2016 08:22, Jan Beulich wrote: >>>> On 19.12.16 at 17:37, <andrew.cooper3@citrix.com> wrote: >> Introduce vendor_is() to allow emulation to have vendor-specific >> behaviour. Adjust the SYSCALL behaviour on Intel to raise #UD when >> executed outside of 64bit mode. > I'd rather not see us go this route. I've been carrying a patch > making the vendor an input (not submitted so far due to other > at least contextual prereqs missing when I last check, but sent > out just a minute ago). What do you think? Rebasing on top of that would be far more simple. > >> in_longmode() has different return semantics from rc, so use a separate >> integer for the purpose. > The ugliness of the additional #ifdef doesn't seem to warrant > this. It is a matter of correctness. The only reason we exit from it cleanly is because the cannot_emulate path discards rc, while the other paths overwrite rc because of hitting the read_msr() or write_segment() hooks in each case. > What I've been thinking the other day though is: Why > don't we put the whole SYSCALL emulation into a __XEN__ > conditional (implying __x86_64__, i.e. allowing the inner ones > to be removed)? This depends on whether we think it is ever realistic to be able to be able to test things like this in the userspace harness. I am not sure we realistically can. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation 2016-12-20 12:14 ` Andrew Cooper @ 2016-12-20 13:00 ` Jan Beulich 0 siblings, 0 replies; 6+ messages in thread From: Jan Beulich @ 2016-12-20 13:00 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 20.12.16 at 13:14, <andrew.cooper3@citrix.com> wrote: > On 20/12/2016 08:22, Jan Beulich wrote: >>>>> On 19.12.16 at 17:37, <andrew.cooper3@citrix.com> wrote: >> What I've been thinking the other day though is: Why >> don't we put the whole SYSCALL emulation into a __XEN__ >> conditional (implying __x86_64__, i.e. allowing the inner ones >> to be removed)? > > This depends on whether we think it is ever realistic to be able to be > able to test things like this in the userspace harness. I am not sure > we realistically can. Well - it's all about register modifications, which we surely could test. But this would be a rather contrived test, the usefulness of which I would question. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept 2016-12-19 16:37 [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Andrew Cooper 2016-12-19 16:37 ` [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation Andrew Cooper @ 2016-12-20 8:09 ` Jan Beulich 1 sibling, 0 replies; 6+ messages in thread From: Jan Beulich @ 2016-12-20 8:09 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 19.12.16 at 17:37, <andrew.cooper3@citrix.com> wrote: > Having the instruction emulator fill in all #UDs when using FEP is unhelpful > when trying to test emulation behaviour against hardware. > > Restrict emulation from the #UD intercept to the cross-vendor case, and when a > postive Forced Emulation Prefix has been identified. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-20 13:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-19 16:37 [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Andrew Cooper 2016-12-19 16:37 ` [PATCH 2/2] x86/emul: Bugfixes to SYSCALL emulation Andrew Cooper 2016-12-20 8:22 ` Jan Beulich 2016-12-20 12:14 ` Andrew Cooper 2016-12-20 13:00 ` Jan Beulich 2016-12-20 8:09 ` [PATCH 1/2] x86/hvm: Don't emulate all instructions hitting the #UD intercept Jan Beulich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).