Re: [Xen-devel] [PATCH v2 1/3] x86: also allow REP STOS emulation acceleration

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.01.15 at 12:18, <tim@xxxxxxx> wrote:
> At 10:56 +0000 on 09 Jan (1420797418), Andrew Cooper wrote:
>> On 09/01/15 10:27, Jan Beulich wrote:
>> > While the REP MOVS acceleration appears to have helped qemu-traditional
>> > based guests, qemu-upstream (or really the respective video BIOSes)
>> > doesn't appear to benefit from that. Instead the acceleration added
>> > here provides a visible performance improvement during very early HVM
>> > guest boot.
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> > ---
>> > v2: fix asm() constraints in hvmemul_rep_stos(), as pointed out by
>> >     Andrew. Add output operand telling the compiler that "buf" is being
>> >     written.
>> 
>> Is writing buf wise? it looks like you will xfree() a wild pointer, and
>> appears to interfere the "buf != p_data" logic.
> 
> I think the constraints are correct, though the 'memory' clobber makes
> the rest of it moot.  But this:

Ah, yes, the memory clobber could be dropped now that the "=m"
is there.

>> > +            default:
>> > +                xfree(buf);
>> > +                ASSERT(!buf);
> 
> looks dodgy...

In which way? The "default" is supposed to be unreachable, and sits
in the else branch to an if(!buf), i.e. in a release build we'll correctly
free the buffer, while in a debug build the ASSERT() will trigger.

Jan


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