[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] target-arm: kvm: Differentiate registers ba
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [RFC PATCH] target-arm: kvm: Differentiate registers based on write-back levels |
Date: |
Fri, 10 Jul 2015 12:22:31 +0100 |
On 10 July 2015 at 12:00, Christoffer Dall <address@hidden> wrote:
> Some registers like the CNTVCT register should only be written to the
> kernel as part of machine initialization or on vmload operations, but
> never during runtime, as this can potentially make time go backwards or
> create inconsistent time observations between VCPUs.
>
> Introduce a list of registers that should not be written back at runtime
> and check this list on syncing the register state to the KVM state.
Thanks for picking this one up...
> Signed-off-by: Christoffer Dall <address@hidden>
> ---
> target-arm/kvm.c | 34 +++++++++++++++++++++++++++++++++-
> target-arm/kvm32.c | 2 +-
> target-arm/kvm64.c | 2 +-
> target-arm/kvm_arm.h | 3 ++-
> target-arm/machine.c | 2 +-
> 5 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 548bfd7..2e92699 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -409,7 +409,35 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
> return ok;
> }
>
> -bool write_list_to_kvmstate(ARMCPU *cpu)
> +typedef struct cpreg_state_level {
> + uint64_t kvm_idx;
> + int level;
> +} cpreg_state_level;
(QEMU's coding style prefers CPRegStateLevel for struct types.)
> +
> +/* All system registers not listed in the following table are assumed to be
> + * of the level KVM_PUT_RUNTIME_STATE, a register should be written less
> + * often, you must add it to this table with a state of either
> + * KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
> + */
> +cpreg_state_level non_runtime_cpregs[] = {
> + { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
This should be KVM_PUT_RESET_STATE, right?
> +};
The other option here would be to keep the level information
in the cpreg structs (which is where we put everything else
we know about cpregs); we'd probably need to then initialise
some other data structure if we wanted to avoid the hash
table lookup for every register in write_list_to_kvmstate.
I guess if we expect this list to remain a fairly small
set of exceptional cases then this is OK (and vaguely
comparable to the existing kvm_arm_reg_syncs_via-cpreg_list
handling).
Don't we need separate 32-bit and 64-bit versions of
this list?
thanks
-- PMM