Message ID | 20210517180256.2881891-1-robh@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] arm64: Restrict undef hook for cpufeature registers | expand |
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.
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
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,
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,
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,