Hi Will,
On Tue, Nov 27, 2012 at 11:09:00AM -0800, Will Auld wrote:
> 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 a guest
> vcpu specific location to store the value of the emulated MSR while adding
> the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will
> be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This
> is of course as long as the "use TSC counter offsetting" VM-execution
> control is enabled as well as the IA32_TSC_ADJUST control.
>
> However, because hardware will only return the TSC + IA32_TSC_ADJUST + vmsc
> tsc_offset for a guest process when it does and rdtsc (with the correct
> settings) the value of our virtualized IA32_TSC_ADJUST must be stored in
> one of these three locations. The argument against storing it in the actual
> MSR is performance. This is likely to be seldom used while the save/restore
> is required on every transition. IA32_TSC_ADJUST was created as a way to
> solve some issues with writing TSC itself so that is not an option either.
> The remaining option, defined above as our solution has the problem of
> returning incorrect vmcs tsc_offset values (unless we intercept and fix, not
> done here) as mentioned above. However, more problematic is that storing the
> data in vmcs tsc_offset will have a different semantic effect on the system
> than does using the actual MSR. This is illustrated in the following example:
> The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest
> process performs a rdtsc. In this case the guest process will get TSC +
> IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including IA32_TSC_ADJUST_guest.
> While the total system semantics changed the semantics as seen by the guest
> do not and hence this will not cause a problem.
>
> Signed-off-by: Will Auld <will.auld@xxxxxxxxx>
> ---
> arch/x86/include/asm/cpufeature.h | 1 +
> arch/x86/include/asm/kvm_host.h | 3 +++
> arch/x86/include/asm/msr-index.h | 1 +
> arch/x86/kvm/cpuid.c | 2 ++
> arch/x86/kvm/cpuid.h | 8 ++++++++
> arch/x86/kvm/svm.c | 7 +++++++
> arch/x86/kvm/vmx.c | 9 +++++++++
> arch/x86/kvm/x86.c | 22 ++++++++++++++++++++++
> 8 files changed, 53 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 6b7ee5f..e574d81 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -199,6 +199,7 @@
>
> /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
> #define X86_FEATURE_FSGSBASE (9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
> +#define X86_FEATURE_TSC_ADJUST (9*32+ 1) /* TSC adjustment MSR 0x3b */
> #define X86_FEATURE_BMI1 (9*32+ 3) /* 1st group bit manipulation extensions */
> #define X86_FEATURE_HLE (9*32+ 4) /* Hardware Lock Elision */
> #define X86_FEATURE_AVX2 (9*32+ 5) /* AVX2 instructions */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index da34027..cf8c7e0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
> u32 virtual_tsc_mult;
> u32 virtual_tsc_khz;
>
> + s64 ia32_tsc_adjust_msr;
> +
> atomic_t nmi_queued; /* unprocessed asynchronous NMIs */
> unsigned nmi_pending; /* NMI queued after currently running handler */
> bool nmi_injected; /* Trying to inject an NMI this entry */
> @@ -690,6 +692,7 @@ struct kvm_x86_ops {
> bool (*has_wbinvd_exit)(void);
>
> void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale);
> + u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
> void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
>
> u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 957ec87..6486569 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -231,6 +231,7 @@
> #define MSR_IA32_EBL_CR_POWERON 0x0000002a
> #define MSR_EBC_FREQUENCY_ID 0x0000002c
> #define MSR_IA32_FEATURE_CONTROL 0x0000003a
> +#define MSR_IA32_TSC_ADJUST 0x0000003b
>
> #define FEATURE_CONTROL_LOCKED (1<<0)
> #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX (1<<1)
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0595f13..e817bac 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> if (index == 0) {
> entry->ebx &= kvm_supported_word9_x86_features;
> cpuid_mask(&entry->ebx, 9);
> + // TSC_ADJUST is emulated
> + entry->ebx |= F(TSC_ADJUST);
> } else
> entry->ebx = 0;
> entry->eax = 0;
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index a10e460..3a8b504 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct kvm_vcpu *vcpu)
> return best && (best->ecx & bit(X86_FEATURE_XSAVE));
> }
>
> +static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 7, 0);
> + return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST));
> +}
> +
> static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)
> {
> struct kvm_cpuid_entry2 *best;
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 5ac11f0..7f5e6eb 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
> svm->tsc_ratio = ratio;
> }
>
> +static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> + return svm->vmcb->control.tsc_offset;
> +}
> +
> static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> @@ -4333,6 +4339,7 @@ static struct kvm_x86_ops svm_x86_ops = {
> .has_wbinvd_exit = svm_has_wbinvd_exit,
>
> .set_tsc_khz = svm_set_tsc_khz,
> + .read_tsc_offset = svm_read_tsc_offset,
> .write_tsc_offset = svm_write_tsc_offset,
> .adjust_tsc_offset = svm_adjust_tsc_offset,
> .compute_tsc_offset = svm_compute_tsc_offset,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 819970f..0a73e72 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
> WARN(1, "user requested TSC rate below hardware speed\n");
> }
>
> +static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu)
> +{
> + return vmcs_read64(TSC_OFFSET);
> +}
> +
> /*
> * writes 'offset' into guest's timestamp counter offset register
> */
> @@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> }
> ret = kvm_set_msr_common(vcpu, msr_info);
> + case MSR_IA32_TSC_ADJUST:
> + ret = kvm_set_msr_common(vcpu, msr_info);
> + break;
> case MSR_TSC_AUX:
> if (!vmx->rdtscp_enabled)
> return 1;
> @@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
> .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
>
> .set_tsc_khz = vmx_set_tsc_khz,
> + .read_tsc_offset = vmx_read_tsc_offset,
> .write_tsc_offset = vmx_write_tsc_offset,
> .adjust_tsc_offset = vmx_adjust_tsc_offset,
> .compute_tsc_offset = vmx_compute_tsc_offset,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index edafa29..7c2990b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {
> static unsigned num_msrs_to_save;
>
> static u32 emulated_msrs[] = {
> + MSR_IA32_TSC_ADJUST,
> MSR_IA32_TSCDEADLINE,
> MSR_IA32_MISC_ENABLE,
> MSR_IA32_MCG_STATUS,
> @@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
> return tsc;
> }
>
> +static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64 offset)
> +{
> + u64 curr_offset = kvm_x86_ops->read_tsc_offset(vcpu);
Extra space.
Other than that, looks fine. Remaining comments:
- Basic functional test must be improved (testing
that wrmsr(tsc, val) reflects on tsc_adjust and
wrmsr(tsc_adjust_msr, val) reflects on tsc is
enough.
- Behaviour on reset: what is the behaviour on RESET?
- Please rebase against queue branch on kvm.git.
Thanks
--
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
