Re: [Xen-devel] [PATCH] x86/pv: Fix guest crashes following f75b1a5247b "x86/pv: Drop int80_bounce from struct pv_vcpu"

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.03.18 at 12:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> The original init_int80_direct_trap() was in fact buggy; `int $0x80` is not 
> an
> exception.  This went unnoticed for years because int80_bounce and 
> trap_bounce
> were separate structures, but were combined by this change.
> 
> Exception handling is different to interrupt handling for PV guests.  By
> reusing trap_bounce, the following corner case can occur:
> 
>  * Handle a guest `int $0x80` instruction.  Latches TBF_EXCEPTION into
>    trap_bounce.
>  * Handle an exception, which emulates to success (such as ptwr support),
>    which leaves trap_bounce unmodified.
>  * The exception exit path sees TBF_EXCEPTION set and re-injects the `int
>    $0x80` a second time.

Oh, and then it was the clearing of trap_bounce after consuming it
in your conversion to C which masked the problem?

> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -373,10 +373,10 @@ UNLIKELY_END(msi_check)
>          mov   %cx, TRAPBOUNCE_cs(%rdx)
>          mov   %rdi, TRAPBOUNCE_eip(%rdx)
>  
> -        /* TB_flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */
> +        /* TB_flags = (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */
>          testb $4, 0x80 * TRAPINFO_sizeof + TRAPINFO_flags(%rsi)
>          setnz %cl
> -        lea   TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx
> +        lea   (, %rcx, TBF_INTERRUPT), %ecx

With the immediate gone I think

    shl   $3, %ecx

would be more readable and perhaps no worse code wise (the
use of LEA was introduced in cases like this only to combine the
shift with the ORing in of other flags). I won't insist on that
change though (the more that there's no symbolic constant
available for that literal 3 right now), so with or without it

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel