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

[Christoffer Dall]

On Thu, Jul 09, 2015 at 03:26:20PM +0100, Peter Maydell wrote:
> On 9 July 2015 at 15:17, Christoffer Dall <christoffer.dall at linaro.org> wrote:
> > 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.
> 
> > 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