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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> I'd suggest putting this patch in as-is to unblock the register
> renaming, and defer any potential improvements to separate work.

May I take this as an ack then?

Jan


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