| Message ID | 20220129230719.111339-1-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/i386: Add kvm_get_one_msr helper | expand |
Ping... Does this v2 meet the requirement? On 1/30/2022 7:07 AM, Yang Weijiang wrote: > 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. > > v2: > Per Paolo's suggestion, move the helper before uses to eliminate > a forward declaration. > > base-commit: 48302d4eb628ff0bea4d7e92cbf6b726410eb4c3 > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > target/i386/kvm/kvm.c | 48 ++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 2c8feb4a6f..627535395a 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -201,32 +201,45 @@ bool kvm_hv_vpindex_settable(void) > return hv_vpindex_settable; > } > > -static int kvm_get_tsc(CPUState *cs) > +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value) > { > - X86CPU *cpu = X86_CPU(cs); > - CPUX86State *env = &cpu->env; > + int ret; > struct { > struct kvm_msrs info; > struct kvm_msr_entry entries[1]; > - } msr_data = {}; > + } 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; > +} > + > +static int kvm_get_tsc(CPUState *cs) > +{ > + X86CPU *cpu = X86_CPU(cs); > + CPUX86State *env = &cpu->env; > + 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 +1491,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; > }
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 2c8feb4a6f..627535395a 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -201,32 +201,45 @@ bool kvm_hv_vpindex_settable(void) return hv_vpindex_settable; } -static int kvm_get_tsc(CPUState *cs) +static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value) { - X86CPU *cpu = X86_CPU(cs); - CPUX86State *env = &cpu->env; + int ret; struct { struct kvm_msrs info; struct kvm_msr_entry entries[1]; - } msr_data = {}; + } 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; +} + +static int kvm_get_tsc(CPUState *cs) +{ + X86CPU *cpu = X86_CPU(cs); + CPUX86State *env = &cpu->env; + 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 +1491,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; }
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. v2: Per Paolo's suggestion, move the helper before uses to eliminate a forward declaration. base-commit: 48302d4eb628ff0bea4d7e92cbf6b726410eb4c3 Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- target/i386/kvm/kvm.c | 48 ++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-)