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

["Dave Lively" <dlively@xxxxxxxxxxxxxxx>]

You're right.  We don't currently see this behavior since we build
with frame pointers, so those local variable references are
%bp-relative instead of %sp-relative.  (But -fomit-frame-pointers is
the default, and this definitely won't work there.)

it seems fairly easy to fix with another stack temp or (better)
register.  But it would be nice to find a solution that doesn't
require another temp.  (I'm looking too ...)

Dave

On 10/19/07, Keir Fraser <Keir.Fraser@xxxxxxxxxxxx> wrote:
> 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
>

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