* [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 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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.