Messages in this thread | | | Subject | Re: [PATCH v2 5/5] arm64: perf: Add support for chaining event counters | From | Suzuki K Poulose <> | Date | Mon, 11 Jun 2018 17:18:58 +0100 |
| |
On 11/06/18 15:24, Mark Rutland wrote: > On Mon, Jun 11, 2018 at 02:54:16PM +0100, Suzuki K Poulose wrote: >> On 08/06/18 15:46, Suzuki K Poulose wrote: >>> On 06/06/2018 07:01 PM, Mark Rutland wrote: >>>> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote: >> >>>>> - value |= 0xffffffff00000000ULL; >>>>> + if (!armv8pmu_event_is_64bit(event)) >>>>> + value |= 0xffffffff00000000ULL; >>>>> write_sysreg(value, pmccntr_el0); >>>>> - } else if (armv8pmu_select_counter(idx) == idx) >>>>> - write_sysreg(value, pmxevcntr_el0); >>>>> + } else >>>>> + armv8pmu_write_hw_counter(event, value); >>>>> } >>>> >>>>> +static inline void armv8pmu_write_event_type(struct perf_event *event) >>>>> +{ >>>>> + struct hw_perf_event *hwc = &event->hw; >>>>> + int idx = hwc->idx; >>>>> + >>>>> + /* >>>>> + * For chained events, write the the low counter event type >>>>> + * followed by the high counter. The high counter is programmed >>>>> + * with CHAIN event code with filters set to count at all ELs. >>>>> + */ >>>>> + if (armv8pmu_event_is_chained(event)) { >>>>> + u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN | >>>>> + ARMV8_PMU_INCLUDE_EL2; >>>>> + >>>>> + armv8pmu_write_evtype(idx - 1, hwc->config_base); >>>>> + isb(); >>>>> + armv8pmu_write_evtype(idx, chain_evt); >>>> >>>> The ISB isn't necessary here, AFAICT. We only do this while the PMU is >>>> disabled; no? >>> >>> You're right. I was just following the ARM ARM. >> >> Taking another look, it is not clear about the semantics of "pmu->enable()" >> and pmu->disable() callbacks. > > I was talking about pmu::{pmu_disable,pmu_enable}(), so I'm not sure I > follow how arm_pmu::{enable,disable}() are relevant here.> > The arm_pmu::{enable,disable}() callbacks enable or disable individual > counters. For example, leaving unused counters disabled may save power, > even if the PMU as a whole is enabled.
Ah, I mistook cpu_pmu->enable/disable for the core pmu ops. My bad. Sorry about the noise.
Suzuki
| |