Messages in this thread | | | Date | Tue, 9 Apr 2024 17:09:16 +0100 | From | Will Deacon <> | Subject | Re: [PATCH] arm64: arm_pmuv3: Correctly extract and check the PMUVer |
| |
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
| |