[RFC PATCH] KVM: arm/arm64: Don't let userspace update CNTVOFF once guest is running

On Thu, Jul 09, 2015 at 03:26:20PM +0100, Peter Maydell wrote:

On 9 July 2015 at 15:17, Christoffer Dall [off-list ref] wrote:

quoted

On Thu, Jul 09, 2015 at 02:24:06PM +0200, Christoffer Dall wrote:
So I ran this through GDB, and this happens when the guest probes the
virtio devices, specifically the backtrace tells me that

virtio_current_cpu_endian () at /root/src/qemu/hw/virtio/virtio.c:594
=> cc->virtio_is_big_endian
  -> arm_cpu_is_big_endian
    -> cpu_synchronize_state
      -> kvm_cpu_synchronize_state

which causes cpu->kvm_vcpu_dirty = true, which causes the run-loop to
write the CNTVOFF on a per-vcpu basis without stopping anything, as far
as I can tell.

Ah, I was wondering if it was going to turn out to be this,
but I hadn't figured out why it was going to cause us to do
a write-back of state rather than just a read.

quoted

So yeah, I guess the only required fix here is to fix QEMU in some way
as to not fiddle with the timer registers in this way, and then I
honestly don't know if we should try to fix legacy userspace in the
kernel, but based on the feedback from Jan, I suppose not.

arm_cpu_is_big_endian() doesn't actually want to write
back any state -- all it's interested in is reading.
So we really ought not to need to write anything back there.
kvm-all.c's sync functions don't seem to provide a "sync
kernel state to userspace but I promise I'm not going to
dirty it" function though. I guess we could add one.

(Overall it's kind of fragile even if we can avoid this
specific case where we're writing back the counter state,
though -- we should probably think about the sync level
stuff as well.)

Sounds to me like we should add the sync level stuff first, as it should
catch all callers, and afterwards consider adding the sync-one-way
optimization.

-Christoffer