On 03/30/16 22:59, Paolo Bonzini wrote: > This would have caught the bug in the previous patch.
Should this patch share a series with <http://thread.gmane.org/gmane.comp.emulators.qemu/404245>? Otherwise they could be separated by other patches in the commit history, and then "previous patch" would be misleading. (Alternatively, the reference to "previous patch" could be made by subject.) > Signed-off-by: Paolo Bonzini <[email protected]> > --- > target-i386/kvm.c | 34 ++++++++++++++++++++++++++++++---- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 19e2d94..799fdfa 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -141,6 +141,7 @@ static int kvm_get_tsc(CPUState *cs) > return ret; > } > > + assert(ret == 1); > env->tsc = msr_data.entries[0].data; > return 0; > } > @@ -1446,6 +1447,7 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu) > struct kvm_msr_entry entries[1]; > } msr_data; > struct kvm_msr_entry *msrs = msr_data.entries; > + int ret; > > if (!has_msr_tsc_deadline) { > return 0; > @@ -1457,7 +1459,13 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu) > .nmsrs = 1, > }; > > - return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + if (ret < 0) { > + return ret; > + } > + > + assert(ret == 1); > + return 0; > } This changes the return value of kvm_put_tscdeadline_msr() -- and friends below -- for successful invocations. I guess that's fine, but a note about it in the commit message would be nice. Anyway, I'm not an "expert" in this area, so the best I can offer for this two-part (almost-) series, with the commit message nits fixed, is Acked-by: Laszlo Ersek <[email protected]> Thanks Laszlo > > /* > @@ -1472,6 +1480,11 @@ static int kvm_put_msr_feature_control(X86CPU *cpu) > struct kvm_msrs info; > struct kvm_msr_entry entry; > } msr_data; > + int ret; > + > + if (!has_msr_feature_control) { > + return 0; > + } > > kvm_msr_entry_set(&msr_data.entry, MSR_IA32_FEATURE_CONTROL, > cpu->env.msr_ia32_feature_control); > @@ -1480,7 +1493,13 @@ static int kvm_put_msr_feature_control(X86CPU *cpu) > .nmsrs = 1, > }; > > - return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + if (ret < 0) { > + return ret; > + } > + > + assert(ret == 1); > + return 0; > } > > static int kvm_put_msrs(X86CPU *cpu, int level) > @@ -1492,6 +1511,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > } msr_data; > struct kvm_msr_entry *msrs = msr_data.entries; > int n = 0, i; > + int ret; > > kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs); > kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp); > @@ -1685,8 +1705,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > .nmsrs = n, > }; > > - return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data); > + if (ret < 0) { > + return ret; > + } > > + assert(ret == n); > + return 0; > } > > > @@ -2055,6 +2080,7 @@ static int kvm_get_msrs(X86CPU *cpu) > return ret; > } > > + assert(ret == n); > for (i = 0; i < ret; i++) { > uint32_t index = msrs[i].index; > switch (index) { > @@ -2511,7 +2537,7 @@ int kvm_arch_put_registers(CPUState *cpu, int level) > > assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu)); > > - if (level >= KVM_PUT_RESET_STATE && has_msr_feature_control) { > + if (level >= KVM_PUT_RESET_STATE) { > ret = kvm_put_msr_feature_control(x86_cpu); > if (ret < 0) { > return ret; >
