Re: [Xen-devel] [PATCH 06/15] x86/emul: Rework emulator event injection

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/11/16 16:19, Tim Deegan wrote:

Hi,

At 15:38 +0000 on 23 Nov (1479915529), Andrew Cooper wrote:

The emulator needs to gain an understanding of interrupts and exceptions
generated by its actions.

Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
are visible to the emulator.  This removes the need for the
inject_{hw,sw}_interrupt() hooks, which are dropped and replaced with
x86_emul_{hw_exception,software_event}() instead.

The shadow pagetable and PV uses of x86_emulate() previously failed with
X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks, but this behaviour
has subtly changed.  Adjust the return value checking to cause a pending event
to fall back into the previous codepath.

No overall functional change.

AIUI this does have a change in the shadow callers in the case where
the emulated instruction would inject an event.  Previously we would
have failed the emulation, perhaps unshadowed something, and returned
to the guest to retry.

Now the emulator records the event in the context struct, updates the
register state and returns success, so we'll return on the *next*
instruction.  I think that's OK, though.

We are still passing X86EMUL_EXCEPTION back into the emulator, so nothing changes immediately from that point of view.  It will still "goto done" and skip the writeback phase.

Also, handle_mmio() and other callers of the emulator check for that
pending event and pass it to the hardware but you haven't added
anything in the shadow code to do that.  Does the event get dropped?

Yes.  That was the intended purpose of these hunks:

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index d70b1c6..84cb6b6 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3378,7 +3378,7 @@ static int sh_page_fault(struct vcpu *v,
      * would be a good unshadow hint. If we *do* decide to unshadow-on-fault
      * then it must be 'failable': we cannot require the unshadow to succeed.
      */
-    if ( r == X86EMUL_UNHANDLEABLE )
+    if ( r == X86EMUL_UNHANDLEABLE || emul_ctxt.ctxt.event_pending )
     {
         perfc_incr(shadow_fault_emulate_failed);
 #if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
@@ -3433,7 +3433,7 @@ static int sh_page_fault(struct vcpu *v,
             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 )
+            if ( r == X86EMUL_OKAY && !emul_ctxt.ctxt.event_pending )
             {
                 emulation_count++;
                 if ( v->arch.paging.last_write_was_pt )

To take the failure path any time an event is seen pending.

~Andrew

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