lkml.org 
[lkml]   [2023]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] arm64/arm: perf: Don't discard upper 32 bits from PMCEID0/1 registers

Hi Mark,

On Fri, 27 Oct 2023, Mark Rutland wrote:
> On Thu, Oct 26, 2023 at 06:22:43PM -0700, Ilkka Koskinen wrote:
>> The upper 32 bits of PMCEID[n] registers are used to describe whether
>> architectural and microarchitectural events in range 0x4000-0x401f
>> exist. Due to discarding the bits, the driver made the events invisible,
>> even if they existed.
>>
>> Fixes: df29ddf4f04b ("arm64: perf: Abstract system register accesses away")
>
> Hmm... are we silently truncating anything else?
>
> I see that read_pmxevcntr() and write_pmxevcntr() treat PMXEVCNTR_EL0 as 32-bit
> despite it being 64-bit with PMUv3.5, but it doesn't matter as those functions
> are never used, and should just be deleted.
>
> Similarly PMCR_EL0 is 64-bit, with bit 32 bit FZS when FEAT_SPEv1p2 is
> implemented. That doesn't matter to us right now as we don't use FZS, but it's
> something that could catch us out in future if new bitss get added.
>
> Could we please go check all the accessors added in df29ddf4f04b, and fix those
> in one go?

I quickly look at the registers and unless I'm mistaken there's just one
more register, PMCCFILTR_EL0, that needs to be taken care. It seems to
have VS bits [57:56], if FEAT_PMUv3_SME is implemented.

I'll try to submit a new patch that handles all of them tomorrow.

Cheers, Ilkka

>
>> Reported-by: Carl Worth <carl@os.amperecomputing.com>
>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> ---
>>
>> Changes since v1:
>>
>> * Took arm32 specific code from Marc Zyngier's review comment
>> * Fixed a couple of typos in the commit message
>>
>> I have tested the patch on Arm64. However, what comes to Arm32 part, I have
>> only compared the code with Arm32 specification and cross compiled it.
>>
>> v1: https://lore.kernel.org/all/20231025200815.104017-1-ilkka@os.amperecomputing.com/
>>
>> ---
>>
>> arch/arm/include/asm/arm_pmuv3.h | 33 +++++++++++++++++++++---------
>> arch/arm64/include/asm/arm_pmuv3.h | 4 ++--
>> 2 files changed, 25 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
>> index 72529f5e2bed..90841cb7ce43 100644
>> --- a/arch/arm/include/asm/arm_pmuv3.h
>> +++ b/arch/arm/include/asm/arm_pmuv3.h
>> @@ -23,6 +23,8 @@
>> #define PMUSERENR __ACCESS_CP15(c9, 0, c14, 0)
>> #define PMINTENSET __ACCESS_CP15(c9, 0, c14, 1)
>> #define PMINTENCLR __ACCESS_CP15(c9, 0, c14, 2)
>> +#define PMCEID2 __ACCESS_CP15(c9, 0, c14, 4)
>> +#define PMCEID3 __ACCESS_CP15(c9, 0, c14, 5)
>> #define PMMIR __ACCESS_CP15(c9, 0, c14, 6)
>> #define PMCCFILTR __ACCESS_CP15(c14, 0, c15, 7)
>>
>> @@ -205,16 +207,6 @@ static inline void write_pmuserenr(u32 val)
>> write_sysreg(val, PMUSERENR);
>> }
>>
>> -static inline u32 read_pmceid0(void)
>> -{
>> - return read_sysreg(PMCEID0);
>> -}
>> -
>> -static inline u32 read_pmceid1(void)
>> -{
>> - return read_sysreg(PMCEID1);
>> -}
>> -
>> static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
>> static inline void kvm_clr_pmu_events(u32 clr) {}
>> static inline bool kvm_pmu_counter_deferred(struct perf_event_attr *attr)
>> @@ -231,6 +223,7 @@ static inline void kvm_vcpu_pmu_resync_el0(void) {}
>>
>> /* PMU Version in DFR Register */
>> #define ARMV8_PMU_DFR_VER_NI 0
>> +#define ARMV8_PMU_DFR_VER_V3P1 0x4
>> #define ARMV8_PMU_DFR_VER_V3P4 0x5
>> #define ARMV8_PMU_DFR_VER_V3P5 0x6
>> #define ARMV8_PMU_DFR_VER_IMP_DEF 0xF
>> @@ -251,4 +244,24 @@ static inline bool is_pmuv3p5(int pmuver)
>> return pmuver >= ARMV8_PMU_DFR_VER_V3P5;
>> }
>>
>> +static inline u64 read_pmceid0(void)
>> +{
>> + u64 val = read_sysreg(PMCEID0);
>> +
>> + if (read_pmuver() >= ARMV8_PMU_DFR_VER_V3P1)
>> + val |= (u64)read_sysreg(PMCEID2) << 32;
>> +
>> + return val;
>> +}
>> +
>> +static inline u64 read_pmceid1(void)
>> +{
>> + u64 val = read_sysreg(PMCEID1);
>> +
>> + if (read_pmuver() >= ARMV8_PMU_DFR_VER_V3P1)
>> + val |= (u64)read_sysreg(PMCEID3) << 32;
>> +
>> + return val;
>> +}
>> +
>> #endif
>> diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
>> index 18dc2fb3d7b7..3e92b7cb57a4 100644
>> --- a/arch/arm64/include/asm/arm_pmuv3.h
>> +++ b/arch/arm64/include/asm/arm_pmuv3.h
>> @@ -126,12 +126,12 @@ static inline void write_pmuserenr(u32 val)
>> write_sysreg(val, pmuserenr_el0);
>> }
>>
>> -static inline u32 read_pmceid0(void)
>> +static inline u64 read_pmceid0(void)
>> {
>> return read_sysreg(pmceid0_el0);
>> }
>>
>> -static inline u32 read_pmceid1(void)
>> +static inline u64 read_pmceid1(void)
>> {
>> return read_sysreg(pmceid1_el0);
>> }
>> --
>> 2.41.0
>>
>

\
 
 \ /
  Last update: 2023-10-31 05:44    [W:0.079 / U:0.712 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site