Date: Sun, 2 Aug 2015 12:02:12 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: Takao Indoh <indou.takao@jp.fujitsu.com>
cc: Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Vivek Goyal <vgoyal@redhat.com>, linux-kernel@vger.kernel.org,
        x86@kernel.org
Subject: Re: [PATCH RFC 1/3] x86: Add Intel PT common files
In-Reply-To: <1438145496-5932-2-git-send-email-indou.takao@jp.fujitsu.com>
Message-ID: <alpine.DEB.2.11.1508021157520.3825@nanos>
References: <1438145496-5932-1-git-send-email-indou.takao@jp.fujitsu.com> <1438145496-5932-2-git-send-email-indou.takao@jp.fujitsu.com>
User-Agent: Alpine 2.11 (DEB 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2623
Lines: 122

On Wed, 29 Jul 2015, Takao Indoh wrote:
> +/*
> + * 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,
> +};

While moving this around, can we pretty please clean that up? That
enum just pointless. None of the values is ever used and they hardly
have any value as they are just computable.

> +static inline unsigned int sizes(enum topa_sz tsz)
> +{
> +	return 1 << (tsz + 12);

12?? PAGE_SHIFT perhaps?

> +#define TOPA_SHIFT 12

Sigh.

> diff --git a/arch/x86/kernel/cpu/intel_pt_cap.c b/arch/x86/kernel/cpu/intel_pt_cap.c
> new file mode 100644
> index 0000000..a2cfbfc
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/intel_pt_cap.c
> @@ -0,0 +1,69 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/mm.h>
> +#include <asm/intel_pt.h>
> +
> +enum cpuid_regs {
> +	CR_EAX = 0,
> +	CR_ECX,
> +	CR_EDX,
> +	CR_EBX
> +};
> +
> +static u32 cpuid_cache[4 * PT_CPUID_LEAVES];

4 ? Magic constant pulled from thin air?

> +static int pt_cap_initialized;
> +
> +#define PT_CAP(_n, _l, _r, _m)						\
> +	[PT_CAP_ ## _n] = { .name = __stringify(_n), .leaf = _l,	\
> +			    .reg = _r, .mask = _m }
> +
> +static struct pt_cap_desc {
> +	const char	*name;
> +	u32		leaf;
> +	u8		reg;
> +	u32		mask;
> +} pt_caps[] = {
> +	PT_CAP(max_subleaf,		0, CR_EAX, 0xffffffff),
> +	PT_CAP(cr3_filtering,		0, CR_EBX, BIT(0)),
> +	PT_CAP(topa_output,		0, CR_ECX, BIT(0)),
> +	PT_CAP(topa_multiple_entries,	0, CR_ECX, BIT(1)),
> +	PT_CAP(payloads_lip,		0, CR_ECX, BIT(31)),
> +};
> +
> +u32 pt_cap_get(enum pt_capabilities cap)
> +{
> +	struct pt_cap_desc *cd = &pt_caps[cap];
> +	u32 c = cpuid_cache[cd->leaf * 4 + cd->reg];

Ditto

> +	unsigned int shift = __ffs(cd->mask);
> +
> +	return (c & cd->mask) >> shift;
> +}
> +
> +const char *pt_cap_name(enum pt_capabilities cap)
> +{
> +	return pt_caps[cap].name;
> +}
> +
> +int pt_cap_num(void)
> +{
> +	return ARRAY_SIZE(pt_caps);
> +}
> +
> +void __init pt_cap_init(void)
> +{
> +	int i;
> +
> +	if (pt_cap_initialized)
> +		return;
> +
> +	for (i = 0; i < PT_CPUID_LEAVES; i++) {
> +		cpuid_count(20, i,
> +			    &cpuid_cache[CR_EAX + i*4],

Once more.

Thanks,

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