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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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