[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 3/5] target/arm/kvm: Implement virtual time adjustment
From: |
Peter Maydell |
Subject: |
Re: [RFC PATCH v2 3/5] target/arm/kvm: Implement virtual time adjustment |
Date: |
Mon, 16 Dec 2019 15:14:00 +0000 |
On Thu, 12 Dec 2019 at 17:33, Andrew Jones <address@hidden> wrote:
>
> When a VM is stopped (guest is paused) guest virtual time
> should stop counting. Otherwise, when the VM is resumed it
> will experience time jumps and its kernel may report soft
> lockups. Not counting virtual time while the VM is stopped
> has the side effect of making the guest's time appear to lag
> when compared with real time, and even with time derived from
> the physical counter. For this reason, this change, which is
> enabled by default, comes with a KVM CPU feature allowing it
> to be disabled, restoring legacy behavior.
>
> This patch only provides the implementation of the virtual
> time adjustment. A subsequent patch will provide the CPU
> property allowing the change to be enabled and disabled.
>
> Reported-by: Bijan Mottahedeh <address@hidden>
> Signed-off-by: Andrew Jones <address@hidden>
> ---
> target/arm/cpu.h | 9 +++++++++
> target/arm/kvm.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> target/arm/kvm32.c | 3 +++
> target/arm/kvm64.c | 3 +++
> target/arm/kvm_arm.h | 23 +++++++++++++++++++++
> 5 files changed, 86 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 83a809d4bac4..a79ea74125b3 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -821,6 +821,15 @@ struct ARMCPU {
> /* KVM init features for this CPU */
> uint32_t kvm_init_features[7];
>
> + /* KVM CPU features */
> + bool kvm_adjvtime;
> +
> + /* VCPU virtual counter value used with kvm_adjvtime */
> + uint64_t kvm_vtime;
How does this new state interact with migration ?
> +
> + /* True if the run state is, or transitioning from, RUN_STATE_PAUSED */
> + bool runstate_paused;
> +
> /* Uniprocessor system with MP extensions */
> bool mp_is_up;
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 5b82cefef608..a55fe7d7aefd 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -348,6 +348,24 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t
> devid, uint64_t group,
> memory_region_ref(kd->mr);
> }
>
> +void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
> +{
> + CPUState *cs = opaque;
> + ARMCPU *cpu = ARM_CPU(cs);
> +
> + if (running) {
> + if (cpu->kvm_adjvtime && cpu->runstate_paused) {
> + kvm_arm_set_virtual_time(cs, cpu->kvm_vtime);
> + }
> + cpu->runstate_paused = false;
> + } else if (state == RUN_STATE_PAUSED) {
> + cpu->runstate_paused = true;
> + if (cpu->kvm_adjvtime) {
> + kvm_arm_get_virtual_time(cs, &cpu->kvm_vtime);
> + }
> + }
> +}
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 ?
thanks
-- PMM
[RFC PATCH v2 4/5] tests/arm-cpu-features: Check feature default values, Andrew Jones, 2019/12/12
[RFC PATCH v2 5/5] target/arm/cpu: Add the kvm-no-adjvtime CPU property, Andrew Jones, 2019/12/12