Re: [PATCH v2] target/i386: Add kvm_get_one_msr helper

[Yang, Weijiang]

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;
>           }