* [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup
@ 2016-12-01 16:55 Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:55 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper
This is the quantity of changes required to fix some edgecases in XSA-191
which were ultimately chosen not to go out in the security fix. The main
purpose of this series is to fix emulation sufficiently to allow the final
patch to avoid opencoding all of the segmenation logic.
Because this is a very long series and there are not many changes from v3, I
am only sending out the patches which have changes, to avoid spamming the
list.
Andrew Cooper (24):
x86/shadow: Fix #PFs from emulated writes crossing a page boundary
x86/emul: Drop X86EMUL_CMPXCHG_FAILED
x86/emul: Simplfy emulation state setup
x86/emul: Rename hvm_trap to x86_event and move it into the emulation infrastructure
x86/emul: Rename HVM_DELIVER_NO_ERROR_CODE to X86_EVENT_NO_EC
x86/pv: Implement pv_inject_{event,page_fault,hw_exception}()
x86/emul: Clean up the naming of the retire union
x86/emul: Correct the behaviour of pop %ss and interrupt shadowing
x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour
x86/emul: Always use fault semantics for software events
x86/emul: Implement singlestep as a retire flag
x86/emul: Remove opencoded exception generation
x86/emul: Rework emulator event injection
x86/vmx: Use hvm_{get,set}_segment_register() rather than vmx_{get,set}_segment_register()
x86/hvm: Reposition the modification of raw segment data from the VMCB/VMCS
x86/emul: Avoid raising faults behind the emulators back
x86/pv: Avoid raising faults behind the emulators back
x86/shadow: Avoid raising faults behind the emulators back
x86/hvm: Extend the hvm_copy_*() API with a pagefault_info pointer
x86/hvm: Reimplement hvm_copy_*_nofault() in terms of no pagefault_info
x86/hvm: Rename hvm_copy_*_guest_virt() to hvm_copy_*_guest_linear()
x86/hvm: Avoid __hvm_copy() raising #PF behind the emulators back
x86/emul: Prepare to allow use of system segments for memory references
x86/emul: Use system-segment relative memory accesses
tools/tests/x86_emulator/test_x86_emulator.c | 1 +
tools/tests/x86_emulator/x86_emulate.c | 4 +-
xen/arch/x86/hvm/emulate.c | 147 ++++-------
xen/arch/x86/hvm/hvm.c | 370 +++++++++++++++++++--------
xen/arch/x86/hvm/io.c | 4 +-
xen/arch/x86/hvm/nestedhvm.c | 2 +-
xen/arch/x86/hvm/svm/nestedsvm.c | 13 +-
xen/arch/x86/hvm/svm/svm.c | 156 +++++------
xen/arch/x86/hvm/vmx/intr.c | 2 +-
xen/arch/x86/hvm/vmx/realmode.c | 16 +-
xen/arch/x86/hvm/vmx/vmx.c | 109 ++++----
xen/arch/x86/hvm/vmx/vvmx.c | 44 ++--
xen/arch/x86/mm.c | 91 ++++++-
xen/arch/x86/mm/shadow/common.c | 40 +--
xen/arch/x86/mm/shadow/multi.c | 71 ++++-
xen/arch/x86/traps.c | 147 ++++++-----
xen/arch/x86/x86_emulate/x86_emulate.c | 355 ++++++++++++++-----------
xen/arch/x86/x86_emulate/x86_emulate.h | 221 +++++++++++++---
xen/include/asm-x86/desc.h | 6 +
xen/include/asm-x86/domain.h | 26 ++
xen/include/asm-x86/hvm/emulate.h | 3 -
xen/include/asm-x86/hvm/hvm.h | 86 +++----
xen/include/asm-x86/hvm/support.h | 42 ++-
xen/include/asm-x86/hvm/svm/nestedsvm.h | 6 +-
xen/include/asm-x86/hvm/vcpu.h | 2 +-
xen/include/asm-x86/hvm/vmx/vmx.h | 2 -
xen/include/asm-x86/hvm/vmx/vvmx.h | 4 +-
xen/include/asm-x86/mm.h | 1 -
28 files changed, 1209 insertions(+), 762 deletions(-)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
@ 2016-12-01 16:55 ` Andrew Cooper
2016-12-02 11:26 ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:55 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
In debug builds, confirm that some properties of x86_emulate()'s behaviour
actually hold. The first property, fixed in a previous change, is that retire
flags are only ever set in the X86EMUL_OKAY case.
While adjusting the userspace test harness to cope with ASSERT() in
x86_emulate.h, fix a build problem introduced in c/s 122dd9575c7 "x86emul:
in_longmode() should not ignore ->read_msr() errors" by providing an
implementation of likely()/unlikely().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
v4:
* Shuffle #ifdefary
* Correct the documentation for x86_emulate()
* Use true/false in likely/unlikely definitions
v3:
* New
---
tools/tests/x86_emulator/test_x86_emulator.c | 1 +
tools/tests/x86_emulator/x86_emulate.c | 4 +++-
xen/arch/x86/x86_emulate/x86_emulate.c | 3 +++
xen/arch/x86/x86_emulate/x86_emulate.h | 27 ++++++++++++++++++++++++++-
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index f255fef..b54fd11 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1,3 +1,4 @@
+#include <assert.h>
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index c46b7fc..897b9ab 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -16,7 +16,6 @@ typedef bool bool_t;
#define EFER_LMA (1 << 10)
#define BUG() abort()
-#define ASSERT assert
#define ASSERT_UNREACHABLE() assert(!__LINE__)
#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
@@ -50,4 +49,7 @@ typedef bool bool_t;
#define __init
#define __maybe_unused __attribute__((__unused__))
+#define likely(x) __builtin_expect(!!(x), true)
+#define unlikely(x) __builtin_expect(!!(x), false)
+
#include "x86_emulate/x86_emulate.c"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index bfcc05d..3e602da 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2404,6 +2404,9 @@ x86_decode(
#undef insn_fetch_bytes
#undef insn_fetch_type
+/* Undo DEBUG wrapper. */
+#undef x86_emulate
+
int
x86_emulate(
struct x86_emulate_ctxt *ctxt,
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index ef39601..60f9105 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -23,6 +23,10 @@
#ifndef __X86_EMULATE_H__
#define __X86_EMULATE_H__
+#if !defined(__XEN__) && !defined(ASSERT)
+#define ASSERT assert
+#endif
+
#define MAX_INST_LEN 15
struct x86_emulate_ctxt;
@@ -546,13 +550,34 @@ struct x86_emulate_stub {
/*
* x86_emulate: Emulate an instruction.
- * Returns -1 on failure, 0 on success.
+ * Returns X86EMUL_* constants.
*/
int
x86_emulate(
struct x86_emulate_ctxt *ctxt,
const struct x86_emulate_ops *ops);
+#ifndef NDEBUG
+/*
+ * In debug builds, wrap x86_emulate() with some assertions about its expected
+ * behaviour.
+ */
+static inline int x86_emulate_wrapper(
+ struct x86_emulate_ctxt *ctxt,
+ const struct x86_emulate_ops *ops)
+{
+ int rc = x86_emulate(ctxt, ops);
+
+ /* Retire flags should only be set for successful instruction emulation. */
+ if ( rc != X86EMUL_OKAY )
+ ASSERT(ctxt->retire.raw == 0);
+
+ return rc;
+}
+
+#define x86_emulate x86_emulate_wrapper
+#endif
+
/*
* Given the 'reg' portion of a ModRM byte, and a register block, return a
* pointer into the block that addresses the relevant register.
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 10/24] x86/emul: Always use fault semantics for software events
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
@ 2016-12-01 16:55 ` Andrew Cooper
2016-12-01 19:07 ` Boris Ostrovsky
2016-12-02 11:28 ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
` (2 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:55 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Boris Ostrovsky, Tim Deegan, Suravee Suthikulpanit,
Jan Beulich
The common case is already using fault semantics out of x86_emulate(), as that
is how VT-x/SVM expects to inject the event (given suitable hardware support).
However, x86_emulate() returning X86EMUL_EXCEPTION and also completing a
register writeback is problematic for callers.
Switch the logic to always using fault semantics, and leave svm_inject_trap()
to fix up %eip if necessary.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
v4:
* s/eip/rip/
* Zero the high bits of regs->rip and vmcb->nextrip outside of 64bit mode.
* Spelling fixes
v3:
* New
---
xen/arch/x86/hvm/svm/svm.c | 60 ++++++++++++++++++++++------------
xen/arch/x86/x86_emulate/x86_emulate.c | 2 --
xen/arch/x86/x86_emulate/x86_emulate.h | 8 ++++-
3 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 912d871..8c4f3c7 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1209,7 +1209,7 @@ static void svm_inject_event(const struct x86_event *event)
struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
eventinj_t eventinj = vmcb->eventinj;
struct x86_event _event = *event;
- const struct cpu_user_regs *regs = guest_cpu_user_regs();
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
switch ( _event.vector )
{
@@ -1242,44 +1242,52 @@ static void svm_inject_event(const struct x86_event *event)
eventinj.fields.v = 1;
eventinj.fields.vector = _event.vector;
- /* Refer to AMD Vol 2: System Programming, 15.20 Event Injection. */
+ /*
+ * Refer to AMD Vol 2: System Programming, 15.20 Event Injection.
+ *
+ * On hardware lacking NextRIP support, and all hardware in the case of
+ * icebp, software events with trap semantics need emulating, so %rip in
+ * the trap frame points after the instruction.
+ *
+ * The x86 emulator (if requested by the x86_swint_emulate_* choice) will
+ * have performed checks such as presence/dpl/etc and believes that the
+ * event injection will succeed without faulting.
+ *
+ * The x86 emulator will always provide fault semantics for software
+ * events, with _trap.insn_len set appropriately. If the injection
+ * requires emulation, move %rip forwards at this point.
+ */
switch ( _event.type )
{
case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
- /*
- * Software interrupts (type 4) cannot be properly injected if the
- * processor doesn't support NextRIP. Without NextRIP, the emulator
- * will have performed DPL and presence checks for us, and will have
- * moved eip forward if appropriate.
- */
if ( cpu_has_svm_nrips )
- vmcb->nextrip = regs->eip + _event.insn_len;
+ vmcb->nextrip = regs->rip + _event.insn_len;
+ else
+ regs->rip += _event.insn_len;
eventinj.fields.type = X86_EVENTTYPE_SW_INTERRUPT;
break;
case X86_EVENTTYPE_PRI_SW_EXCEPTION: /* icebp */
/*
- * icebp's injection must always be emulated. Software injection help
- * in x86_emulate has moved eip forward, but NextRIP (if used) still
- * needs setting or execution will resume from 0.
+ * icebp's injection must always be emulated, as hardware does not
+ * special case HW_EXCEPTION with vector 1 (#DB) as having trap
+ * semantics.
*/
+ regs->rip += _event.insn_len;
if ( cpu_has_svm_nrips )
- vmcb->nextrip = regs->eip;
+ vmcb->nextrip = regs->rip;
eventinj.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
break;
case X86_EVENTTYPE_SW_EXCEPTION: /* int3, into */
/*
- * The AMD manual states that .type=3 (HW exception), .vector=3 or 4,
- * will perform DPL checks. Experimentally, DPL and presence checks
- * are indeed performed, even without NextRIP support.
- *
- * However without NextRIP support, the event injection still needs
- * fully emulating to get the correct eip in the trap frame, yet get
- * the correct faulting eip should a fault occur.
+ * Hardware special cases HW_EXCEPTION with vectors 3 and 4 as having
+ * trap semantics, and will perform DPL checks.
*/
if ( cpu_has_svm_nrips )
- vmcb->nextrip = regs->eip + _event.insn_len;
+ vmcb->nextrip = regs->rip + _event.insn_len;
+ else
+ regs->rip += _event.insn_len;
eventinj.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
break;
@@ -1290,6 +1298,16 @@ static void svm_inject_event(const struct x86_event *event)
break;
}
+ /*
+ * If injecting an event outside of 64bit mode, zero the upper bits of the
+ * %eip and nextrip after the adjustments above.
+ */
+ if ( !((vmcb->_efer & EFER_LMA) && vmcb->cs.attr.fields.l) )
+ {
+ regs->rip = regs->_eip;
+ vmcb->nextrip = (uint32_t)vmcb->nextrip;
+ }
+
vmcb->eventinj = eventinj;
if ( _event.vector == TRAP_page_fault )
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 3e602da..fa8d98f 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1694,8 +1694,6 @@ static int inject_swint(enum x86_swint_type type,
goto raise_exn;
}
}
-
- ctxt->regs->eip += insn_len;
}
rc = ops->inject_sw_interrupt(type, vector, insn_len, ctxt);
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 60f9105..f4bcf36 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -64,7 +64,13 @@ enum x86_swint_type {
x86_swint_int, /* 0xcd $n */
};
-/* How much help is required with software event injection? */
+/*
+ * How much help is required with software event injection?
+ *
+ * All software events return from x86_emulate() with X86EMUL_EXCEPTION and
+ * fault-like semantics. This just controls whether the emulator performs
+ * presence/dpl/etc checks and possibly raises exceptions instead.
+ */
enum x86_swint_emulation {
x86_swint_emulate_none, /* Hardware supports all software injection properly */
x86_swint_emulate_icebp,/* Help needed with `icebp` (0xf1) */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
@ 2016-12-01 16:55 ` Andrew Cooper
2016-12-02 11:32 ` Jan Beulich
2016-12-01 16:56 ` [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back Andrew Cooper
2016-12-01 16:56 ` [PATCH v4 18/24] x86/shadow: " Andrew Cooper
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:55 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
The behaviour of singlestep is to raise #DB after the instruction has been
completed, but implementing it with inject_hw_exception() causes x86_emulate()
to return X86EMUL_EXCEPTION, despite succesfully completing execution of the
instruction, including register writeback.
Instead, use a retire flag to indicate singlestep, which causes x86_emulate()
to return X86EMUL_OKAY.
Update all callers of x86_emulate() to use the new retire flag. This fixes
the behaviour of singlestep for shadow pagetable updates and mmcfg/mmio_ro
intercepts, which previously discarded the exception.
With this change, all uses of X86EMUL_EXCEPTION from x86_emulate() are
believed to have strictly fault semantics.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
---
CC: Jan Beulich <JBeulich@suse.com>
v4:
* s/is_hvm_vcpu/has_hvm_container_domain/
* Adjust comments and entry condition into the PAE second-half case.
v3:
* New
---
xen/arch/x86/hvm/emulate.c | 3 +++
xen/arch/x86/mm.c | 11 ++++++++++-
xen/arch/x86/mm/shadow/multi.c | 35 ++++++++++++++++++++++++++++++----
xen/arch/x86/x86_emulate/x86_emulate.c | 9 ++++-----
xen/arch/x86/x86_emulate/x86_emulate.h | 6 ++++++
5 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index fe62500..91c79fa 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1788,6 +1788,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
if ( rc != X86EMUL_OKAY )
return rc;
+ if ( hvmemul_ctxt->ctxt.retire.singlestep )
+ hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
new_intr_shadow = hvmemul_ctxt->intr_shadow;
/* MOV-SS instruction toggles MOV-SS shadow, else we just clear it. */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b7c7122..231c7bf 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5382,6 +5382,9 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long addr,
if ( rc == X86EMUL_UNHANDLEABLE )
goto bail;
+ if ( ptwr_ctxt.ctxt.retire.singlestep )
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
perfc_incr(ptwr_emulations);
return EXCRET_fault_fixed;
@@ -5503,7 +5506,13 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long addr,
else
rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
- return rc != X86EMUL_UNHANDLEABLE ? EXCRET_fault_fixed : 0;
+ if ( rc == X86EMUL_UNHANDLEABLE )
+ return 0;
+
+ if ( ctxt.retire.singlestep )
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
+ return EXCRET_fault_fixed;
}
void *alloc_xen_pagetable(void)
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 9ee48a8..eac2330 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3422,18 +3422,36 @@ static int sh_page_fault(struct vcpu *v,
v->arch.paging.last_write_emul_ok = 0;
#endif
+ if ( emul_ctxt.ctxt.retire.singlestep )
+ {
+ if ( has_hvm_container_domain(d) )
+ hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ else
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ }
+
#if GUEST_PAGING_LEVELS == 3 /* PAE guest */
- if ( r == X86EMUL_OKAY ) {
+ /*
+ * If there are no pending actions, emulate up to four extra instructions
+ * in the hope of catching the "second half" of a 64-bit pagetable write.
+ */
+ if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
+ {
int i, emulation_count=0;
this_cpu(trace_emulate_initial_va) = va;
- /* Emulate up to four extra instructions in the hope of catching
- * the "second half" of a 64-bit pagetable write. */
+
for ( i = 0 ; i < 4 ; i++ )
{
shadow_continue_emulation(&emul_ctxt, regs);
v->arch.paging.last_write_was_pt = 0;
r = x86_emulate(&emul_ctxt.ctxt, emul_ops);
- if ( r == X86EMUL_OKAY )
+
+ /*
+ * Only continue the search for the second half if there are no
+ * exceptions or pending actions. Otherwise, give up and re-enter
+ * the guest.
+ */
+ if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.retire.raw )
{
emulation_count++;
if ( v->arch.paging.last_write_was_pt )
@@ -3449,6 +3467,15 @@ static int sh_page_fault(struct vcpu *v,
{
perfc_incr(shadow_em_ex_fail);
TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED);
+
+ if ( emul_ctxt.ctxt.retire.singlestep )
+ {
+ if ( has_hvm_container_domain(d) )
+ hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ else
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ }
+
break; /* Don't emulate again if we failed! */
}
}
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index fa8d98f..7bc1cd9 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2415,7 +2415,6 @@ x86_emulate(
struct x86_emulate_state state;
int rc;
uint8_t b, d;
- bool tf = ctxt->regs->eflags & EFLG_TF;
struct operand src = { .reg = PTR_POISON };
struct operand dst = { .reg = PTR_POISON };
enum x86_swint_type swint_type;
@@ -5413,11 +5412,11 @@ x86_emulate(
if ( !mode_64bit() )
_regs.eip = (uint32_t)_regs.eip;
- *ctxt->regs = _regs;
+ /* Was singestepping active at the start of this instruction? */
+ if ( (rc == X86EMUL_OKAY) && (ctxt->regs->eflags & EFLG_TF) )
+ ctxt->retire.singlestep = true;
- /* Inject #DB if single-step tracing was enabled at instruction start. */
- if ( tf && (rc == X86EMUL_OKAY) && ops->inject_hw_exception )
- rc = ops->inject_hw_exception(EXC_DB, -1, ctxt) ? : X86EMUL_EXCEPTION;
+ *ctxt->regs = _regs;
done:
_put_fpu();
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index f4bcf36..5a4f9b7 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -483,6 +483,7 @@ struct x86_emulate_ctxt
bool hlt:1; /* Instruction HLTed. */
bool mov_ss:1; /* Instruction sets MOV-SS irq shadow. */
bool sti:1; /* Instruction sets STI irq shadow. */
+ bool singlestep:1; /* Singlestepping was active. */
};
} retire;
};
@@ -572,12 +573,17 @@ static inline int x86_emulate_wrapper(
struct x86_emulate_ctxt *ctxt,
const struct x86_emulate_ops *ops)
{
+ unsigned long orig_eip = ctxt->regs->eip;
int rc = x86_emulate(ctxt, ops);
/* Retire flags should only be set for successful instruction emulation. */
if ( rc != X86EMUL_OKAY )
ASSERT(ctxt->retire.raw == 0);
+ /* All cases returning X86EMUL_EXCEPTION should have fault semantics. */
+ if ( rc == X86EMUL_EXCEPTION )
+ ASSERT(ctxt->regs->eip == orig_eip);
+
return rc;
}
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
` (2 preceding siblings ...)
2016-12-01 16:55 ` [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
@ 2016-12-01 16:56 ` Andrew Cooper
2016-12-01 16:56 ` [PATCH v4 18/24] x86/shadow: " Andrew Cooper
4 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper
Use x86_emul_pagefault() rather than pv_inject_page_fault() to cause raised
pagefaults to be known to the emulator. This requires altering the callers of
x86_emulate() to properly re-inject the event.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v4:
* Tweak comment wording.
* Drop default cases.
v3:
* Split out #DB handling to an earlier part of the series
* Don't raise #GP faults for unexpected events, but do return back to the
guest.
v2:
* New
---
xen/arch/x86/mm.c | 101 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 63 insertions(+), 38 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5d59479..14552a1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5136,7 +5136,7 @@ static int ptwr_emulated_read(
if ( !__addr_ok(addr) ||
(rc = __copy_from_user(p_data, (void *)addr, bytes)) )
{
- pv_inject_page_fault(0, addr + bytes - rc); /* Read fault. */
+ x86_emul_pagefault(0, addr + bytes - rc, ctxt); /* Read fault. */
return X86EMUL_EXCEPTION;
}
@@ -5177,8 +5177,9 @@ static int ptwr_emulated_update(
addr &= ~(sizeof(paddr_t)-1);
if ( (rc = copy_from_user(&full, (void *)addr, sizeof(paddr_t))) != 0 )
{
- pv_inject_page_fault(0, /* Read fault. */
- addr + sizeof(paddr_t) - rc);
+ x86_emul_pagefault(0, /* Read fault. */
+ addr + sizeof(paddr_t) - rc,
+ &ptwr_ctxt->ctxt);
return X86EMUL_EXCEPTION;
}
/* Mask out bits provided by caller. */
@@ -5379,27 +5380,38 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long addr,
page_unlock(page);
put_page(page);
- /*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE. Such exceptions
- * now set event_pending instead. Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before. Future patches will fix this
- * properly.
- */
- if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
- rc = X86EMUL_UNHANDLEABLE;
+ /* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+ ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
- if ( rc == X86EMUL_UNHANDLEABLE )
- goto bail;
+ switch ( rc )
+ {
+ case X86EMUL_EXCEPTION:
+ /*
+ * This emulation only covers writes to pagetables which are marked
+ * read-only by Xen. We tolerate #PF (in case a concurrent pagetable
+ * update has succeeded on a different vcpu). Anything else is an
+ * emulation bug, or a guest playing with the instruction stream under
+ * Xen's feet.
+ */
+ if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+ ptwr_ctxt.ctxt.event.vector == TRAP_page_fault )
+ pv_inject_event(&ptwr_ctxt.ctxt.event);
+ else
+ gdprintk(XENLOG_WARNING,
+ "Unexpected event (type %u, vector %#x) from emulation\n",
+ ptwr_ctxt.ctxt.event.type, ptwr_ctxt.ctxt.event.vector);
- if ( ptwr_ctxt.ctxt.retire.singlestep )
- pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ /* Fallthrough */
+ case X86EMUL_OKAY:
- perfc_incr(ptwr_emulations);
- return EXCRET_fault_fixed;
+ if ( ptwr_ctxt.ctxt.retire.singlestep )
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
+ /* Fallthrough */
+ case X86EMUL_RETRY:
+ perfc_incr(ptwr_emulations);
+ return EXCRET_fault_fixed;
+ }
bail:
return 0;
@@ -5519,26 +5531,39 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long addr,
else
rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
- /*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE. Such exceptions
- * now set event_pending instead. Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before. Future patches will fix this
- * properly.
- */
- if ( rc == X86EMUL_EXCEPTION && ctxt.event_pending )
- rc = X86EMUL_UNHANDLEABLE;
+ /* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+ ASSERT(ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
- if ( rc == X86EMUL_UNHANDLEABLE )
- return 0;
+ switch ( rc )
+ {
+ case X86EMUL_EXCEPTION:
+ /*
+ * This emulation only covers writes to MMCFG space or read-only MFNs.
+ * We tolerate #PF (from hitting an adjacent page or a successful
+ * concurrent pagetable update). Anything else is an emulation bug,
+ * or a guest playing with the instruction stream under Xen's feet.
+ */
+ if ( ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+ ctxt.event.vector == TRAP_page_fault )
+ pv_inject_event(&ctxt.event);
+ else
+ gdprintk(XENLOG_WARNING,
+ "Unexpected event (type %u, vector %#x) from emulation\n",
+ ctxt.event.type, ctxt.event.vector);
+
+ /* Fallthrough */
+ case X86EMUL_OKAY:
- if ( ctxt.retire.singlestep )
- pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ if ( ctxt.retire.singlestep )
+ pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
- return EXCRET_fault_fixed;
+ /* Fallthrough */
+ case X86EMUL_RETRY:
+ perfc_incr(ptwr_emulations);
+ return EXCRET_fault_fixed;
+ }
+
+ return 0;
}
void *alloc_xen_pagetable(void)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 18/24] x86/shadow: Avoid raising faults behind the emulators back
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
` (3 preceding siblings ...)
2016-12-01 16:56 ` [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back Andrew Cooper
@ 2016-12-01 16:56 ` Andrew Cooper
2016-12-02 11:37 ` Jan Beulich
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-12-01 16:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Use x86_emul_{hw_exception,pagefault}() rather than
{pv,hvm}_inject_page_fault() and hvm_inject_hw_exception() to cause raised
faults to be known to the emulator. This requires altering the callers of
x86_emulate() to properly re-inject the event.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
---
CC: Jan Beulich <JBeulich@suse.com>
v4:
* Fix the commit message following v3's changes.
* s/is_hvm_vcpu/has_hvm_container_domain/
* Adjust description of permitted exceptions.
* Disallow #GP/#SS with non-zero error codes.
v3:
* Split out #DB handling to an earlier part of the series
* Don't inject #GP faults for unexpected events, but do reenter the guest.
v2:
* New
---
xen/arch/x86/mm/shadow/common.c | 13 ++++++-------
xen/arch/x86/mm/shadow/multi.c | 40 +++++++++++++++++++++++++++++-----------
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index f07803b..e509cc1 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -162,8 +162,9 @@ static int hvm_translate_linear_addr(
if ( !okay )
{
- hvm_inject_hw_exception(
- (seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault, 0);
+ x86_emul_hw_exception(
+ (seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault,
+ 0, &sh_ctxt->ctxt);
return X86EMUL_EXCEPTION;
}
@@ -323,7 +324,7 @@ pv_emulate_read(enum x86_segment seg,
if ( (rc = copy_from_user(p_data, (void *)offset, bytes)) != 0 )
{
- pv_inject_page_fault(0, offset + bytes - rc); /* Read fault. */
+ x86_emul_pagefault(0, offset + bytes - rc, ctxt); /* Read fault. */
return X86EMUL_EXCEPTION;
}
@@ -1720,10 +1721,8 @@ static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned long vaddr,
gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec);
if ( gfn == gfn_x(INVALID_GFN) )
{
- if ( is_hvm_vcpu(v) )
- hvm_inject_page_fault(pfec, vaddr);
- else
- pv_inject_page_fault(pfec, vaddr);
+ x86_emul_pagefault(pfec, vaddr, &sh_ctxt->ctxt);
+
return _mfn(BAD_GVA_TO_GFN);
}
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 7d9081b..fc18d65 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3373,18 +3373,36 @@ static int sh_page_fault(struct vcpu *v,
r = x86_emulate(&emul_ctxt.ctxt, emul_ops);
- /*
- * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
- * by the emulator itself to become X86EMUL_UNHANDLEABLE. Such exceptions
- * now set event_pending instead. Exceptions raised behind the back of
- * the emulator don't yet set event_pending.
- *
- * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
- * for no functional change from before. Future patches will fix this
- * properly.
- */
if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
- r = X86EMUL_UNHANDLEABLE;
+ {
+ /*
+ * This emulation covers writes to shadow pagetables. We tolerate #PF
+ * (from accesses spanning pages, concurrent paging updated from
+ * vcpus, etc) and #GP[0]/#SS[0] (from segmentation errors). Anything
+ * else is an emulation bug, or a guest playing with the instruction
+ * stream under Xen's feet.
+ */
+ if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+ ((emul_ctxt.ctxt.event.vector == TRAP_page_fault) ||
+ (((emul_ctxt.ctxt.event.vector == TRAP_gp_fault) ||
+ (emul_ctxt.ctxt.event.vector == TRAP_stack_error)) &&
+ emul_ctxt.ctxt.event.error_code == 0)) )
+ {
+ if ( has_hvm_container_domain(d) )
+ hvm_inject_event(&emul_ctxt.ctxt.event);
+ else
+ pv_inject_event(&emul_ctxt.ctxt.event);
+
+ goto emulate_done;
+ }
+ else
+ {
+ SHADOW_PRINTK(
+ "Unexpected event (type %u, vector %#x) from emulation\n",
+ emul_ctxt.ctxt.event.type, emul_ctxt.ctxt.event.vector);
+ r = X86EMUL_UNHANDLEABLE;
+ }
+ }
/*
* NB. We do not unshadow on X86EMUL_EXCEPTION. It's not clear that it
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 10/24] x86/emul: Always use fault semantics for software events
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
@ 2016-12-01 19:07 ` Boris Ostrovsky
2016-12-02 11:28 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Boris Ostrovsky @ 2016-12-01 19:07 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Tim Deegan, Suravee Suthikulpanit, Jan Beulich
On 12/01/2016 11:55 AM, Andrew Cooper wrote:
> The common case is already using fault semantics out of x86_emulate(), as that
> is how VT-x/SVM expects to inject the event (given suitable hardware support).
>
> However, x86_emulate() returning X86EMUL_EXCEPTION and also completing a
> register writeback is problematic for callers.
>
> Switch the logic to always using fault semantics, and leave svm_inject_trap()
> to fix up %eip if necessary.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
@ 2016-12-02 11:26 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-12-02 11:26 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 01.12.16 at 17:55, <andrew.cooper3@citrix.com> wrote:
> In debug builds, confirm that some properties of x86_emulate()'s behaviour
> actually hold. The first property, fixed in a previous change, is that retire
> flags are only ever set in the X86EMUL_OKAY case.
>
> While adjusting the userspace test harness to cope with ASSERT() in
> x86_emulate.h, fix a build problem introduced in c/s 122dd9575c7 "x86emul:
> in_longmode() should not ignore ->read_msr() errors" by providing an
> implementation of likely()/unlikely().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Some of the #define-ary will be able to go away again with a patch
of mine splitting out common definitions of the test harness into a
private header (which I decided to create due to the increasing
amount of things needed/wanted in both of its source files).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 10/24] x86/emul: Always use fault semantics for software events
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
2016-12-01 19:07 ` Boris Ostrovsky
@ 2016-12-02 11:28 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-12-02 11:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Tim Deegan, Suravee Suthikulpanit, Xen-devel
>>> On 01.12.16 at 17:55, <andrew.cooper3@citrix.com> wrote:
> The common case is already using fault semantics out of x86_emulate(), as that
> is how VT-x/SVM expects to inject the event (given suitable hardware support).
>
> However, x86_emulate() returning X86EMUL_EXCEPTION and also completing a
> register writeback is problematic for callers.
>
> Switch the logic to always using fault semantics, and leave svm_inject_trap()
> to fix up %eip if necessary.
>
> 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] 12+ messages in thread
* Re: [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag
2016-12-01 16:55 ` [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
@ 2016-12-02 11:32 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-12-02 11:32 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 01.12.16 at 17:55, <andrew.cooper3@citrix.com> wrote:
> The behaviour of singlestep is to raise #DB after the instruction has been
> completed, but implementing it with inject_hw_exception() causes x86_emulate()
> to return X86EMUL_EXCEPTION, despite succesfully completing execution of the
> instruction, including register writeback.
>
> Instead, use a retire flag to indicate singlestep, which causes x86_emulate()
> to return X86EMUL_OKAY.
>
> Update all callers of x86_emulate() to use the new retire flag. This fixes
> the behaviour of singlestep for shadow pagetable updates and mmcfg/mmio_ro
> intercepts, which previously discarded the exception.
>
> With this change, all uses of X86EMUL_EXCEPTION from x86_emulate() are
> believed to have strictly fault semantics.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Tim Deegan <tim@xen.org>
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] 12+ messages in thread
* Re: [PATCH v4 18/24] x86/shadow: Avoid raising faults behind the emulators back
2016-12-01 16:56 ` [PATCH v4 18/24] x86/shadow: " Andrew Cooper
@ 2016-12-02 11:37 ` Jan Beulich
2016-12-02 11:48 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-12-02 11:37 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 01.12.16 at 17:56, <andrew.cooper3@citrix.com> wrote:
> Use x86_emul_{hw_exception,pagefault}() rather than
> {pv,hvm}_inject_page_fault() and hvm_inject_hw_exception() to cause raised
> faults to be known to the emulator. This requires altering the callers of
> x86_emulate() to properly re-inject the event.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Tim Deegan <tim@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit ...
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3373,18 +3373,36 @@ static int sh_page_fault(struct vcpu *v,
>
> r = x86_emulate(&emul_ctxt.ctxt, emul_ops);
>
> - /*
> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
> - * by the emulator itself to become X86EMUL_UNHANDLEABLE. Such exceptions
> - * now set event_pending instead. Exceptions raised behind the back of
> - * the emulator don't yet set event_pending.
> - *
> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
> - * for no functional change from before. Future patches will fix this
> - * properly.
> - */
> if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
> - r = X86EMUL_UNHANDLEABLE;
> + {
> + /*
> + * This emulation covers writes to shadow pagetables. We tolerate #PF
> + * (from accesses spanning pages, concurrent paging updated from
> + * vcpus, etc) and #GP[0]/#SS[0] (from segmentation errors). Anything
> + * else is an emulation bug, or a guest playing with the instruction
> + * stream under Xen's feet.
> + */
> + if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
> + ((emul_ctxt.ctxt.event.vector == TRAP_page_fault) ||
> + (((emul_ctxt.ctxt.event.vector == TRAP_gp_fault) ||
> + (emul_ctxt.ctxt.event.vector == TRAP_stack_error)) &&
> + emul_ctxt.ctxt.event.error_code == 0)) )
> + {
> + if ( has_hvm_container_domain(d) )
> + hvm_inject_event(&emul_ctxt.ctxt.event);
> + else
> + pv_inject_event(&emul_ctxt.ctxt.event);
> +
> + goto emulate_done;
> + }
> + else
... the else following a goto is kind of pointless and imo makes the
code slightly harder to follow.
Jan
> + {
> + SHADOW_PRINTK(
> + "Unexpected event (type %u, vector %#x) from emulation\n",
> + emul_ctxt.ctxt.event.type, emul_ctxt.ctxt.event.vector);
> + r = X86EMUL_UNHANDLEABLE;
> + }
> + }
>
> /*
> * NB. We do not unshadow on X86EMUL_EXCEPTION. It's not clear that it
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 18/24] x86/shadow: Avoid raising faults behind the emulators back
2016-12-02 11:37 ` Jan Beulich
@ 2016-12-02 11:48 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-12-02 11:48 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 02/12/16 11:37, Jan Beulich wrote:
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -3373,18 +3373,36 @@ static int sh_page_fault(struct vcpu *v,
>>
>> r = x86_emulate(&emul_ctxt.ctxt, emul_ops);
>>
>> - /*
>> - * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
>> - * by the emulator itself to become X86EMUL_UNHANDLEABLE. Such exceptions
>> - * now set event_pending instead. Exceptions raised behind the back of
>> - * the emulator don't yet set event_pending.
>> - *
>> - * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
>> - * for no functional change from before. Future patches will fix this
>> - * properly.
>> - */
>> if ( r == X86EMUL_EXCEPTION && emul_ctxt.ctxt.event_pending )
>> - r = X86EMUL_UNHANDLEABLE;
>> + {
>> + /*
>> + * This emulation covers writes to shadow pagetables. We tolerate #PF
>> + * (from accesses spanning pages, concurrent paging updated from
>> + * vcpus, etc) and #GP[0]/#SS[0] (from segmentation errors). Anything
>> + * else is an emulation bug, or a guest playing with the instruction
>> + * stream under Xen's feet.
>> + */
>> + if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
>> + ((emul_ctxt.ctxt.event.vector == TRAP_page_fault) ||
>> + (((emul_ctxt.ctxt.event.vector == TRAP_gp_fault) ||
>> + (emul_ctxt.ctxt.event.vector == TRAP_stack_error)) &&
>> + emul_ctxt.ctxt.event.error_code == 0)) )
>> + {
>> + if ( has_hvm_container_domain(d) )
>> + hvm_inject_event(&emul_ctxt.ctxt.event);
>> + else
>> + pv_inject_event(&emul_ctxt.ctxt.event);
>> +
>> + goto emulate_done;
>> + }
>> + else
> ... the else following a goto is kind of pointless and imo makes the
> code slightly harder to follow.
Oh - good point. This is actually a slight behavioural change, as it
skips the trace. I will drop the goto.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-12-02 11:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
2016-12-02 11:26 ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
2016-12-01 19:07 ` Boris Ostrovsky
2016-12-02 11:28 ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
2016-12-02 11:32 ` Jan Beulich
2016-12-01 16:56 ` [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back Andrew Cooper
2016-12-01 16:56 ` [PATCH v4 18/24] x86/shadow: " Andrew Cooper
2016-12-02 11:37 ` Jan Beulich
2016-12-02 11:48 ` Andrew Cooper
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).