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
> [...]
>
