Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v2 3/4] perf/x86/intel/pt: Add Intel PT logger | Date | Tue, 08 Sep 2015 12:48:38 +0300 |
| |
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?
> +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
| |