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

Re: [Xen-devel] [PATCH RFC] x86/emulate: implement hvmemul_cmpxchg() with an actual CMPXCHG



>>> On 28.03.17 at 12:25, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 28/03/17 11:03, Jan Beulich wrote:
>> There is a severe downside to MMIO accesses with this model
>> though: Other than for RAM, we shouldn't be reading (and even
>> less so writing) the memory location more than once.
> 
> With MMIO, the hardware analogy is sending a single PCIe transaction
> with the atomic flag set.
> 
> However, atomic options on non-RAM locations tend not to actually be
> atomic on real hardware anyway, so so-long as we don't make multiple
> reads/writes, the lack of atomicity won't be a problem in practice.

Are you sure about this? Before caches were introduced, all LOCKed
insns uses a bus lock; I would very much assume that this model is
being followed for backwards compatibility for all LOCKed memory
accesses not going through the cache.

Furthermore I've been thinking a little more about the proposed
new memory write approach: That won't work for the original
purpose the emulator has - emulating insns in order to send get
data to/from qemu. In that case there simply is no page to map,
and this then also explains the current ->write() mechanism.
Multiple reads in this case are being avoided by going through
holding the data in struct hvm_mmio_cache. Multiple writes
_currently_ are being avoided by hvmemul_cmpxchg() forwarding
to hvmemul_write(). This property would need to be retained
for any adjustments we make.

As to the CMPXCHG insn (mis-)handling, I think it'll be best if I
come forward with the outlined x86_emulate() adjustment.
Whether that'll be acceptable for 4.9 we'll have to see.

Jan


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