[PATCH v2] arm64: Restrict undef hook for cpufeature registers

[Rob Herring]

On Tue, Jun 8, 2021 at 10:35 AM Will Deacon <will at kernel.org> wrote:
>
> On Tue, Jun 08, 2021 at 04:20:43PM +0100, Catalin Marinas wrote:
> > On Mon, May 17, 2021 at 01:02:56PM -0500, Rob Herring wrote:
> > > From: Raphael Gault <raphael.gault at arm.com>
> > >
> > > This commit modifies the mask of the mrs_hook declared in
> > > arch/arm64/kernel/cpufeatures.c which emulates only feature register
> > > access. This is necessary because this hook's mask was too large and
> > > thus masking any mrs instruction, even if not related to the emulated
> > > registers which made the pmu emulation inefficient.
> > >
> > > Signed-off-by: Raphael Gault <raphael.gault at arm.com>
> > > Signed-off-by: Rob Herring <robh at kernel.org>
> > > ---
> > > I don't *think* I'm going to need this for the perf userspace counter
> > > access, but this patch stands on its own as the PMU registers are not
> > > emulated. So please apply it.
> > >
> > > v2:
> > >  - Fix warning for set but unused sys_reg
> > > ---
> > >  arch/arm64/kernel/cpufeature.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index efed2830d141..c773f7c3c007 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -2905,8 +2905,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
> > >  }
> > >
> > >  static struct undef_hook mrs_hook = {
> > > -   .instr_mask = 0xfff00000,
> > > -   .instr_val  = 0xd5300000,
> > > +   .instr_mask = 0xffff0000,
> > > +   .instr_val  = 0xd5380000,
> >
> > Acked-by: Catalin Marinas <catalin.marinas at arm.com>
> >
> > and changing Will's email address.
>
> Should we update is_emulated() at the same time, or at least try to generate
> the instr_val value here using the same constants?

Something like the below patch? We can actually mask a bit more of the
instruction (Crn and Crm:3) and then eliminate some of the
is_emulated() checks. I'm not all that convinced it's an improvement
in readability compared to a raw instr_val and mask nor having
is_emulated depend on what was checked in the undef code.

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index c773f7c3c007..6b3f50c11ab5 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2825,11 +2825,7 @@ static void __maybe_unused
cpu_enable_cnp(struct arm64_cpu_capabilities const *c
  */
 static inline bool __attribute_const__ is_emulated(u32 id)
 {
-       return (sys_reg_Op0(id) == 0x3 &&
-               sys_reg_CRn(id) == 0x0 &&
-               sys_reg_Op1(id) == 0x0 &&
-               (sys_reg_CRm(id) == 0 ||
-                ((sys_reg_CRm(id) >= 4) && (sys_reg_CRm(id) <= 7))));
+       return ((sys_reg_CRm(id) == 0) || (sys_reg_CRm(id) >= 4));
 }

 /*
@@ -2905,8 +2901,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
 }

 static struct undef_hook mrs_hook = {
-       .instr_mask = 0xffff0000,
-       .instr_val  = 0xd5380000,
+       .instr_mask = 0xffe00000 | sys_reg(Op0_mask, Op1_mask, CRn_mask, 8, 0),
+       .instr_val  = 0xd5200000 | sys_reg(3, 0, 0, 0, 0),
        .pstate_mask = PSR_AA32_MODE_MASK,
        .pstate_val = PSR_MODE_EL0t,
        .fn = emulate_mrs,