[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 23.11.16 at 16:38, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5377,7 +5377,7 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
> addr,
>      page_unlock(page);
>      put_page(page);
>  
> -    if ( rc == X86EMUL_UNHANDLEABLE )
> +    if ( rc == X86EMUL_UNHANDLEABLE || ptwr_ctxt.ctxt.event_pending )
>          goto bail;
>  
>      perfc_incr(ptwr_emulations);
> @@ -5501,7 +5501,8 @@ 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;
> +    return ((rc != X86EMUL_UNHANDLEABLE && !ctxt.event_pending)
> +            ? EXCRET_fault_fixed : 0);
>  }

Wouldn't these two better be adjusted to check for OKAY and RETRY,
the more that iirc we had settled on it not (yet) being guaranteed to
see event_pending set whenever getting back EXCEPTION?

> --- 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 )

Same here then.

> @@ -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 )

Aiui you need this for the swint case. But wouldn't you then need to
add similar checks in OKAY paths elsewhere? Or alternatively,
wouldn't it be better to have x86_emulate() return EXCEPTION also
for successful swint emulation (albeit that would likely require other
not very nice adjustments)?

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.