From: Zhang, Yanmin on
On Mon, 2010-04-19 at 11:59 +0300, Avi Kivity wrote:
> On 04/19/2010 11:55 AM, Zhang, Yanmin wrote:
> > On Mon, 2010-04-19 at 11:37 +0300, Avi Kivity wrote:
> >
> >> On 04/19/2010 08:32 AM, Zhang, Yanmin wrote:
> >>
> >>> Below patch introduces perf_guest_info_callbacks and related register/unregister
> >>> functions. Add more PERF_RECORD_MISC_XXX bits meaning guest kernel and guest user
> >>> space.
> >>>
> >>>
> >> This doesn't apply against upstream.
> >>
> > What does upstream mean here? The vanilla 2.6.34-rc4?
> >
>
> Yes, sorry for being unclear.
>
> >> What branch was this generated
> >> against?
> >>
> >>
> > It's against the latest tip/master. I checked out to 19b26586090 as the latest
> > tip/master has some updates on perf.
> >
>
> I don't want to merge tip/master... does tip/perf/core contain the
> needed updates?
I think so. A moment ago, I checked out to b5a80b7e9 of tip/perf/core. All 3
patches could be applied cleanly and compilation is ok. A quick testing shows
tip/perf/core kernel does work.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on
On 04/19/2010 12:22 PM, Zhang, Yanmin wrote:
>
>> I don't want to merge tip/master... does tip/perf/core contain the
>> needed updates?
>>
> I think so. A moment ago, I checked out to b5a80b7e9 of tip/perf/core. All 3
> patches could be applied cleanly and compilation is ok. A quick testing shows
> tip/perf/core kernel does work.
>
>

Thanks. I applied all three patches to the 'perf' branch in kvm.git and
merged it to master.

Ingo, please pull

git://git.kernel.org/pub/scm/virt/kvm/kvm.git perf

into tip's perf/core to receive those three patches.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Zhang, Yanmin on
On Tue, 2010-04-20 at 08:09 +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo(a)elte.hu> wrote:
>
> >
> > * Zhang, Yanmin <yanmin_zhang(a)linux.intel.com> wrote:
> >
> > > unsigned long perf_misc_flags(struct pt_regs *regs)
> > > {
> > > int misc = 0;
> > > +
> > > if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
> > > + if (perf_guest_cbs->is_user_mode())
> > > + misc |= PERF_RECORD_MISC_GUEST_USER;
> > > + else
> > > + misc |= PERF_RECORD_MISC_GUEST_KERNEL;
> > > + } else if (user_mode(regs))
> > > + misc |= PERF_RECORD_MISC_USER;
> > > + else
> > > + misc |= PERF_RECORD_MISC_KERNEL;
> > > +
> >
> > We try to use balanced curly braces. I.e.:
> >
> > if (x) {
> > boo();
> > } else {
> > if (y)
> > foo();
> > else
> > bar();
> > }
> >
> > And avoid unbalanced ones:
> >
> > if (x) {
> > boo();
> > } else
> > if (y)
> > foo();
> > else
> > bar();
>
> Note, i fixed this in the patch and applied it to perf/core. (the invalid-C
> problem was causing build failures)
Thanks for your teaching. Originally, I used {} in the 2nd half, but deleted it.

Yanmin


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Zhang, Yanmin on
On Mon, 2010-04-19 at 20:11 +0200, Ingo Molnar wrote:
> FYI, i found a few problems that need fixing:
>
> > + unsigned long ip;
> > + if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
>
> missing newline.
>
> > + int misc = 0;
> > + if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
>
> ditto.
>
> > + PERF_RECORD_MISC_GUEST_KERNEL;
> > + } else
> > + misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
> > + PERF_RECORD_MISC_KERNEL;
>
> - unbalanced curly braces
> - missing curly brace for multi-line statement.
> - unnecessary line-break due to col80 warning from checkpatch
>
> > +extern struct perf_guest_info_callbacks *perf_guest_cbs;
> > +extern int perf_register_guest_info_callbacks(
> > + struct perf_guest_info_callbacks *);
> > +extern int perf_unregister_guest_info_callbacks(
> > + struct perf_guest_info_callbacks *);
>
> - unnecessary line-break due to col80 warning from checkpatch
>
> > +static inline int perf_register_guest_info_callbacks
> > +(struct perf_guest_info_callbacks *) {return 0; }
> > +static inline int perf_unregister_guest_info_callbacks
> > +(struct perf_guest_info_callbacks *) {return 0; }
>
> - invalid C: function parameter needs name even if unused
> - missing space after opening curly brace
>
> Please provide delta fixes.

Here is the fix on the top of the prior 3 patches of V5.

From: Zhang, Yanmin <yanmin_zhang(a)linux.intel.com>

Fix some programming style issues on the top of perf kvm
enhancement V5.

Signed-off-by: Zhang Yanmin <yanmin_zhang(a)linux.intel.com>

---

diff -Nraup linux-2.6_tip0417_perfkvm/arch/x86/kernel/cpu/perf_event.c linux-2.6_tip0417_perfkvmstyle/arch/x86/kernel/cpu/perf_event.c
--- linux-2.6_tip0417_perfkvm/arch/x86/kernel/cpu/perf_event.c 2010-04-19 09:53:59.689452915 +0800
+++ linux-2.6_tip0417_perfkvmstyle/arch/x86/kernel/cpu/perf_event.c 2010-04-20 10:48:18.500999849 +0800
@@ -1752,23 +1752,29 @@ void perf_arch_fetch_caller_regs(struct
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
unsigned long ip;
+
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
ip = perf_guest_cbs->get_guest_ip();
else
ip = instruction_pointer(regs);
+
return ip;
}

unsigned long perf_misc_flags(struct pt_regs *regs)
{
int misc = 0;
+
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
- misc |= perf_guest_cbs->is_user_mode() ?
- PERF_RECORD_MISC_GUEST_USER :
- PERF_RECORD_MISC_GUEST_KERNEL;
- } else
- misc |= user_mode(regs) ? PERF_RECORD_MISC_USER :
- PERF_RECORD_MISC_KERNEL;
+ if (perf_guest_cbs->is_user_mode())
+ misc |= PERF_RECORD_MISC_GUEST_USER;
+ else
+ misc |= PERF_RECORD_MISC_GUEST_KERNEL;
+ } else if (user_mode(regs))
+ misc |= PERF_RECORD_MISC_USER;
+ else
+ misc |= PERF_RECORD_MISC_KERNEL;
+
if (regs->flags & PERF_EFLAGS_EXACT)
misc |= PERF_RECORD_MISC_EXACT;

diff -Nraup linux-2.6_tip0417_perfkvm/arch/x86/kvm/x86.c linux-2.6_tip0417_perfkvmstyle/arch/x86/kvm/x86.c
--- linux-2.6_tip0417_perfkvm/arch/x86/kvm/x86.c 2010-04-19 09:53:59.691378953 +0800
+++ linux-2.6_tip0417_perfkvmstyle/arch/x86/kvm/x86.c 2010-04-20 10:11:40.507545564 +0800
@@ -3776,16 +3776,20 @@ static int kvm_is_in_guest(void)
static int kvm_is_user_mode(void)
{
int user_mode = 3;
+
if (percpu_read(current_vcpu))
user_mode = kvm_x86_ops->get_cpl(percpu_read(current_vcpu));
+
return user_mode != 0;
}

static unsigned long kvm_get_guest_ip(void)
{
unsigned long ip = 0;
+
if (percpu_read(current_vcpu))
ip = kvm_rip_read(percpu_read(current_vcpu));
+
return ip;
}

diff -Nraup linux-2.6_tip0417_perfkvm/include/linux/perf_event.h linux-2.6_tip0417_perfkvmstyle/include/linux/perf_event.h
--- linux-2.6_tip0417_perfkvm/include/linux/perf_event.h 2010-04-19 09:53:59.691378953 +0800
+++ linux-2.6_tip0417_perfkvmstyle/include/linux/perf_event.h 2010-04-20 10:08:03.531551890 +0800
@@ -941,10 +941,8 @@ static inline void perf_event_mmap(struc
}

extern struct perf_guest_info_callbacks *perf_guest_cbs;
-extern int perf_register_guest_info_callbacks(
- struct perf_guest_info_callbacks *);
-extern int perf_unregister_guest_info_callbacks(
- struct perf_guest_info_callbacks *);
+extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
+extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);

extern void perf_event_comm(struct task_struct *tsk);
extern void perf_event_fork(struct task_struct *tsk);
@@ -1016,9 +1014,9 @@ static inline void
perf_bp_event(struct perf_event *event, void *data) { }

static inline int perf_register_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }
static inline int perf_unregister_guest_info_callbacks
-(struct perf_guest_info_callbacks *) {return 0; }
+(struct perf_guest_info_callbacks *callbacks) { return 0; }

static inline void perf_event_mmap(struct vm_area_struct *vma) { }
static inline void perf_event_comm(struct task_struct *tsk) { }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sheng Yang on
On Monday 19 April 2010 16:25:17 Avi Kivity wrote:
> On 04/17/2010 09:12 PM, Avi Kivity wrote:
> > I think you were right the first time around.
>
> Re-reading again (esp. the part about treatment of indirect NMI
> vmexits), I think this was wrong, and that the code is correct. I am
> now thoroughly confused.
>
My fault...

To my understanding now, "If an event causes a VM exit directly, it does not
update architectural state as it would have if it had it not caused the VM
exit:", means: in NMI case, NMI would involve the NMI handler, and change the
"architectural state" to NMI block. In VMX non-root mode, the behavior of
calling NMI handler changed(determine by some VMCS fields), but not the
affection to the "architectural state". So the NMI block state would remain
the same.

--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/