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

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 16 Oct 2019 at 15:34, Andrew Jones <drjones@redhat.com> 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.

thanks
-- PMM