| Message ID | 20200504131326.18290-3-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 07dcd9677c5d0f4735041885d3c3c348cc11b75c |
| Headers | show |
| Series | arm64: Make NOP handling a whitelist | expand |
On Mon, May 04, 2020 at 02:13:24PM +0100, Mark Brown wrote: > The current aarch64_insn_is_nop() has exactly one caller which uses it > solely to identify if the instruction is a HINT that can safely be stepped, > requiring us to list things that aren't NOPs and make things more confusing > than they need to be. Rename the function to reflect the actual usage and > make things more clear. > > Suggested-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Mark Brown <broonie@kernel.org> Thanks for this! Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > arch/arm64/include/asm/insn.h | 2 +- > arch/arm64/kernel/insn.c | 3 +-- > arch/arm64/kernel/probes/decode-insn.c | 2 +- > 3 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h > index 575675145fe2..0bc46149e491 100644 > --- a/arch/arm64/include/asm/insn.h > +++ b/arch/arm64/include/asm/insn.h > @@ -368,7 +368,7 @@ __AARCH64_INSN_FUNCS(msr_reg, 0xFFF00000, 0xD5100000) > > #undef __AARCH64_INSN_FUNCS > > -bool aarch64_insn_is_nop(u32 insn); > +bool aarch64_insn_is_steppable_hint(u32 insn); > bool aarch64_insn_is_branch_imm(u32 insn); > > static inline bool aarch64_insn_is_adr_adrp(u32 insn) > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c > index d63d9cd8b4a2..0829bb5b45ec 100644 > --- a/arch/arm64/kernel/insn.c > +++ b/arch/arm64/kernel/insn.c > @@ -51,8 +51,7 @@ enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn) > return aarch64_insn_encoding_class[(insn >> 25) & 0xf]; > } > > -/* NOP is an alias of HINT */ > -bool __kprobes aarch64_insn_is_nop(u32 insn) > +bool __kprobes aarch64_insn_is_steppable_hint(u32 insn) > { > if (!aarch64_insn_is_hint(insn)) > return false; > diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c > index b78fac9e546c..263d5fba4c8a 100644 > --- a/arch/arm64/kernel/probes/decode-insn.c > +++ b/arch/arm64/kernel/probes/decode-insn.c > @@ -46,7 +46,7 @@ static bool __kprobes aarch64_insn_is_steppable(u32 insn) > * except for the NOP case. > */ > if (aarch64_insn_is_hint(insn)) > - return aarch64_insn_is_nop(insn); > + return aarch64_insn_is_steppable_hint(insn); > > return true; > } > -- > 2.20.1 >
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 575675145fe2..0bc46149e491 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -368,7 +368,7 @@ __AARCH64_INSN_FUNCS(msr_reg, 0xFFF00000, 0xD5100000) #undef __AARCH64_INSN_FUNCS -bool aarch64_insn_is_nop(u32 insn); +bool aarch64_insn_is_steppable_hint(u32 insn); bool aarch64_insn_is_branch_imm(u32 insn); static inline bool aarch64_insn_is_adr_adrp(u32 insn) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index d63d9cd8b4a2..0829bb5b45ec 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -51,8 +51,7 @@ enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn) return aarch64_insn_encoding_class[(insn >> 25) & 0xf]; } -/* NOP is an alias of HINT */ -bool __kprobes aarch64_insn_is_nop(u32 insn) +bool __kprobes aarch64_insn_is_steppable_hint(u32 insn) { if (!aarch64_insn_is_hint(insn)) return false; diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c index b78fac9e546c..263d5fba4c8a 100644 --- a/arch/arm64/kernel/probes/decode-insn.c +++ b/arch/arm64/kernel/probes/decode-insn.c @@ -46,7 +46,7 @@ static bool __kprobes aarch64_insn_is_steppable(u32 insn) * except for the NOP case. */ if (aarch64_insn_is_hint(insn)) - return aarch64_insn_is_nop(insn); + return aarch64_insn_is_steppable_hint(insn); return true; }
The current aarch64_insn_is_nop() has exactly one caller which uses it solely to identify if the instruction is a HINT that can safely be stepped, requiring us to list things that aren't NOPs and make things more confusing than they need to be. Rename the function to reflect the actual usage and make things more clear. Suggested-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/include/asm/insn.h | 2 +- arch/arm64/kernel/insn.c | 3 +-- arch/arm64/kernel/probes/decode-insn.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-)