Hi James, On Friday 28 Feb 2020 at 16:44:53 (+0000), James Morse wrote: > Hi Ionela, > > On 26/02/2020 13:29, Ionela Voinescu wrote: > > The activity monitors extension is an optional extension introduced > > by the ARMv8.4 CPU architecture. In order to access the activity > > monitors counters safely, if desired, the kernel should detect the > > presence of the extension through the feature register, and mediate > > the access. > > > > Therefore, disable direct accesses to activity monitors counters > > from EL0 (userspace) and trap them to EL1 (kernel). > > > > To be noted that the ARM64_AMU_EXTN kernel config and the disable_amu > > kernel parameter do not have an effect on this code. Given that the > > amuserenr_el0 resets to an UNKNOWN value, setting the trap of EL0 > > accesses to EL1 is always attempted for safety and security > > considerations. > > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > > index aafed6902411..7103027b4e64 100644 > > --- a/arch/arm64/mm/proc.S > > +++ b/arch/arm64/mm/proc.S > > > @@ -131,6 +131,7 @@ alternative_endif > > ubfx x11, x11, #1, #1 > > msr oslar_el1, x11 > > reset_pmuserenr_el0 x0 // Disable PMU access from EL0 > > + reset_amuserenr_el0 x0 // Disable AMU access from EL0 > > > > alternative_if ARM64_HAS_RAS_EXTN > > msr_s SYS_DISR_EL1, xzr > > (This above hunk is in: cpu_do_resume, and this next one is __cpu_setup,) > > > @@ -423,6 +424,8 @@ SYM_FUNC_START(__cpu_setup) > > isb // Unmask debug exceptions now, > > enable_dbg // since this is per-cpu > > reset_pmuserenr_el0 x0 // Disable PMU access from EL0 > > + reset_amuserenr_el0 x0 // Disable AMU access from EL0 > > I think you only need this in __cpu_setup. The entry-point from cpu-idle calls: > | cpu_resume > | ->__cpu_setup > | -->reset_amuserenr_el0 > | ->_cpu_resume > | -->cpu_do_resume > | --->reset_amuserenr_el0 > > (Which means the PMU reset call is redundant too). > Thanks, that seems to be so. I'll submit a separate fix for both amu and pmu, if you don't mind, after this set, so it will be a specific cleanup patch. > Its harmless, and needs cleaning up already, so regardless: > Reviewed-by: James Morse <james.morse@xxxxxxx> > Thank you very much for the review, Ionela. > > > Thanks, > > James
