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

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

I checked in an alternative as changeset 16143.

 K.

On 19/10/07 17:59, "Dave Lively" <dlively@xxxxxxxxxxxxxxx> wrote:

> 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