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

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

[Rob Herring]

From: Raphael Gault <raphael.gault@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@arm.com>
Signed-off-by: Rob Herring <robh@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,
 	.pstate_mask = PSR_AA32_MODE_MASK,
 	.pstate_val = PSR_MODE_EL0t,
 	.fn = emulate_mrs,
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Catalin Marinas]

On Mon, May 17, 2021 at 01:02:56PM -0500, Rob Herring wrote:
> From: Raphael Gault <raphael.gault@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@arm.com>
> Signed-off-by: Rob Herring <robh@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@arm.com>

and changing Will's email address.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Will Deacon]

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@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@arm.com>
> > Signed-off-by: Rob Herring <robh@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@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?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

On Tue, Jun 8, 2021 at 10:35 AM Will Deacon <will@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@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@arm.com>
> > > Signed-off-by: Rob Herring <robh@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@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,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Will Deacon]

On Mon, 17 May 2021 13:02:56 -0500, Rob Herring wrote:
> 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.

Applied to arm64 (for-next/cpufeature), thanks!

[1/1] arm64: Restrict undef hook for cpufeature registers
      https://git.kernel.org/arm64/c/cf292e93f423

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel