linux-kernel - Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87oahdfdhl.fsf@ashishki-desk.ger.corp.intel.com>
Date:	Tue, 08 Sep 2015 12:48:38 +0300
From:	Alexander Shishkin <alexander.shishkin@...ux.intel.com>
To:	Takao Indoh <indou.takao@...fujitsu.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Vivek Goyal <vgoyal@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger

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

> +/* intel_pt */
> +static struct perf_event_attr pt_attr_pt = {
> +	.config		= 0x400, /* bit10: TSCEn */

Doesn't it make sense to make these things configurable via sysfs or
whatnot?

> +static int pt_log_buf_nr_pages = 128; /* number of pages for log buffer */

Same here.

> +static struct cpumask pt_log_cpu_mask;
> +
> +static DEFINE_PER_CPU(struct perf_event *, pt_perf_event_pt);
> +static DEFINE_PER_CPU(struct perf_event *, pt_perf_event_sched);
> +static DEFINE_PER_CPU(struct perf_event *, pt_perf_event_dummy);
> +
> +/* Saved registers on panic */
> +static DEFINE_PER_CPU(u64, saved_msr_ctl);
> +static DEFINE_PER_CPU(u64, saved_msr_status);
> +static DEFINE_PER_CPU(u64, saved_msr_output_base);
> +static DEFINE_PER_CPU(u64, saved_msr_output_mask);
> +
> +void save_intel_pt_registers(void)
> +{
> +	int cpu = smp_processor_id();
> +	u64 ctl;
> +
> +	if (!cpumask_test_cpu(cpu, &pt_log_cpu_mask))
> +		return;
> +
> +	/* Save RTIT_CTL register */
> +	rdmsrl(MSR_IA32_RTIT_CTL, ctl);
> +	per_cpu(saved_msr_ctl, cpu) = ctl;
> +
> +	/* Stop tracing */
> +	ctl &= ~RTIT_CTL_TRACEEN;
> +	wrmsrl(MSR_IA32_RTIT_CTL, ctl);
> +
> +	/* Save other registers */
> +	rdmsrl(MSR_IA32_RTIT_STATUS, per_cpu(saved_msr_status, cpu));
> +	rdmsrl(MSR_IA32_RTIT_OUTPUT_BASE, per_cpu(saved_msr_output_base, cpu));
> +	rdmsrl(MSR_IA32_RTIT_OUTPUT_MASK, per_cpu(saved_msr_output_mask, cpu));

I'd really like to keep the PT msr accesses confined to the intel_pt
driver. Maybe have a similar function there? That way you could also use
pt_config_start() instead of clearing TraceEn by hand.

Do you need these saved msr values for the crash tool? I'm guessing
you'd need the write pointer to figure out where the most recent data
is. But then again, if you go the perf_event_disable() path, it'll all
happen automatically in the driver. Or rather __perf_event_disable()
type of thing since this is strictly cpu-local. Or even
event::pmu::stop() would do the trick. The buffer's write head would
then be in this_cpu_ptr(&pt_ctx)->handle.head.

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