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

From	Alexander Shishkin <alexander.shishkin@linux.intel.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger
Date	2015-09-15 14:10 +0200
Message-ID	<q8XdD-4lR-15@gated-at.bofh.it> (permalink)
References	<q6jX3-3zi-7@gated-at.bofh.it> <q6k6J-3KE-3@gated-at.bofh.it> <q6nHj-tV-7@gated-at.bofh.it> <q6DCq-6DS-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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

> On 2015/09/08 18:48, Alexander Shishkin wrote:
>> Takao Indoh <indou.takao@jp.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?
>
> That make sense, will do.
>
>> 
>>> +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.
>
> Yes, what I need is the last position where Intel PT hardware wrote
> data. Once kernel panic occurs, basically we should minimize the access
> to kernel data or functions because they may be broken. That is why I
> touch msr directly in this patch. But I agree to limit the access to msr
> except intel_pt driver. Using pmu.stop() or pt_event_stop() looks good
> to me.

Ok, thanks!

Regards,
--
Alex
--
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/

Thread

[PATCH v2 0/4] x86: Intel Processor Trace Logger Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-08 07:50 +0200
  [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-08 08:00 +0200
    Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-09-08 11:50 +0200
      Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-09 04:50 +0200
        Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-09-15 14:10 +0200
  [PATCH v2 4/4] x86: Stop Intel PT and save its registers when panic occurs Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-08 08:00 +0200
  [PATCH v2 2/4] perf: Add function to enable perf events in kernel with ring buffer Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-08 08:00 +0200
    Re: [PATCH v2 2/4] perf: Add function to enable perf events in kernel with ring buffer Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-09-08 11:40 +0200
      Re: [PATCH v2 2/4] perf: Add function to enable perf events in kernel  with ring buffer Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-09 04:20 +0200
        Re: [PATCH v2 2/4] perf: Add function to enable perf events in kernel with ring buffer Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-09-15 14:10 +0200
  [PATCH v2 1/4] perf/trace: Add function to find event type by name Takao Indoh <indou.takao@jp.fujitsu.com> - 2015-09-08 08:00 +0200

csiph-web