Re: [PATCH] x86: Clean up files of Intel Processor Trace

[Alexander Shishkin <alexander.shishkin@linux.intel.com>]

Takao Indoh <indou.takao@jp.fujitsu.com> writes:

> This patch just cleans up some files of Intel Processor Trace, does not
> change its behavior. Removing unused definition, replace a constant
> value with macro, etc.
>
> Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
> ---
>  arch/x86/kernel/cpu/intel_pt.h            |   33 +++++-----------------------
>  arch/x86/kernel/cpu/perf_event_intel_pt.c |   14 ++++++------
>  2 files changed, 13 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/intel_pt.h b/arch/x86/kernel/cpu/intel_pt.h
> index 1c338b0..6b48ba8 100644
> --- a/arch/x86/kernel/cpu/intel_pt.h
> +++ b/arch/x86/kernel/cpu/intel_pt.h
> @@ -25,32 +25,11 @@
>   */
>  #define TOPA_PMI_MARGIN 512
>  
> -/*
> - * Table of Physical Addresses bits
> - */
> -enum topa_sz {
> -	TOPA_4K	= 0,
> -	TOPA_8K,
> -	TOPA_16K,
> -	TOPA_32K,
> -	TOPA_64K,
> -	TOPA_128K,
> -	TOPA_256K,
> -	TOPA_512K,
> -	TOPA_1MB,
> -	TOPA_2MB,
> -	TOPA_4MB,
> -	TOPA_8MB,
> -	TOPA_16MB,
> -	TOPA_32MB,
> -	TOPA_64MB,
> -	TOPA_128MB,
> -	TOPA_SZ_END,
> -};
> +#define TOPA_SHIFT PAGE_SHIFT

Even though TOPA_SHIFT happens to be the same as PAGE_SHIFT, it is a
property of a separate hardware block, not mmu. PAGE_SHIFT is 12, but
12 is not always PAGE_SHIFT.

> -static inline unsigned int sizes(enum topa_sz tsz)
> +static inline unsigned int sizes(unsigned int tsz)
>  {
> -	return 1 << (tsz + 12);
> +	return 1 << (tsz + TOPA_SHIFT);
>  };
>  
>  struct topa_entry {
> @@ -66,8 +45,8 @@ struct topa_entry {
>  	u64	rsvd4	: 16;
>  };
>  
> -#define TOPA_SHIFT 12
> -#define PT_CPUID_LEAVES 2
> +#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,
> @@ -79,7 +58,7 @@ enum pt_capabilities {
>  
>  struct pt_pmu {
>  	struct pmu		pmu;
> -	u32			caps[4 * PT_CPUID_LEAVES];
> +	u32			caps[PT_CPUID_REGS_NUM * PT_CPUID_LEAVES];
>  };
>  
>  /**
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_pt.c b/arch/x86/kernel/cpu/perf_event_intel_pt.c
> index 183de71..1e7d89e 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_pt.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_pt.c
> @@ -37,9 +37,9 @@ static struct pt_pmu pt_pmu;
>  
>  enum cpuid_regs {
>  	CR_EAX = 0,
> +	CR_EBX,
>  	CR_ECX,
> -	CR_EDX,
> -	CR_EBX
> +	CR_EDX
>  };

This doesn't seem necessary.

Thanks,
--
Alex