[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/2] x86emul: make _PRE_EFLAGS() tolerate first argument being 32-bit



On 04/01/17 11:42, Andrew Cooper wrote:
> On 04/01/17 11:38, Jan Beulich wrote:
>>>>> On 04.01.17 at 11:56, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 04/01/17 10:22, Jan Beulich wrote:
>>>> While this may appear to introduce a truncation issue, the high 32 bits
>>>> get zapped already anyway (early in _PRE_EFLAGS() as well as in
>>>> _POST_EFLAGS()). Once a subsequent patch switches to use proper 32-bit
>>>> EFLAGS operands, we'll in fact end up with more correct code, as that
>>>> zeroing of the upper halves will then go away.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>> ---
>>>> As this adds an instruction, the question is whether it would be worth
>>>> forking _PRE_EFLAGS() into two flavors: One dealing with _sav in a
>>>> register (allowing several instructions to be dropped) and another
>>>> dealing with it being on the stack (in which case the logic needs to
>>>> remain as is, since between the first PUSH and the last POP we mustn't
>>>> access variables possibly living on the stack).
>>> Looking at the code, why does so much of this need to be written in
>>> ASM?  Most looks like it could be moved into C.
>>>
>>> All that is needed in ASM is something like:
>>>
>>> push %[flags_before]
>>> popf
>>> ... op ...
>>> pushf
>>> pop %[flags_after]
>>>
>>> And the actual masking calculations can be done in C.
>> I did think about this yesterday, but came to the conclusion that it
>> can't be easily converted. Yet now that I look at the sketched out
>> code above, I can't see why I came to that conclusion.

It is always possible that my 30s thinking about this is subtly wrong...

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.