Re: [PATCH 4/4] kvm: x86: export TSC offset to user-space

[Paolo Bonzini <pbonzini@redhat.com>]

On 31/08/2016 19:05, Luiz Capitulino wrote:
>   vcpu0: 18446742405270834952
>   vcpu1: 18446742405270834952
>   vcpu2: 18446742405270834952
>   vcpu3: 18446742405270834952
> 
>  - We'll probably need to export the TSC multiplier too.
>    However, I've been using only the TSC offset for now.
>    So, let's get this merged first and do the TSC multiplier
>    as a second step

You'll need to export the number of fractional bits in the multiplier,
too.  It's going to be a very simple patch, so please do everything now.

arch/x86/kvm/x86.c is huge; please create a new file arch/x86/kvm/debugfs.c.

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/svm.c              |  1 +
>  arch/x86/kvm/vmx.c              |  8 ++++++++
>  arch/x86/kvm/x86.c              | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 33ae3a4..5714bbd 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -952,6 +952,7 @@ struct kvm_x86_ops {
>  	bool (*has_wbinvd_exit)(void);
>  
>  	u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
> +	u64 (*read_cached_tsc_offset)(struct kvm_vcpu *vcpu);
>  	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
>  
>  	u64 (*read_l1_tsc)(struct kvm_vcpu *vcpu, u64 host_tsc);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index af523d8..c851477 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5065,6 +5065,7 @@ static struct kvm_x86_ops svm_x86_ops = {
>  	.has_wbinvd_exit = svm_has_wbinvd_exit,
>  
>  	.read_tsc_offset = svm_read_tsc_offset,
> +	.read_cached_tsc_offset = svm_read_tsc_offset,
>  	.write_tsc_offset = svm_write_tsc_offset,
>  	.adjust_tsc_offset_guest = svm_adjust_tsc_offset_guest,
>  	.read_l1_tsc = svm_read_l1_tsc,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5cede40..82dfe42 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -616,6 +616,7 @@ struct vcpu_vmx {
>  	u64 hv_deadline_tsc;
>  
>  	u64 current_tsc_ratio;
> +	u64 cached_tsc_offset;
>  
>  	bool guest_pkru_valid;
>  	u32 guest_pkru;
> @@ -2608,6 +2609,11 @@ static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu)
>  	return vmcs_read64(TSC_OFFSET);
>  }
>  
> +static u64 vmx_read_cached_tsc_offset(struct kvm_vcpu *vcpu)
> +{
> +	return to_vmx(vcpu)->cached_tsc_offset;
> +}
> +
>  /*
>   * writes 'offset' into guest's timestamp counter offset register
>   */
> @@ -2632,6 +2638,7 @@ static void vmx_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
>  					   vmcs_read64(TSC_OFFSET), offset);
>  		vmcs_write64(TSC_OFFSET, offset);
>  	}
> +	to_vmx(vcpu)->cached_tsc_offset = offset;
>  }
>  
>  static void vmx_adjust_tsc_offset_guest(struct kvm_vcpu *vcpu, s64 adjustment)
> @@ -11275,6 +11282,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
>  	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
>  
>  	.read_tsc_offset = vmx_read_tsc_offset,
> +	.read_cached_tsc_offset = vmx_read_cached_tsc_offset,
>  	.write_tsc_offset = vmx_write_tsc_offset,
>  	.adjust_tsc_offset_guest = vmx_adjust_tsc_offset_guest,
>  	.read_l1_tsc = vmx_read_l1_tsc,

You need to handle SVM as well.  So you might as well simplify the code:

- add a kvm_vcpu_write_tsc_offset wrapper for kvm_x86_ops->write_tsc_offset

- add a tsc_offset field in struct kvm_vcpu_arch

- replace kvm_x86_ops->read_tsc_offset with accesses to the new field

Then in a fifth patch export the TSC offset (and multiplier ;)) to
userspace.

I'm not very happy about having a single file for all TSC offsets.
Creating subdirectories under the PID-FD per-VM directory would be nicer
in the long run.

Paolo