| Message ID | 20200428172433.48830-4-broonie@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Make NOP handling a whitelist | expand |
On Tue, Apr 28, 2020 at 06:24:33PM +0100, Mark Brown wrote: > In order to allow probing of PAC and BTI instructions without more > specialized support recognize them as NOPs. > > Signed-off-by: Mark Brown <broonie@kernel.org> > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > --- > arch/arm64/kernel/insn.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c > index fd77cdd87c47..82afc582d29a 100644 > --- a/arch/arm64/kernel/insn.c > +++ b/arch/arm64/kernel/insn.c > @@ -57,7 +57,27 @@ bool __kprobes aarch64_insn_is_nop(u32 insn) > if (!aarch64_insn_is_hint(insn)) > return false; > > + /* > + * The PAC and BTI instructons are not strictly NOPs but until > + * better support is added we can treat them as such. > + */ > switch (insn & 0xFE0) { Are you deliberately omitting XPACLRI? If so, maybe add a comment to say why, since it looks a bit weird without it. Will
On Thu, Apr 30, 2020 at 05:09:29PM +0100, Will Deacon wrote: > On Tue, Apr 28, 2020 at 06:24:33PM +0100, Mark Brown wrote: > > + /* > > + * The PAC and BTI instructons are not strictly NOPs but until > > + * better support is added we can treat them as such. > > + */ > > switch (insn & 0xFE0) { > Are you deliberately omitting XPACLRI? If so, maybe add a comment to say > why, since it looks a bit weird without it. Not deliberately, no - I'm not sure I'd heard of it before and it's one of those cases where the spec doesn't call out the instruction as being in the hint space.
On Thu, Apr 30, 2020 at 06:16:45PM +0100, Mark Brown wrote: > On Thu, Apr 30, 2020 at 05:09:29PM +0100, Will Deacon wrote: > > On Tue, Apr 28, 2020 at 06:24:33PM +0100, Mark Brown wrote: > > > > + /* > > > + * The PAC and BTI instructons are not strictly NOPs but until > > > + * better support is added we can treat them as such. > > > + */ > > > switch (insn & 0xFE0) { > > > Are you deliberately omitting XPACLRI? If so, maybe add a comment to say > > why, since it looks a bit weird without it. > > Not deliberately, no - I'm not sure I'd heard of it before and it's one > of those cases where the spec doesn't call out the instruction as being > in the hint space. Hmm, so I only spotted it because of "Decode" pseudocode for HINT on p.929 of rev F.a of the Arm ARM (the one with the AUTHASP typo). Will
On Thu, Apr 30, 2020 at 06:20:40PM +0100, Will Deacon wrote: > On Thu, Apr 30, 2020 at 06:16:45PM +0100, Mark Brown wrote: > > On Thu, Apr 30, 2020 at 05:09:29PM +0100, Will Deacon wrote: > > > Are you deliberately omitting XPACLRI? If so, maybe add a comment to say > > > why, since it looks a bit weird without it. > > Not deliberately, no - I'm not sure I'd heard of it before and it's one > > of those cases where the spec doesn't call out the instruction as being > > in the hint space. > Hmm, so I only spotted it because of "Decode" pseudocode for HINT on p.929 > of rev F.a of the Arm ARM (the one with the AUTHASP typo). Ah, I see - that's indeed a useful enumeration of the currently defined HINTs. IIRC I wrote this code before I became aware that HINT was defined as an instruction rather than just being an encoding space! Thanks for the pointer.
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index fd77cdd87c47..82afc582d29a 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -57,7 +57,27 @@ bool __kprobes aarch64_insn_is_nop(u32 insn) if (!aarch64_insn_is_hint(insn)) return false; + /* + * The PAC and BTI instructons are not strictly NOPs but until + * better support is added we can treat them as such. + */ switch (insn & 0xFE0) { + case AARCH64_INSN_HINT_PACIA_1716: + case AARCH64_INSN_HINT_PACIB_1716: + case AARCH64_INSN_HINT_AUTIA_1716: + case AARCH64_INSN_HINT_AUTIB_1716: + case AARCH64_INSN_HINT_PACIAZ: + case AARCH64_INSN_HINT_PACIASP: + case AARCH64_INSN_HINT_PACIBZ: + case AARCH64_INSN_HINT_PACIBSP: + case AARCH64_INSN_HINT_AUTIAZ: + case AARCH64_INSN_HINT_AUTIASP: + case AARCH64_INSN_HINT_AUTIBZ: + case AARCH64_INSN_HINT_AUTIBSP: + case AARCH64_INSN_HINT_BTI: + case AARCH64_INSN_HINT_BTIC: + case AARCH64_INSN_HINT_BTIJ: + case AARCH64_INSN_HINT_BTIJC: case AARCH64_INSN_HINT_NOP: return true; default: