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

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2808 bytes --]

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

[-- Attachment #1.2: Type: text/html, Size: 3695 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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