[PATCH][boot-wrapper] aarch64: Enable ECV to allow access to CNTPOFF_EL2
Marc Zyngier
maz at kernel.org
Wed Aug 11 07:45:21 PDT 2021
On Wed, 11 Aug 2021 14:55:58 +0100,
Mark Rutland <mark.rutland at arm.com> wrote:
>
> On Wed, Aug 11, 2021 at 10:22:26AM +0100, Marc Zyngier wrote:
> > If the implemnentation supports ID_AA64MMFR0_EL1.ECV==2,
> > set SCR_EL3.ECVEn to allow EL2 access to CNTPOFF_EL2.
>
> I was about to ask if that was a typo (why is this in an MMFR?), but
> that is what the ARM ARM says!
I was just as surprised. PFR would have made a lot more sense, but I
guess someone wanted to use these last four bits as quickly as
possible... ;-)
>
> > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > ---
> > arch/aarch64/boot.S | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> > index 7f208b5..f0aa3cb 100644
> > --- a/arch/aarch64/boot.S
> > +++ b/arch/aarch64/boot.S
> > @@ -54,10 +54,17 @@ ASM_FUNC(_start)
> > 1:
> > /* Enable FGT if present */
> > mrs x1, id_aa64mmfr0_el1
> > - ubfx x1, x1, #56, #4
> > - cbz x1, 1f
> > + ubfx x2, x1, #56, #4
> > + cbz x2, 1f
> >
> > orr x0, x0, #(1 << 27) // FGT enable
> > +1:
> > + /* Enable ECV2 if present (allows CNTPOFF_EL2) */
> > + ubfx x2, x1, #60, #4
> > + cmp x2, #2
> > + bne 1f
>
> We need to check ID_AA64MMFR0_EL1.ECV >= 2 (to handle any futrue
> variants) so the conditional branch needs to be something like `b.lt`.
Good point. I'm surprised ECV3 hasn't hit yet! :D
> > +
> > + orr x0, x0, #(1 << 28) // ECV enable
>
> Aside from the above, this looks good to me, and I plan to apply this
> with the below tweaks (updated patch below):
>
> * Use b.lt, as above
>
> * To keep each check self-contained, the ECV check will read
> id_aa64mmfr0_el1 itself. That removes the need to keep x1 around, and
> so we can leave the FGT check as-is.
>
> As part of some cleanup, I'm planning to move this to C, where feature
> checks will read ID fields via a helepr that implicitly reads the
> register, like:
>
> if (read_reg_field(ID_AA64MMFR0_EL1, ECV) >= 2)
> scr |= SCR_EL3_ECVEN;
If you are actively rewriting this, maybe don't bother with the patch
and just add this as part of your rewrite.
>
> Thanks,
> Mark.
> ---->8----
> From bc6a9380eb3c5afc96735a54d455f2487df48700 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz at kernel.org>
> Date: Wed, 11 Aug 2021 10:22:26 +0100
> Subject: [PATCH] aarch64: Enable ECV to allow access to CNTPOFF_EL2
>
> If the implemnentation supports ID_AA64MMFR0_EL1.ECV==2,
> set SCR_EL3.ECVEn to allow EL2 access to CNTPOFF_EL2.
>
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> [Mark: read id_aa64mmfr0_el1 separately, s/bne/b.lt/]
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> ---
> arch/aarch64/boot.S | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index 7f208b5..2215f7e 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -59,6 +59,14 @@ ASM_FUNC(_start)
>
> orr x0, x0, #(1 << 27) // FGT enable
> 1:
> + /* Enable ECV2 if present (allows CNTPOFF_EL2) */
> + mrs x1, id_aa64mmfr0_el1
> + ubfx x1, x1, #60, #4
> + cmp x1, #2
> + b.lt 1f
> +
> + orr x0, x0, #(1 << 28) // ECV enable
> +1:
> /* Enable MTE if present */
> mrs x10, id_aa64pfr1_el1
> ubfx x10, x10, #8, #4
Otherwise looks fine to me.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list