Re: [RFC PATCH v2 3/5] target/arm/kvm: Implement virtual time adjustment

[Andrew Jones]

On Mon, Dec 16, 2019 at 03:40:16PM +0000, Peter Maydell wrote:
> On Mon, 16 Dec 2019 at 15:14, Peter Maydell <address@hidden> wrote:
> > How does this interact with the usual register sync to/from
> > KVM (ie kvm_arch_get_registers(), which I think will do a
> > GET_ONE_REG read of the TIMER_CNT register the way it does
> > any other sysreg, inside write_kvmstate_to_list(), plus
> > kvm_arch_set_registers() which does the write back to the
> > kernel in write_list_to_kvmstate()) ? Presumably we want this
> > version to take precedence by the set_virtual_time call
> > happening after the kvm_arch_set_registers, but is this
> > guaranteed ?
> 
> ...you might also want to look at the effects of simply
> removing the KVM_REG_ARM_TIMER_CNT entry from the
> 'non_runtime_cpregs[]' array -- in commit 4b7a6bf402bd064
> we explicitly stopped reading/writing this register's value
> to/from the kernel except for inbound migration, and it
> feels like this patchset is now rolling back that approach,
> so maybe we should also be (configurably) rolling back some
> of its implementation rather than just leaving it in place.

I feel like I already considered that, maybe even tried it, a few months
ago when I first looked at this. I must have decided against it for some
reason at the time, but I don't recall what. Now I can say the reason is
because we only do this save/restore when we transition to/from paused
state, though.

> 
> I note also that the commit message there had a remark
> about inconsistencies between VCPUs -- is the right thing
> to handle this per-VM rather than per-VCPU somehow?

per-VM would make sense, because the counters should be synchronized
among the VCPUs. KVM does that for us, though, so whichever VCPU last
restores its counter is the one that will determine the final value.

Maybe we should have a VM ioctl instead, but ATM we only have VCPU ioctls.

Thanks,
drew