On Thu, Feb 06, 2020 at 09:41:10PM +0000, Beata Michalska wrote: > On Tue, 4 Feb 2020 at 10:34, Andrew Jones <drjones@xxxxxxxxxx> wrote: > > > > On Wed, Jan 29, 2020 at 08:24:40PM +0000, Beata Michalska wrote: > > > KVM_SET_VCPU_EVENTS might actually lead to vcpu registers being modified. > > > As such this should be the last step of sync to avoid potential overwriting > > > of whatever changes KVM might have done. > > > > > > Signed-off-by: Beata Michalska <beata.michalska@xxxxxxxxxx> > > > --- > > > target/arm/kvm32.c | 20 ++++++++++---------- > > > target/arm/kvm64.c | 20 ++++++++++---------- > > > 2 files changed, 20 insertions(+), 20 deletions(-) > > > > > > diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c > > > index 32bf8d6..cf2b47f 100644 > > > --- a/target/arm/kvm32.c > > > +++ b/target/arm/kvm32.c > > > @@ -386,17 +386,17 @@ int kvm_arch_put_registers(CPUState *cs, int level) > > > return ret; > > > } > > > > > > - ret = kvm_put_vcpu_events(cpu); > > > - if (ret) { > > > - return ret; > > > - } > > > - > > > write_cpustate_to_list(cpu, true); > > > > > > if (!write_list_to_kvmstate(cpu, level)) { > > > return EINVAL; > > > } > > > > > > + ret = kvm_put_vcpu_events(cpu); > > > + if (ret) { > > > + return ret; > > > + } > > > + > > > > I think we should put a comment above this that says basically the same > > thing as the commit message in order to explain why kvm_put_vcpu_events() > > *must* be after write_list_to_kvmstate(). > > > Will do that. > > > > kvm_arm_sync_mpstate_to_kvm(cpu); > > > > > > return ret; > > > @@ -462,11 +462,6 @@ int kvm_arch_get_registers(CPUState *cs) > > > } > > > vfp_set_fpscr(env, fpscr); > > > > > > - ret = kvm_get_vcpu_events(cpu); > > > - if (ret) { > > > - return ret; > > > - } > > > - > > > if (!write_kvmstate_to_list(cpu)) { > > > return EINVAL; > > > } > > > @@ -475,6 +470,11 @@ int kvm_arch_get_registers(CPUState *cs) > > > */ > > > write_list_to_cpustate(cpu); > > > > > > + ret = kvm_get_vcpu_events(cpu); > > > + if (ret) { > > > + return ret; > > > + } > > > + > > > > Why are we moving kvm_get_vcpu_events()? > > This is only to make things consistent with put_registeres. > There is no functional change per se. Without a functional change I wouldn't move it. It's much more appealing to have the final state writes at the bottom of this function. Thanks, drew _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm