On Fri, 2 Sep 2016 19:00:41 +0200
Paolo Bonzini <pbonz...@redhat.com> wrote:
> 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.
I didn't want to expose the multiplier before testing our tracing
procedure with it. So far we've been only using the TSC offset (and
it works great). I don't even know if I have a machine around to
test it, so it could take a bit.
> arch/x86/kvm/x86.c is huge; please create a new file arch/x86/kvm/debugfs.c.
Will do.
> > Signed-off-by: Luiz Capitulino <lcapitul...@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:
SVM is handled:
+ .read_cached_tsc_offset = svm_read_tsc_offset,
> - 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
Given that SVM is handled, you still want me to do this?
> 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.
I think Steven would also prefer that, but some people raised the
concern at KVM Forum that creating per vcpu dirs in debugfs may
consume considerable memory for a system running several dozen
if not hundreds of VMs. This concern seems valid to me, but I
can do either way.
