>>> On 14.03.18 at 12:51, <[email protected]> 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 <[email protected]>
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
