Re: [Xen-devel] Possible issue with x86_emulate when writing results back to memory

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/01/14 16:07, Simon Graham wrote:
>>> Does this seem a reasonable explanation of the issue?
>> Yes - as long as you can also explain why a spin lock operation
>> would make it into the emulation code in the first place.
>>
> Well, that one is tough and I don't have a good answer... the only thing I 
> would say is that in our system we ALWAYS have the shadow memory tracking 
> enabled (to track changes to framebuffers)

Full shadow, or just logdirty?

With logdirty, frequent pagefaults will occur (which are costly in terms
of vmexits), but I would not expect emulation to occur.

Even with full shadow, emulation only kicks in for non-standard RAM,
which is basically IO to qemu, and instructions trying to write to the
pagetables themselves, which are trapped and emulated for safety reasons.

>
>>> The attached patch is for discussion purposes only - if it is deemed
>>> acceptable I'll resubmit a proper patch request against unstable.
>> I'd rather not add limited scope special casing like that, but instead
>> make the copying much more like real hardware (i.e. not just deal
>> with the 16- and 32-bit cases, and especially not rely on memcpy()
>> using 64-bit reads/writes when it can). IOW - don't use memcpy()
>> here at all (and have a single routine doing The Right Thing (tm)
>> rather than having two clones now, and perhaps more later on -
>> I'd in particular think that the read side in shadow code would also
>> need a similar adjustment).
> My concern was that memcpy is (I assume!) highly optimized - it certainly 
> should be if it isn't and I would worry that a change to make it atomic for 
> the purposes of instruction emulation would result in an across the board 
> perf hit when in most cases it isn't necessary that it be atomic.
>
> This would be fine for the writeback code in the shadow module BUT the 
> __hvm_copy routine is used generically in situations where atomicity is not 
> required...

__hvm_copy() is probably too low to be thinking about this.  There are
many things such as grant_copy() which do not want "hardware like" copy
properties, preferring instead to have less overhead.

~Andrew

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