Re: [Xen-devel] [PATCH] x86_emulate fix

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

Good point! Unfortunately the patch is also subtly wrong. We cannot access
the _sav argument while we have unpopped items on the stack. This is because
_sav is a memory argument referencing a local variable (hence is on-stack).
Hence gcc will probably emit a stack-pointer-relative effective address,
which will be incorrect because the stack pointer is different from what gcc
expects.

I'll have a think about how to fix this one.

 -- Keir

On 19/10/07 16:43, "David Lively" <dlively@xxxxxxxxxxxxxxx> wrote:

> The x86_emulate code uses the _PRE_EFLAGS macro to setup eflags
> immediately before executing (an emulated version of) the instruction.
> But _PRE_EFLAGS ends in a "andl" instruction, which clobbers the real
> eflags we've just carefully set up.  This fix simply leaves the new
> eflags value on the stack until the final "popf" into eflags.
> 
> Signed-off-by: David Lively <dlively@xxxxxxxxxxxxxxx>
> 
> diff -r 85791ff698bd xen/arch/x86/x86_emulate.c
> --- a/xen/arch/x86/x86_emulate.c Fri Oct 19 11:31:38 2007 -0400
> +++ b/xen/arch/x86/x86_emulate.c Fri Oct 19 11:31:38 2007 -0400
> @@ -300,7 +300,7 @@ struct operand {
>  
>  /* Before executing instruction: restore necessary bits in EFLAGS. */
>  #define _PRE_EFLAGS(_sav, _msk, _tmp)           \
> -/* EFLAGS = (_sav & _msk) | (EFLAGS & ~_msk); */\
> +/* push (_sav & _msk) | (EFLAGS & ~_msk); */\
>  "push %"_sav"; "                                \
>  "movl %"_msk",%"_LO32 _tmp"; "                  \
>  "andl %"_LO32 _tmp",("_STK"); "                 \
> @@ -309,11 +309,12 @@ struct operand {
>  "andl %"_LO32 _tmp",("_STK"); "                 \
>  "pop  %"_tmp"; "                                \
>  "orl  %"_LO32 _tmp",("_STK"); "                 \
> -"popf; "                                        \
>  /* _sav &= ~msk; */                             \
>  "movl %"_msk",%"_LO32 _tmp"; "                  \
>  "notl %"_LO32 _tmp"; "                          \
> -"andl %"_LO32 _tmp",%"_sav"; "
> +"andl %"_LO32 _tmp",%"_sav"; "          \
> +/* pop EFLAGS */    \
> +"popf; "
>  
>  /* After executing instruction: write-back necessary bits in EFLAGS. */
>  #define _POST_EFLAGS(_sav, _msk, _tmp)          \
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel