Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1611502 > unrolled thread

[PATCH V4 0/2] measure SMI cost (kernel)

Back to article view | Back to linux.kernel

Contents

  [PATCH V4 0/2] measure SMI cost (kernel) kan.liang@intel.com - 2017-03-29 04:10 +0200
    [PATCH V4 1/2] x86/msr: expose msr_flip_bit function kan.liang@intel.com - 2017-03-29 04:10 +0200
      Re: [PATCH V4 1/2] x86/msr: expose msr_flip_bit function Thomas Gleixner <tglx@linutronix.de> - 2017-03-29 07:50 +0200
        RE: [PATCH V4 1/2] x86/msr: expose msr_flip_bit function "Liang, Kan" <kan.liang@intel.com> - 2017-03-29 15:10 +0200

#1611502 — [PATCH V4 0/2] measure SMI cost (kernel)

From: Kan Liang <Kan.liang@intel.com>

Currently, there is no way to measure the time cost in System management
mode (SMM) by perf.

Intel perfmon supports FREEZE_WHILE_SMM bit in IA32_DEBUGCTL. Once it sets,
the PMU core counters will freeze on SMI handler. But it will not have an
effect on free running counters. E.g. APERF counter.
The cost of SMI can be measured by (aperf - cycles).

A new sysfs entry /sys/device/cpu/freeze_on_smi is introduced to set
FREEZE_WHILE_SMM bit in IA32_DEBUGCTL.

A new --smi-cost mode in perf stat is implemented to measure the SMI cost
by calculating cycles and aperf results. In practice, the percentages of
SMI cycles should be more useful than absolute value. So the output will be
the percentage of SMI cycles and SMI#.
If user wants to get the actual cycles, they can apply --no-metric-only.

Here is an example output.

 Performance counter stats for 'sudo echo ':

SMI cycles%          SMI#
    0.1%              1

       0.010858678 seconds time elapsed

Changes since V1:
 - Only include kernel patch
 - New functions to set msr bit on cpu and cpus.
   Using the new functions to replace rdmsrl_on_cpu and wrmsrl_on_cpu.
   That avoids the extra IPIs and atomic issue.
 - Support hotplug

Changes since V2:
 - reuse msr_info

Changes since V3:
 - Add hotplug protection
 - Thanks to Thomas's suggestion. Using msr_flip_bit interfaces to
   replace msr_set/clear_on_cpu(s) interfaces.
 - Serialize the entire setting of freeze_on_smi

Kan Liang (2):
  x86/msr: expose msr_flip_bit function
  perf/x86: add sysfs entry to freeze counter on SMI

 arch/x86/events/core.c           | 10 +++++++
 arch/x86/events/intel/core.c     | 60 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/events/perf_event.h     |  3 ++
 arch/x86/include/asm/msr-index.h |  2 ++
 arch/x86/include/asm/msr.h       |  1 +
 arch/x86/lib/msr.c               |  7 +++--
 6 files changed, 80 insertions(+), 3 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1611506 — [PATCH V4 1/2] x86/msr: expose msr_flip_bit function

From: Kan Liang <Kan.liang@intel.com>

There is no exported kernel interfaces which can flip a MSR bit. It has
to do read-modify-write operation on the MSR through rd/wrmsr*
interfaces. But the method is not atomic.

There is already __flip_bit support. Just rename and expose it.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kan Liang <Kan.liang@intel.com>
---
 arch/x86/include/asm/msr.h | 1 +
 arch/x86/lib/msr.c         | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 898dba2..c1e3026 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -312,6 +312,7 @@ struct msr *msrs_alloc(void);
 void msrs_free(struct msr *msrs);
 int msr_set_bit(u32 msr, u8 bit);
 int msr_clear_bit(u32 msr, u8 bit);
+int msr_flip_bit(u32 msr, u8 bit, bool set);
 
 #ifdef CONFIG_SMP
 int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);
diff --git a/arch/x86/lib/msr.c b/arch/x86/lib/msr.c
index 0776425..9bda539 100644
--- a/arch/x86/lib/msr.c
+++ b/arch/x86/lib/msr.c
@@ -58,7 +58,7 @@ int msr_write(u32 msr, struct msr *m)
 	return wrmsrl_safe(msr, m->q);
 }
 
-static inline int __flip_bit(u32 msr, u8 bit, bool set)
+int msr_flip_bit(u32 msr, u8 bit, bool set)
 {
 	struct msr m, m1;
 	int err = -EINVAL;
@@ -85,6 +85,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool set)
 
 	return 1;
 }
+EXPORT_SYMBOL_GPL(msr_flip_bit);
 
 /**
  * Set @bit in a MSR @msr.
@@ -96,7 +97,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool set)
  */
 int msr_set_bit(u32 msr, u8 bit)
 {
-	return __flip_bit(msr, bit, true);
+	return msr_flip_bit(msr, bit, true);
 }
 
 /**
@@ -109,7 +110,7 @@ int msr_set_bit(u32 msr, u8 bit)
  */
 int msr_clear_bit(u32 msr, u8 bit)
 {
-	return __flip_bit(msr, bit, false);
+	return msr_flip_bit(msr, bit, false);
 }
 
 #ifdef CONFIG_TRACEPOINTS
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1611597 — Re: [PATCH V4 1/2] x86/msr: expose msr_flip_bit function

On Tue, 28 Mar 2017, kan.liang@intel.com wrote:
> From: Kan Liang <Kan.liang@intel.com>
> 
> There is no exported kernel interfaces which can flip a MSR bit. It has
> to do read-modify-write operation on the MSR through rd/wrmsr*
> interfaces. But the method is not atomic.
> 
> There is already __flip_bit support. Just rename and expose it.

This function is not atomic either. Protection has to be provided by the
caller.

> -static inline int __flip_bit(u32 msr, u8 bit, bool set)
> +int msr_flip_bit(u32 msr, u8 bit, bool set)
>  {
>  	struct msr m, m1;
>  	int err = -EINVAL;
> @@ -85,6 +85,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool set)
>  
>  	return 1;
>  }
> +EXPORT_SYMBOL_GPL(msr_flip_bit);

That export is not required. The call site is always built in.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1611918 — RE: [PATCH V4 1/2] x86/msr: expose msr_flip_bit function

> > -static inline int __flip_bit(u32 msr, u8 bit, bool set)
> > +int msr_flip_bit(u32 msr, u8 bit, bool set)
> >  {
> >  	struct msr m, m1;
> >  	int err = -EINVAL;
> > @@ -85,6 +85,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool
> > set)
> >
> >  	return 1;
> >  }
> > +EXPORT_SYMBOL_GPL(msr_flip_bit);
> 
> That export is not required. The call site is always built in.
> 

If so, msr_set_bit/msr_clear_bit should be enough for our requirement.
msr_flip_bit is just a duplicate interface.
I think I will drop this patch.

Thanks,
Kan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web