Message ID | 20220127155845.95594-1-weijiang.yang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | target/i386: Add kvm_get_one_msr helper | expand |
On 1/27/22 16:58, Yang Weijiang wrote: > @@ -135,6 +135,7 @@ static struct kvm_msr_list *kvm_feature_msrs; > > #define BUS_LOCK_SLICE_TIME 1000000000ULL /* ns */ > static RateLimit bus_lock_ratelimit_ctrl; > +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value); > > int kvm_has_pit_state2(void) > { ... > @@ -2734,6 +2721,25 @@ static int kvm_put_one_msr(X86CPU *cpu, int index, uint64_t value) > return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf); > } > > +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value) > +{ > + int ret; > + struct { > + struct kvm_msrs info; > + struct kvm_msr_entry entries[1]; > + } msr_data = { > + .info.nmsrs = 1, > + .entries[0].index = index, > + }; > + > + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); > + if (ret < 0) { > + return ret; > + } > + assert(ret == 1); > + *value = msr_data.entries[0].data; > + return ret; > +} > void kvm_put_apicbase(X86CPU *cpu, uint64_t value) > { > int ret; The patch is a good idea, but you can put the function before the uses. This way there will be no need for a forward declaration, either. Thanks, Paolo
On 1/28/2022 6:55 PM, Paolo Bonzini wrote: > On 1/27/22 16:58, Yang Weijiang wrote: >> @@ -135,6 +135,7 @@ static struct kvm_msr_list *kvm_feature_msrs; >> #define BUS_LOCK_SLICE_TIME 1000000000ULL /* ns */ >> static RateLimit bus_lock_ratelimit_ctrl; >> +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value); >> int kvm_has_pit_state2(void) >> { > > ... > >> @@ -2734,6 +2721,25 @@ static int kvm_put_one_msr(X86CPU *cpu, int >> index, uint64_t value) >> return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf); >> } >> +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value) >> +{ >> + int ret; >> + struct { >> + struct kvm_msrs info; >> + struct kvm_msr_entry entries[1]; >> + } msr_data = { >> + .info.nmsrs = 1, >> + .entries[0].index = index, >> + }; >> + >> + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); >> + if (ret < 0) { >> + return ret; >> + } >> + assert(ret == 1); >> + *value = msr_data.entries[0].data; >> + return ret; >> +} >> void kvm_put_apicbase(X86CPU *cpu, uint64_t value) >> { >> int ret; > > The patch is a good idea, but you can put the function before the > uses. This way there will be no need for a forward declaration, either. Thanks Paolo! Is v2 version required for this? > > Thanks, > > Paolo
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 2c8feb4a6f..c897dbaf60 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -135,6 +135,7 @@ static struct kvm_msr_list *kvm_feature_msrs; #define BUS_LOCK_SLICE_TIME 1000000000ULL /* ns */ static RateLimit bus_lock_ratelimit_ctrl; +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value); int kvm_has_pit_state2(void) { @@ -205,28 +206,21 @@ static int kvm_get_tsc(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); CPUX86State *env = &cpu->env; - struct { - struct kvm_msrs info; - struct kvm_msr_entry entries[1]; - } msr_data = {}; + uint64_t value; int ret; if (env->tsc_valid) { return 0; } - memset(&msr_data, 0, sizeof(msr_data)); - msr_data.info.nmsrs = 1; - msr_data.entries[0].index = MSR_IA32_TSC; env->tsc_valid = !runstate_is_running(); - ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); + ret = kvm_get_one_msr(cpu, MSR_IA32_TSC, &value); if (ret < 0) { return ret; } - assert(ret == 1); - env->tsc = msr_data.entries[0].data; + env->tsc = value; return 0; } @@ -1478,21 +1472,14 @@ static int hyperv_init_vcpu(X86CPU *cpu) * the kernel doesn't support setting vp_index; assert that its value * is in sync */ - struct { - struct kvm_msrs info; - struct kvm_msr_entry entries[1]; - } msr_data = { - .info.nmsrs = 1, - .entries[0].index = HV_X64_MSR_VP_INDEX, - }; - - ret = kvm_vcpu_ioctl(cs, KVM_GET_MSRS, &msr_data); + uint64_t value; + + ret = kvm_get_one_msr(cpu, HV_X64_MSR_VP_INDEX, &value); if (ret < 0) { return ret; } - assert(ret == 1); - if (msr_data.entries[0].data != hyperv_vp_index(CPU(cpu))) { + if (value != hyperv_vp_index(CPU(cpu))) { error_report("kernel's vp_index != QEMU's vp_index"); return -ENXIO; } @@ -2734,6 +2721,25 @@ static int kvm_put_one_msr(X86CPU *cpu, int index, uint64_t value) return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf); } +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value) +{ + int ret; + struct { + struct kvm_msrs info; + struct kvm_msr_entry entries[1]; + } msr_data = { + .info.nmsrs = 1, + .entries[0].index = index, + }; + + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); + if (ret < 0) { + return ret; + } + assert(ret == 1); + *value = msr_data.entries[0].data; + return ret; +} void kvm_put_apicbase(X86CPU *cpu, uint64_t value) { int ret;
When try to get one msr from KVM, I found there's no such kind of existing interface while kvm_put_one_msr() is there. So here comes the patch. It'll remove redundant preparation code before finally call KVM_GET_MSRS IOCTL. No functional change intended. Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- target/i386/kvm/kvm.c | 48 ++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-)