lkml.org 
[lkml]   [2011]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 8/9] KVM, VMX: Add support for guest/host-only profiling
On 10/03/2011 05:36 PM, Gleb Natapov wrote:
> On Mon, Oct 03, 2011 at 05:00:25PM +0200, Avi Kivity wrote:
> > On 10/03/2011 03:49 PM, Gleb Natapov wrote:
> > >Support guest/host-only profiling by switch perf msrs on
> > >a guest entry if needed.
> > >
> > >@@ -6052,6 +6056,26 @@ static void vmx_cancel_injection(struct kvm_vcpu *vcpu)
> > > vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, 0);
> > > }
> > >
> > >+static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
> > >+{
> > >+#ifdef CONFIG_PERF_EVENTS
> >
> > No need for #ifdef (if you also define perf_guest_get_msrs() when
> > !CONFIG_PERF_EVENTS).
> >
> Yes, but will compiler be smart enough to remove the code of the
> function completely? It will have to figure that vmx->perf_msrs_cnt is
> always 0 somehow.

It won't, but do we care?

> > >
> > >+
> > >+ perf_guest_get_msrs(vmx->perf_msrs_cnt, vmx->perf_msrs);
> > >+ for (i = 0; i< vmx->perf_msrs_cnt; i++) {
> > >+ struct perf_guest_switch_msr *msr =&vmx->perf_msrs[i];
> > >+ if (msr->host == msr->guest)
> > >+ clear_atomic_switch_msr(vmx, msr->msr);
> > >+ else
> > >+ add_atomic_switch_msr(vmx, msr->msr, msr->guest,
> > >+ msr->host);
> >
> > This generates a lot of VMWRITEs even if nothing changes, just to
> > re-set bits in the VMCS to their existing values. Need to add
> > something like this:
> >
> > if (loaded_vmcs->msr[i].host == msr->host
> > && loaded_vmcs->msr[i].guest == msr->guest)
> > continue;
> VMWRITE happens only when number of autoloaded MSRs changes (which is
> rare), not on each call to add_atomic_switch_msr(). I thought about
> optimizing this write too by doing
> vmcs_write32(VM_(ENTRY|EXIT)_MSR_LOAD_COUNT, m->nr) only once by
> checking that m->nr changed during vmentry. Can be done later.

For EFER and PERF_CTRL, it's done unconditionally, no?

> >
> > btw, shouldn't the msr autoload list be part of loaded_vmcs as well?
> >
> Why?

Any caching is only relative to the vmcs (unless we invalidate the cache
on vmcs switch).

> >
> > Do we really need a private buffer? Perhaps perf_guest_get_msrs()
> > can return a perf-internal buffer (but then, we will need to copy it
> > for the optimization above, but that's a separate issue).
> >
> The buffer will be small, so IMHO private one is not an issue. We can
> make it perf internal per cpu buffer I think.
>

I think the API is nicer with perf returning a read-only internal
buffer; this way there is no kmalloc involved since perf knows its
internal limits.

--
error compiling committee.c: too many arguments to function



\
 
 \ /
  Last update: 2011-10-04 11:31    [W:1.121 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site