On 16 July 2015 at 12:34, Christoffer Dall <christoffer.dall@xxxxxxxxxx> 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. I think this should go into QEMU 2.4, given that it fixes a bug with time misbehaving in guests. Are you happy that it's received enough testing? (I have given 32-bit KVM a spin but have no convenient 64-bit box to test with, and besides, I didn't notice the bug in the first place :-)) > Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> > --- > Changes since RFC: > - Move cpreg_level to kvm_arm_cpreg_level and into kvm32.c and kvm64.c > - Changed struct name and declare as static const I have a couple of minor comments on the comments below, and you forgot to update the stub version of write_list_to_kvmstate(). I can just fix these up as I put it into target-arm.next, though. Fixed up version at: https://git.linaro.org/people/peter.maydell/qemu-arm.git target-arm.next If interested parties could test that by end-of-Monday that would be nice (since rc2 is scheduled for Tuesday). > dtc | 2 +- > target-arm/kvm.c | 6 +++++- > target-arm/kvm32.c | 30 +++++++++++++++++++++++++++++- > target-arm/kvm64.c | 30 +++++++++++++++++++++++++++++- > target-arm/kvm_arm.h | 12 +++++++++++- > target-arm/machine.c | 2 +- > 6 files changed, 76 insertions(+), 6 deletions(-) > > diff --git a/dtc b/dtc > index 65cc4d2..bc895d6 160000 > --- a/dtc > +++ b/dtc > @@ -1 +1 @@ > -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf > +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde Stray submodule change :-) > diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c > index d7e7d68..6769815 100644 > --- a/target-arm/kvm32.c > +++ b/target-arm/kvm32.c > @@ -153,6 +153,34 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx) > } > } > > +typedef struct CPRegStateLevel { > + uint64_t regidx; > + int level; > +} CPRegStateLevel; > + > +/* All coprocessor registers not listed in the following table are assumed to > + * be of the level KVM_PUT_RUNTIME_STATE, a register should be written less ". If a register". > + * often, you must add it to this table with a state of either > + * KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE. > + */ > +static const CPRegStateLevel non_runtime_cpregs[] = { > + { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE }, > +}; > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c > index ac34f51..d59f41c 100644 > --- a/target-arm/kvm64.c > +++ b/target-arm/kvm64.c > @@ -139,6 +139,34 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx) > } > } > > +typedef struct CPRegStateLevel { > + uint64_t regidx; > + int level; > +} CPRegStateLevel; > + > +/* All system not listed in the following table are assumed to be of the level "system registers" > + * KVM_PUT_RUNTIME_STATE, a register should be written less often, you must ". If a register" > + * add it to this table with a state of either KVM_PUT_RESET_STATE or > + * KVM_PUT_FULL_STATE. > + */ > +static const CPRegStateLevel non_runtime_cpregs[] = { > + { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE }, > +}; > --- a/target-arm/kvm_arm.h > +++ b/target-arm/kvm_arm.h > @@ -83,7 +93,7 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx); > * Note that we do not stop early on failure -- we will attempt > * writing all registers in the list. > */ > -bool write_list_to_kvmstate(ARMCPU *cpu); > +bool write_list_to_kvmstate(ARMCPU *cpu, int level); You forgot to update the stub function in target-arm/kvm-stub.c, so this breaks compilation on non-ARM hosts. thanks -- PMM _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
