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

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.

Reported-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Suggested-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h     |  3 +++
 target/arm/kvm.c     | 32 ++++++++++++++++++++++++++++++++
 target/arm/kvm32.c   |  3 +++
 target/arm/kvm64.c   |  3 +++
 target/arm/kvm_arm.h | 14 ++++++++++++++
 5 files changed, 55 insertions(+)

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

On Tue, Dec 10, 2019 at 03:54:11PM +0000, Peter Maydell wrote:
> 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.

That's definitely worth some experimenting. Will do.

Thanks,
drew