Andreas, Thanks. I just sent the update patch (V3) to address your comments. Will > -----Original Message----- > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On > Behalf Of Andreas Färber > Sent: Monday, November 26, 2012 11:05 AM > To: Auld, Will > Cc: Will Auld; qemu-devel; Gleb; Marcelo Tosatti; kvm@xxxxxxxxxxxxxxx; > Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; avi@xxxxxxxxxx > Subject: Re: [Qemu-devel] [PATCH V2] Resend - Enabling IA32_TSC_ADJUST > for Qemu KVM guest VMs > > 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 ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
