qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/2] X86: Move rdmsr/wrmsr functionality to s


From: Julian Kirsch
Subject: Re: [Qemu-devel] [PATCH v3 1/2] X86: Move rdmsr/wrmsr functionality to standalone functions
Date: Wed, 15 Mar 2017 13:59:20 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0

Hi Eduardo,

thanks for taking the time to look through the patch series. To recapitulate for
the next iteration:

1.) Let the rd/wrmsr functions set the valid variable in case of
    CONFIG_USER_ONLY being set.
2.) Split up patch into code movement followed by reordering
3.) I included only MSR_KVM_SYSTEM_TIME_NEW because exposing MSR_KVM_SYSTEM_TIME
    to users will hardcode its presence forever, and from the comments next
    to the macro definitions I figured MSR_KVM_SYSTEM_TIME should not be used
    anymore.
4.) HyperV MSRs were assumed to be x86_64 specific because they have X64
    in their names, but it seems I was wrong on this one. Sorry for being
    lured simply due to the naming convention.

Best,
Julian

On 14.03.2017 23:33, Eduardo Habkost wrote:
> Found something else that confused me:
> 
> On Tue, Mar 14, 2017 at 02:16:32PM -0300, Eduardo Habkost wrote:
> [...]
>> +#if defined CONFIG_KVM && defined TARGET_X86_64
> 
> Why exactly are you making the hyperv MSRs x86_64-specific? I
> don't see anything at the QEMU code or kernel-side KVM code that
> makes it x86_64-specific.
> 
>> +    case HV_X64_MSR_HYPERCALL:
>> +        val = env->msr_hv_hypercall;
>> +        break;
>> +    case HV_X64_MSR_GUEST_OS_ID:
>> +        val = env->msr_hv_guest_os_id;
>> +        break;
>> +    case HV_X64_MSR_APIC_ASSIST_PAGE:
>> +        val = env->msr_hv_vapic;
>> +        break;
>> +    case HV_X64_MSR_REFERENCE_TSC:
>> +        val = env->msr_hv_tsc;
>> +        break;
>> +    case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
>> +        val = env->msr_hv_crash_params[idx - HV_X64_MSR_CRASH_P0];
>> +        break;
>> +    case HV_X64_MSR_VP_RUNTIME:
>> +        val = env->msr_hv_runtime;
>> +        break;
>> +    case HV_X64_MSR_SCONTROL:
>> +        val = env->msr_hv_synic_control;
>> +        break;
>> +    case HV_X64_MSR_SVERSION:
>> +        val = env->msr_hv_synic_version;
>> +        break;
>> +    case HV_X64_MSR_SIEFP:
>> +        val = env->msr_hv_synic_evt_page;
>> +        break;
>> +    case HV_X64_MSR_SIMP:
>> +        val = env->msr_hv_synic_msg_page;
>> +        break;
>> +    case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
>> +        val = env->msr_hv_synic_sint[idx - HV_X64_MSR_SINT0];
>> +        break;
>> +    case HV_X64_MSR_STIMER0_CONFIG:
>> +    case HV_X64_MSR_STIMER1_CONFIG:
>> +    case HV_X64_MSR_STIMER2_CONFIG:
>> +    case HV_X64_MSR_STIMER3_CONFIG:
>> +        val = env->msr_hv_stimer_config[(idx - HV_X64_MSR_STIMER0_CONFIG) / 
>> 2];
>> +        break;
>> +    case HV_X64_MSR_STIMER0_COUNT:
>> +    case HV_X64_MSR_STIMER1_COUNT:
>> +    case HV_X64_MSR_STIMER2_COUNT:
>> +    case HV_X64_MSR_STIMER3_COUNT:
>> +        val = env->msr_hv_stimer_count[(idx - HV_X64_MSR_STIMER0_COUNT) / 
>> 2];
>> +        break;
>> +#endif
> [...]
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]