On Fri, Jul 17, 2015 at 03:29:56PM +0100, Peter Maydell wrote: > 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 :-)) I tested this on both Juno and Mustang, with a simple loop kernel booting and doing hackbench test, but if we want to be on the extra careful side, perhaps Alex can run it through his migration test setup? I don't think that's necessary though. > > > 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 :-) > Damn, keeps happening to me. ok, I'll stop using git commit -a for qemu changes. > > 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. > whoops, who cares about non-ARM hosts anyway. Thanks for fixing these up, the fixed up version looks good. -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
