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

From	Takao Indoh <indou.takao@jp.fujitsu.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger
Date	2015-09-09 04:50 +0200
Message-ID	<q6DCq-6DS-5@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>
Organization	linux.* mail to news gateway

Show all headers | View raw

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.

Thanks,
Takao Indoh


> 
> Thanks,
> --
> 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