Hello,
Am 26.11.2012 19:42, schrieb Will Auld:
> CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
>
> Basic design is to emulate the MSR by allowing reads and writes to the
> hypervisor vcpu specific locations to store the value of the emulated MSRs.
> In this way the IA32_TSC_ADJUST value will be included in all reads to
> the TSC MSR whether through rdmsr or rdtsc.
>
> As this is a new MSR that the guest may access and modify its value needs
> to be migrated along with the other MRSs. The changes here are specifically
> for recognizing when IA32_TSC_ADJUST is enabled in CPUID and code added
> for migrating its value.
>
> Signed-off-by: Will Auld <will.auld@xxxxxxxxx>
$subject should get a prefix of "target-i386: " and "resend" is better
used inside a tag so that it doesn't end up in the commit.
And it's "QEMU". ;)
Some more stylistic issues inline:
> ---
> target-i386/cpu.h | 2 ++
> target-i386/kvm.c | 15 +++++++++++++++
> target-i386/machine.c | 21 +++++++++++++++++++++
> 3 files changed, 38 insertions(+)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index aabf993..13d4152 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -284,6 +284,7 @@
> #define MSR_IA32_APICBASE_BSP (1<<8)
> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> +#define MSR_TSC_ADJUST 0x0000003b
Tabs. You can use scripts/checkpatch.pl to verify.
> #define MSR_IA32_TSCDEADLINE 0x6e0
>
> #define MSR_MTRRcap 0xfe
> @@ -701,6 +702,7 @@ typedef struct CPUX86State {
> uint64_t async_pf_en_msr;
>
> uint64_t tsc;
> + uint64_t tsc_adjust;
> uint64_t tsc_deadline;
>
> uint64_t mcg_status;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 696b14a..e974c42 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -61,6 +61,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>
> static bool has_msr_star;
> static bool has_msr_hsave_pa;
> +static bool has_msr_tsc_adjust;
> static bool has_msr_tsc_deadline;
> static bool has_msr_async_pf_en;
> static bool has_msr_misc_enable;
> @@ -641,6 +642,10 @@ static int kvm_get_supported_msrs(KVMState *s)
> has_msr_hsave_pa = true;
> continue;
> }
> + if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
> + has_msr_tsc_adjust = true;
> + continue;
> + }
> if (kvm_msr_list->indices[i] == MSR_IA32_TSCDEADLINE) {
> has_msr_tsc_deadline = true;
> continue;
> @@ -978,6 +983,10 @@ static int kvm_put_msrs(CPUX86State *env, int level)
> if (has_msr_hsave_pa) {
> kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
> }
> + if (has_msr_tsc_adjust) {
> + kvm_msr_entry_set(&msrs[n++],
> + MSR_TSC_ADJUST, env->tsc_adjust);
Tabs.
> + }
> if (has_msr_tsc_deadline) {
> kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSCDEADLINE, env->tsc_deadline);
> }
> @@ -1234,6 +1243,9 @@ static int kvm_get_msrs(CPUX86State *env)
> if (has_msr_hsave_pa) {
> msrs[n++].index = MSR_VM_HSAVE_PA;
> }
> + if (has_msr_tsc_adjust) {
> + msrs[n++].index = MSR_TSC_ADJUST;
> + }
> if (has_msr_tsc_deadline) {
> msrs[n++].index = MSR_IA32_TSCDEADLINE;
> }
> @@ -1308,6 +1320,9 @@ static int kvm_get_msrs(CPUX86State *env)
> case MSR_IA32_TSC:
> env->tsc = msrs[i].data;
> break;
> + case MSR_TSC_ADJUST:
> + env->tsc_adjust = msrs[i].data;
> + break;
> case MSR_IA32_TSCDEADLINE:
> env->tsc_deadline = msrs[i].data;
> break;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a8be058..95bda9b 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -310,6 +310,24 @@ static const VMStateDescription vmstate_fpop_ip_dp = {
> }
> };
>
> +static bool tsc_adjust_needed(void *opaque)
> +{
> + CPUX86State *cpu = opaque;
Please name this "env" to differentiate from CPUState / X86CPU.
Since there are other tsc_* fields already I won't request that you move
your new field to the containing X86CPU struct but at some point we will
need to convert the VMSDs to X86CPU.
> +
> + return cpu->tsc_adjust != 0;
> +}
> +
> +static const VMStateDescription vmstate_msr_tsc_adjust = {
> + .name = "cpu/msr_tsc_adjust",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_UINT64(tsc_adjust, CPUX86State),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static bool tscdeadline_needed(void *opaque)
> {
> CPUX86State *env = opaque;
> @@ -457,6 +475,9 @@ static const VMStateDescription vmstate_cpu = {
> .vmsd = &vmstate_fpop_ip_dp,
> .needed = fpop_ip_dp_needed,
> }, {
> + .vmsd = &vmstate_msr_tsc_adjust,
> + .needed = tsc_adjust_needed,
> + }, {
> .vmsd = &vmstate_msr_tscdeadline,
> .needed = tscdeadline_needed,
> }, {
Otherwise looks okay to me. I'm expecting this to go through Marcello's
queue unless I'm told otherwise.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
