* [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer @ 2024-04-08 8:11 Yicong Yang 2024-04-09 16:09 ` Will Deacon 0 siblings, 1 reply; 3+ messages in thread From: Yicong Yang @ 2024-04-08 8:11 UTC (permalink / raw) To: will, mark.rutland, catalin.marinas, broonie, linux-arm-kernel, linux-kernel Cc: jonathan.cameron, prime.zeng, linuxarm, yangyicong From: Yicong Yang <yangyicong@hisilicon.com> Currently we're using "sbfx" to extract the PMUVer from ID_AA64DFR0_EL1 and skip the init/reset if no PMU present when the extracted PMUVer is negative or is zero. However for PMUv3p8 the PMUVer will be 0b1000 and PMUVer extracted by "sbfx" will always be negative and we'll skip the init/reset in __init_el2_debug/reset_pmuserenr_el0 unexpectedly. So this patch use "ubfx" instead of "sbfx" to extract the PMUVer. If the PMUVer is implementation defined (0b1111) then reset it to zero and skip the reset/init. Previously we'll also skip the init/reset if the PMUVer is higher than the version we known (currently PMUv3p9), with this patch we'll only skip if the PMU is not implemented or implementation defined. This keeps consistence with how we probe the PMU in the driver with pmuv3_implemented(). Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- arch/arm64/include/asm/assembler.h | 5 ++++- arch/arm64/include/asm/el2_setup.h | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index ab8b396428da..3b7373d6c565 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -480,7 +480,10 @@ alternative_endif */ .macro reset_pmuserenr_el0, tmpreg mrs \tmpreg, id_aa64dfr0_el1 - sbfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 + ubfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 + cmp \tmpreg, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF + csel \tmpreg, xzr, \tmpreg, eq // If PMU's IMP_DEF, regard it + // as not implemented and skip cmp \tmpreg, #1 // Skip if no PMU present b.lt 9000f msr pmuserenr_el0, xzr // Disable PMU access from EL0 diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h index b7afaa026842..2438e12b60c5 100644 --- a/arch/arm64/include/asm/el2_setup.h +++ b/arch/arm64/include/asm/el2_setup.h @@ -59,7 +59,10 @@ .macro __init_el2_debug mrs x1, id_aa64dfr0_el1 - sbfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 + ubfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 + cmp x0, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF + csel x0, xzr, x0, eq // If PMU's IMP_DEF, regard it + // as not implemented and skip cmp x0, #1 b.lt .Lskip_pmu_\@ // Skip if no PMU present mrs x0, pmcr_el0 // Disable debug access traps -- 2.24.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer 2024-04-08 8:11 [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer Yicong Yang @ 2024-04-09 16:09 ` Will Deacon 2024-04-10 10:08 ` Yicong Yang 0 siblings, 1 reply; 3+ messages in thread From: Will Deacon @ 2024-04-09 16:09 UTC (permalink / raw) To: Yicong Yang Cc: mark.rutland, catalin.marinas, broonie, linux-arm-kernel, linux-kernel, jonathan.cameron, prime.zeng, linuxarm, yangyicong On Mon, Apr 08, 2024 at 04:11:58PM +0800, Yicong Yang wrote: > From: Yicong Yang <yangyicong@hisilicon.com> > > Currently we're using "sbfx" to extract the PMUVer from ID_AA64DFR0_EL1 > and skip the init/reset if no PMU present when the extracted PMUVer is > negative or is zero. However for PMUv3p8 the PMUVer will be 0b1000 and > PMUVer extracted by "sbfx" will always be negative and we'll skip the > init/reset in __init_el2_debug/reset_pmuserenr_el0 unexpectedly. > > So this patch use "ubfx" instead of "sbfx" to extract the PMUVer. If > the PMUVer is implementation defined (0b1111) then reset it to zero > and skip the reset/init. Previously we'll also skip the init/reset > if the PMUVer is higher than the version we known (currently PMUv3p9), > with this patch we'll only skip if the PMU is not implemented or > implementation defined. This keeps consistence with how we probe > the PMU in the driver with pmuv3_implemented(). > > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > --- > arch/arm64/include/asm/assembler.h | 5 ++++- > arch/arm64/include/asm/el2_setup.h | 5 ++++- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > index ab8b396428da..3b7373d6c565 100644 > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@ -480,7 +480,10 @@ alternative_endif > */ > .macro reset_pmuserenr_el0, tmpreg > mrs \tmpreg, id_aa64dfr0_el1 > - sbfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 > + ubfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 > + cmp \tmpreg, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF > + csel \tmpreg, xzr, \tmpreg, eq // If PMU's IMP_DEF, regard it > + // as not implemented and skip > cmp \tmpreg, #1 // Skip if no PMU present > b.lt 9000f > msr pmuserenr_el0, xzr // Disable PMU access from EL0 I think the cmp/csel/cmp/b.lt sequence might be a little tidier if you reworked it to use ccmp. For example, something like (totally untested): cmp \tmpreg, ID_AA64DFR0_EL1_PMUVer_NI ccmp \tmpreg, ID_AA64DFR0_EL1_PMUVer_IMP_DEF, #4, ne would then, I think, mean we could just b.eq 9000f. But please check this because encoding nzcv as an immediate always catches me out. > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > index b7afaa026842..2438e12b60c5 100644 > --- a/arch/arm64/include/asm/el2_setup.h > +++ b/arch/arm64/include/asm/el2_setup.h > @@ -59,7 +59,10 @@ > > .macro __init_el2_debug > mrs x1, id_aa64dfr0_el1 > - sbfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 > + ubfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 > + cmp x0, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF > + csel x0, xzr, x0, eq // If PMU's IMP_DEF, regard it > + // as not implemented and skip > cmp x0, #1 > b.lt .Lskip_pmu_\@ // Skip if no PMU present > mrs x0, pmcr_el0 // Disable debug access traps Similar sort of thing here. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer 2024-04-09 16:09 ` Will Deacon @ 2024-04-10 10:08 ` Yicong Yang 0 siblings, 0 replies; 3+ messages in thread From: Yicong Yang @ 2024-04-10 10:08 UTC (permalink / raw) To: Will Deacon Cc: yangyicong, mark.rutland, catalin.marinas, broonie, linux-arm-kernel, linux-kernel, jonathan.cameron, prime.zeng, linuxarm On 2024/4/10 0:09, Will Deacon wrote: > On Mon, Apr 08, 2024 at 04:11:58PM +0800, Yicong Yang wrote: >> From: Yicong Yang <yangyicong@hisilicon.com> >> >> Currently we're using "sbfx" to extract the PMUVer from ID_AA64DFR0_EL1 >> and skip the init/reset if no PMU present when the extracted PMUVer is >> negative or is zero. However for PMUv3p8 the PMUVer will be 0b1000 and >> PMUVer extracted by "sbfx" will always be negative and we'll skip the >> init/reset in __init_el2_debug/reset_pmuserenr_el0 unexpectedly. >> >> So this patch use "ubfx" instead of "sbfx" to extract the PMUVer. If >> the PMUVer is implementation defined (0b1111) then reset it to zero >> and skip the reset/init. Previously we'll also skip the init/reset >> if the PMUVer is higher than the version we known (currently PMUv3p9), >> with this patch we'll only skip if the PMU is not implemented or >> implementation defined. This keeps consistence with how we probe >> the PMU in the driver with pmuv3_implemented(). >> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> --- >> arch/arm64/include/asm/assembler.h | 5 ++++- >> arch/arm64/include/asm/el2_setup.h | 5 ++++- >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h >> index ab8b396428da..3b7373d6c565 100644 >> --- a/arch/arm64/include/asm/assembler.h >> +++ b/arch/arm64/include/asm/assembler.h >> @@ -480,7 +480,10 @@ alternative_endif >> */ >> .macro reset_pmuserenr_el0, tmpreg >> mrs \tmpreg, id_aa64dfr0_el1 >> - sbfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 >> + ubfx \tmpreg, \tmpreg, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 >> + cmp \tmpreg, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF >> + csel \tmpreg, xzr, \tmpreg, eq // If PMU's IMP_DEF, regard it >> + // as not implemented and skip >> cmp \tmpreg, #1 // Skip if no PMU present >> b.lt 9000f >> msr pmuserenr_el0, xzr // Disable PMU access from EL0 > > I think the cmp/csel/cmp/b.lt sequence might be a little tidier if you > reworked it to use ccmp. For example, something like (totally untested): > > cmp \tmpreg, ID_AA64DFR0_EL1_PMUVer_NI > ccmp \tmpreg, ID_AA64DFR0_EL1_PMUVer_IMP_DEF, #4, ne > > would then, I think, mean we could just b.eq 9000f. But please check > this because encoding nzcv as an immediate always catches me out. > ok. will have a test on my boards. Wrote a small demo for checking all the available versions with suggested code and seems work fine. >> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h >> index b7afaa026842..2438e12b60c5 100644 >> --- a/arch/arm64/include/asm/el2_setup.h >> +++ b/arch/arm64/include/asm/el2_setup.h >> @@ -59,7 +59,10 @@ >> >> .macro __init_el2_debug >> mrs x1, id_aa64dfr0_el1 >> - sbfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 >> + ubfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 >> + cmp x0, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF >> + csel x0, xzr, x0, eq // If PMU's IMP_DEF, regard it >> + // as not implemented and skip >> cmp x0, #1 >> b.lt .Lskip_pmu_\@ // Skip if no PMU present >> mrs x0, pmcr_el0 // Disable debug access traps > > Similar sort of thing here. will also need to change the cond after the .Lskip_pmu_\@ like below: .macro __init_el2_debug mrs x1, id_aa64dfr0_el1 - sbfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 - cmp x0, #1 - b.lt .Lskip_pmu_\@ // Skip if no PMU present + ubfx x0, x1, #ID_AA64DFR0_EL1_PMUVer_SHIFT, #4 + cmp x0, #ID_AA64DFR0_EL1_PMUVer_NI + ccmp x0, #ID_AA64DFR0_EL1_PMUVer_IMP_DEF, #4, ne + b.eq .Lskip_pmu_\@ // Skip if no PMU present or IMP_DEF mrs x0, pmcr_el0 // Disable debug access traps ubfx x0, x0, #11, #5 // to EL2 and allow access to .Lskip_pmu_\@: - csel x2, xzr, x0, lt // all PMU counters from EL1 + csel x2, xzr, x0, eq // all PMU counters from EL1 Will respin a v2 after tests. Thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-10 10:09 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-08 8:11 [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer Yicong Yang 2024-04-09 16:09 ` Will Deacon 2024-04-10 10:08 ` Yicong Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox