On Thu, Aug 22, 2019 at 04:46:10PM +0100, Steven Price wrote: > On 22/08/2019 16:28, Sean Christopherson wrote: > > On Wed, Aug 21, 2019 at 04:36:50PM +0100, Steven Price wrote: > >> kvm_put_guest() is analogous to put_user() - it writes a single value to > >> the guest physical address. The implementation is built upon put_user() > >> and so it has the same single copy atomic properties. > > > > What you mean by "single copy atomic"? I.e. what guarantees does > > put_user() provide that __copy_to_user() does not? > > Single-copy atomicity is defined by the Arm architecture[1] and I'm not > going to try to go into the full details here, so this is a summary. > > For the sake of this feature what we care about is that the value > written/read cannot be "torn". In other words if there is a read (in > this case from another VCPU) that is racing with the write then the read > will either get the old value or the new value. It cannot return a > mixture. (This is of course assuming that the read is using a > single-copy atomic safe method). Thanks for the explanation. I assumed that's what you were referring to, but wanted to double check. > __copy_to_user() is implemented as a memcpy() and as such cannot provide > single-copy atomicity in the general case (the buffer could easily be > bigger than the architecture can guarantee). > > put_user() on the other hand is implemented (on arm64) as an explicit > store instruction and therefore is guaranteed by the architecture to be > single-copy atomic (i.e. another CPU cannot see a half-written value). I don't think kvm_put_guest() belongs in generic code, at least not with the current changelog explanation about it providing single-copy atomic semantics. AFAICT, the single-copy thing is very much an arm64 implementation detail, e.g. the vast majority of 32-bit architectures, including x86, do not provide any guarantees, and x86-64 generates more or less the same code for put_user() and __copy_to_user() for 8-byte and smaller accesses. As an alternative to kvm_put_guest() entirely, is it an option to change arm64's raw_copy_to_user() to redirect to __put_user() for sizes that are constant at compile time and can be handled by __put_user()? That would allow using kvm_write_guest() to update stolen time, albeit with arguably an even bigger dependency on the uaccess implementation details.
