On Tue, Jul 30, 2019 at 10:25:25AM +0800, Peter Xu wrote:
> On Tue, Jul 30, 2019 at 10:12:45AM +0800, Peter Xu wrote:
> > On Mon, Jul 29, 2019 at 07:06:07PM -0700, Sean Christopherson wrote:
> > > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > > > > index d98eac371c0a..cc1f98130e6a 100644
> > > > > > --- a/arch/x86/kvm/vmx/vmx.c
> > > > > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > > > > @@ -5214,7 +5214,7 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
> > > > > > if (vmx->ple_window != old)
> > > > > > vmx->ple_window_dirty = true;
> > > > > >
> > > > > > - trace_kvm_ple_window_grow(vcpu->vcpu_id, vmx->ple_window, old);
> > > > > > + trace_kvm_ple_window_changed(vcpu->vcpu_id, vmx->ple_window, old);
> > > > >
> > > > > No need for the macro, the snippet right about already checks 'new != old'.
> > > > > Though I do like the rename, i.e. rename the trace function to
> > > > > trace_kvm_ple_window_changed().
> > > >
> > > > Do you mean this one?
> > > >
> > > > if (vmx->ple_window != old)
> > > > vmx->ple_window_dirty = true;
> > >
> > > Yep.
> > >
> > > > It didn't return, did it? :)
> > >
> > > You lost me. What's wrong with:
> > >
> > > if (vmx->ple_window != old) {
> > > vmx->ple_window_dirty = true;
> > > trace_kvm_ple_window_update(vcpu->vcpu_id, vmx_ple->window, old);
> > > }
> >
> > Yes this looks fine to me. I'll switch.
>
> Btw, I noticed we have this:
>
> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_ple_window);
>
> Is that trying to expose the tracepoints to the outter world? Is that
> whole chunk of EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_*) really needed?
It's needed to invoke tracepoints from VMX/SVM as the implementations live
in kvm.ko. Same reason functions in x86.c and company need to be exported
if they're called by VMX/SVM code.
