diff mbox series

[v12,02/12] perf/x86/intel/pt: Change pt_cap_get() to a public function

Message ID 1532598182-10711-3-git-send-email-luwei.kang@intel.com (mailing list archive)
State New, archived
Headers show
Series Intel Processor Trace virtualization enabling | expand

Commit Message

Luwei Kang July 26, 2018, 9:42 a.m. UTC 
From: Chao Peng <chao.p.peng@linux.intel.com>

Change pt_cap_get() to a public function that KVM
can access this function to check if specific
feature is supported on hardware.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/events/intel/pt.c      |  3 ++-
 arch/x86/events/intel/pt.h      | 21 ---------------------
 arch/x86/include/asm/intel_pt.h | 23 +++++++++++++++++++++++
 3 files changed, 25 insertions(+), 22 deletions(-)

Comments

Thomas Gleixner Oct. 18, 2018, 8:26 a.m. UTC | #1 
On Thu, 26 Jul 2018, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
> 
> Change pt_cap_get() to a public function that KVM
> can access this function to check if specific
> feature is supported on hardware.

Changelog suggestion:

perf/x86/intel/pt: Export pt_cap_get()  

  pt_cap_get() is required by the upcoming PT support in KVM guests.

  Export it and move the capabilites enum to a global header.

Hmm?

> -static u32 pt_cap_get(enum pt_capabilities cap)
> +u32 pt_cap_get(enum pt_capabilities cap)

pt_cap_get() is fine as a local function name, but we try in general to
have clearly identifyable prefixes for global functions, types and enums.

pt_ is already used for ptrace and other things, so it makes sense to use
intel_pt_ as a prefix.

Also cap_get() is a misnomer because its not getting capabilities, it is
validating that the handed in capability is supported by the hardware.

Somehing like intel_pt_validate_cap() or intel_pt_get_supported_cap() might
be more accurate.

Thanks,

	tglx
Luwei Kang Oct. 19, 2018, 12:52 a.m. UTC | #2 
> > From: Chao Peng <chao.p.peng@linux.intel.com>
> >
> > Change pt_cap_get() to a public function that KVM can access this
> > function to check if specific feature is supported on hardware.
> 
> Changelog suggestion:
> 
> perf/x86/intel/pt: Export pt_cap_get()
> 
>   pt_cap_get() is required by the upcoming PT support in KVM guests.
> 
>   Export it and move the capabilites enum to a global header.
> 
> Hmm?
> 
> > -static u32 pt_cap_get(enum pt_capabilities cap)
> > +u32 pt_cap_get(enum pt_capabilities cap)
> 
> pt_cap_get() is fine as a local function name, but we try in general to have clearly identifyable prefixes for global functions, types and
> enums.
> 
> pt_ is already used for ptrace and other things, so it makes sense to use intel_pt_ as a prefix.
> 
> Also cap_get() is a misnomer because its not getting capabilities, it is validating that the handed in capability is supported by the hardware.
> 
> Somehing like intel_pt_validate_cap() or intel_pt_get_supported_cap() might be more accurate.
> 

Got it. Will be fixed in next version.

Thanks,
Luwei Kang
diff mbox series

Patch

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 8d016ce..9f54d8e 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -75,7 +75,7 @@ 
 	PT_CAP(psb_periods,		1, CPUID_EBX, 0xffff0000),
 };
 
-static u32 pt_cap_get(enum pt_capabilities cap)
+u32 pt_cap_get(enum pt_capabilities cap)
 {
 	struct pt_cap_desc *cd = &pt_caps[cap];
 	u32 c = pt_pmu.caps[cd->leaf * PT_CPUID_REGS_NUM + cd->reg];
@@ -83,6 +83,7 @@  static u32 pt_cap_get(enum pt_capabilities cap)
 
 	return (c & cd->mask) >> shift;
 }
+EXPORT_SYMBOL_GPL(pt_cap_get);
 
 static ssize_t pt_cap_show(struct device *cdev,
 			   struct device_attribute *attr,
diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
index 0050ca1..269e15a 100644
--- a/arch/x86/events/intel/pt.h
+++ b/arch/x86/events/intel/pt.h
@@ -45,30 +45,9 @@  struct topa_entry {
 	u64	rsvd4	: 16;
 };
 
-#define PT_CPUID_LEAVES		2
-#define PT_CPUID_REGS_NUM	4 /* number of regsters (eax, ebx, ecx, edx) */
-
 /* TSC to Core Crystal Clock Ratio */
 #define CPUID_TSC_LEAF		0x15
 
-enum pt_capabilities {
-	PT_CAP_max_subleaf = 0,
-	PT_CAP_cr3_filtering,
-	PT_CAP_psb_cyc,
-	PT_CAP_ip_filtering,
-	PT_CAP_mtc,
-	PT_CAP_ptwrite,
-	PT_CAP_power_event_trace,
-	PT_CAP_topa_output,
-	PT_CAP_topa_multiple_entries,
-	PT_CAP_single_range_output,
-	PT_CAP_payloads_lip,
-	PT_CAP_num_address_ranges,
-	PT_CAP_mtc_periods,
-	PT_CAP_cycle_thresholds,
-	PT_CAP_psb_periods,
-};
-
 struct pt_pmu {
 	struct pmu		pmu;
 	u32			caps[PT_CPUID_REGS_NUM * PT_CPUID_LEAVES];
diff --git a/arch/x86/include/asm/intel_pt.h b/arch/x86/include/asm/intel_pt.h
index b523f51..4270421 100644
--- a/arch/x86/include/asm/intel_pt.h
+++ b/arch/x86/include/asm/intel_pt.h
@@ -2,10 +2,33 @@ 
 #ifndef _ASM_X86_INTEL_PT_H
 #define _ASM_X86_INTEL_PT_H
 
+#define PT_CPUID_LEAVES		2
+#define PT_CPUID_REGS_NUM	4 /* number of regsters (eax, ebx, ecx, edx) */
+
+enum pt_capabilities {
+	PT_CAP_max_subleaf = 0,
+	PT_CAP_cr3_filtering,
+	PT_CAP_psb_cyc,
+	PT_CAP_ip_filtering,
+	PT_CAP_mtc,
+	PT_CAP_ptwrite,
+	PT_CAP_power_event_trace,
+	PT_CAP_topa_output,
+	PT_CAP_topa_multiple_entries,
+	PT_CAP_single_range_output,
+	PT_CAP_payloads_lip,
+	PT_CAP_num_address_ranges,
+	PT_CAP_mtc_periods,
+	PT_CAP_cycle_thresholds,
+	PT_CAP_psb_periods,
+};
+
 #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_INTEL)
 void cpu_emergency_stop_pt(void);
+extern u32 pt_cap_get(enum pt_capabilities cap);
 #else
 static inline void cpu_emergency_stop_pt(void) {}
+static inline u32 pt_cap_get(enum pt_capabilities cap) { return 0; }
 #endif
 
 #endif /* _ASM_X86_INTEL_PT_H */