Re: [PATCH v1 3/5] target/arm/kvm: Implement cpu feature kvm-adjvtime

[Andrew Jones]

On Tue, Dec 10, 2019 at 03:54:11PM +0000, Peter Maydell wrote:
> On Wed, 16 Oct 2019 at 15:34, Andrew Jones <address@hidden> wrote:
> >
> > When kvm-adjvtime is enabled the guest's cntvct[_el0] won't count
> > the time when the VM is stopped. That time is skipped by updating
> > cntvoff[_el2] on each transition to vm_running using the current
> > QEMU_CLOCK_VIRTUAL time. QEMU_CLOCK_VIRTUAL only ticks when the VM
> > is running.
> >
> > This patch only provides the implementation. A subsequent patch
> > will provide the CPU property allowing the feature to be enabled.
> 
> 
> > +void kvm_arm_set_virtual_time(CPUState *cs)
> > +{
> > +    uint64_t cnt;
> > +    struct kvm_one_reg reg = {
> > +        .id = KVM_REG_ARM_TIMER_CNT,
> > +        .addr = (uintptr_t)&cnt,
> > +    };
> > +    int ret;
> > +
> > +    cnt = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> > +                   cpu_get_host_tick_frequency(),
> > +                   NANOSECONDS_PER_SECOND);
> > +
> > +    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> > +    if (ret) {
> > +        error_report("Failed to set KVM_REG_ARM_TIMER_CNT");
> > +        abort();
> > +    }
> 
> The commit message (and the doc comment for this function)
> say that we're updating the counter offset, but the
> kvm_one_reg operation here is updating the timer count
> (and relying on the kernel's handling of "if we update
> the timer count implement that by changing the offset").
> That seems a bit confusing.
> 
> Would it be possible to implement "cntvct should not change while the
> VM is stopped" with "read cntvct when the VM stops, and just write
> back that value when the VM is restarted", rather than
> "write back a new value calculated from QEMU_CLOCK_VIRTUAL"?
> If I understand commit 00f4d64ee76e873be8 correctly, that's
> basically how x86 is doing it. It would also let you sidestep
> the need to know the tick frequency of the counter.

That's definitely worth some experimenting. Will do.

Thanks,
drew