Since another function mipsxx_pmu_read_counter_64() has already been
defined, the counter wide judgement should not be needed here. And
yes, U32_MASK is right here to zero out the upper 32 bits of the
64-bit return value.
Deng-Cheng
在 2011年2月17日星期四，Ralf Baechle <ralf@xxxxxxxxxxxxxx> 写道：
> On Thu, Feb 17, 2011 at 06:46:39PM +0800, Deng-Cheng Zhu wrote:
>
>> The reason of the perf-record failure on 32bit platforms is that the 32bit
>> counter read function mipsxx_pmu_read_counter() returns wrong 64bit values.
>> For example, the counter value 0x12345678 will be returned as
>> 0xffffffff12345678. So in mipspmu_event_update(), the delta will be wrong.
>> So here's a possible fix for your reference:
>>
>> --- a/arch/mips/kernel/perf_event_mipsxx.c
>> +++ b/arch/mips/kernel/perf_event_mipsxx.c
>> @@ -184,19 +184,21 @@ static unsigned int
>> mipsxx_pmu_swizzle_perf_idx(unsigned int idx)
>> return idx;
>> }
>>
>> +#define U32_MASK 0xffffffff
>> +
>> static u64 mipsxx_pmu_read_counter(unsigned int idx)
>> {
>> idx = mipsxx_pmu_swizzle_perf_idx(idx);
>>
>> switch (idx) {
>> case 0:
>> - return read_c0_perfcntr0();
>> + return read_c0_perfcntr0() & U32_MASK;
>> case 1:
>> - return read_c0_perfcntr1();
>> + return read_c0_perfcntr1() & U32_MASK;
>> case 2:
>> - return read_c0_perfcntr2();
>> + return read_c0_perfcntr2() & U32_MASK;
>> case 3:
>> - return read_c0_perfcntr3();
>> + return read_c0_perfcntr3() & U32_MASK;
>
> read_c0_perfctrl0 etc. are defined in mipsregs.h as 32-bit reads returning
> a signed int. That was ok on 32-bit kernels. To support the optional
> 64-bit counters the code will have to be changed to something like:
>
> static u64 mipsxx_pmu_read_counter(unsigned int idx)
> {
> idx = mipsxx_pmu_swizzle_perf_idx(idx);
>
> switch (idx) {
> case 0:
> if (read_c0_perfctrl0() & M_PERFCTL_WIDE)
> return read_c0_64_bit_perfcntr0();
> else
> return read_c0_32_bit_perfcntr0();
> case 1:
> if (read_c0_perfctrl1() & M_PERFCTL_WIDE)
> return read_c0_64_bit_perfcntr1();
> else
> return read_c0_32_bit_perfcntr1();
> ...
>
> And read_c0_32_bit_perfcntrX need to zero-extend their return value.
>
> Ralf
>
