* [PATCH v3 0/3] arm64: Make NOP handling a whitelist
@ 2020-05-01 12:37 Mark Brown
2020-05-01 12:37 ` [PATCH v3 1/3] arm64: insn: Don't assume unrecognized HINTs are NOPs Mark Brown
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mark Brown @ 2020-05-01 12:37 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Mark Brown, linux-arm-kernel
Currently we default to assuming any unrecognized instruction in the
hint space can be safely handled as a NOP. This is not robust and any
code that really wants a NOP should be using the explicitly defined NOP
so let's instead invert this and whitelist those instructions which it
is safe to handle as NOPs.
Patch 2 adds defines for the HINTs for BTI landing pads which will be
used by the in-kernel BTI series to generate landing pads in JITed BPF
code so it'd be good if this could be applied on or merged into the BTI
branch.
v3:
- Correct values for more of the HINTs.
- Add constants for additional HINTs
- Handle XPACLRI as a NOP along with all the other PAC HINTs.
- Update commit messages for greater clarity.
- Tweak the comment about what a NOP.
v2:
- Fix values for BTI HINTs.
- Rebase on v5.7-rc3+for-next/bti-user
Mark Brown (3):
arm64: insn: Don't assume unrecognized HINTs are NOPs
arm64: insn: Add constants for new HINT instruction decode
arm64: insn: Report PAC and BTI instructions as NOPs
arch/arm64/include/asm/insn.h | 28 ++++++++++++++++++++++++--
arch/arm64/kernel/insn.c | 37 ++++++++++++++++++++++++++---------
2 files changed, 54 insertions(+), 11 deletions(-)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3 1/3] arm64: insn: Don't assume unrecognized HINTs are NOPs 2020-05-01 12:37 [PATCH v3 0/3] arm64: Make NOP handling a whitelist Mark Brown @ 2020-05-01 12:37 ` Mark Brown 2020-05-01 12:37 ` [PATCH v3 2/3] arm64: insn: Add constants for new HINT instruction decode Mark Brown 2020-05-01 12:37 ` [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs Mark Brown 2 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2020-05-01 12:37 UTC (permalink / raw) To: Catalin Marinas, Will Deacon; +Cc: Mark Brown, linux-arm-kernel Currently the kernel assumes that any HINT which it does not explicitly recognise is a NOP. This is not robust as new instructions may be added which need special handling, including recent extensions like PAC, and in any case software should only be using explicit NOP instructions for deliberate NOPs. This has the effect of rendering PAC and BTI instructions unprobeable which means that probes can't be inserted on the first instruction of functions built with those features. Signed-off-by: Mark Brown <broonie@kernel.org> Acked-by: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm64/kernel/insn.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 4a9e773a177f..595c0fddeeb2 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -51,21 +51,17 @@ 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 */ +/* NOP is a specific HINT */ bool __kprobes aarch64_insn_is_nop(u32 insn) { if (!aarch64_insn_is_hint(insn)) return false; switch (insn & 0xFE0) { - case AARCH64_INSN_HINT_YIELD: - case AARCH64_INSN_HINT_WFE: - case AARCH64_INSN_HINT_WFI: - case AARCH64_INSN_HINT_SEV: - case AARCH64_INSN_HINT_SEVL: - return false; - default: + case AARCH64_INSN_HINT_NOP: return true; + default: + return false; } } -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] arm64: insn: Add constants for new HINT instruction decode 2020-05-01 12:37 [PATCH v3 0/3] arm64: Make NOP handling a whitelist Mark Brown 2020-05-01 12:37 ` [PATCH v3 1/3] arm64: insn: Don't assume unrecognized HINTs are NOPs Mark Brown @ 2020-05-01 12:37 ` Mark Brown 2020-05-01 12:37 ` [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs Mark Brown 2 siblings, 0 replies; 7+ messages in thread From: Mark Brown @ 2020-05-01 12:37 UTC (permalink / raw) To: Catalin Marinas, Will Deacon; +Cc: Mark Brown, linux-arm-kernel Add constants for decoding newer instructions defined in the HINT space. Since we are now decoding both the op2 and CRm fields rename the enum as well; this is compatible with what the existing users are doing. Signed-off-by: Mark Brown <broonie@kernel.org> Acked-by: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm64/include/asm/insn.h | 28 ++++++++++++++++++++++++++-- arch/arm64/kernel/insn.c | 2 +- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index bb313dde58a4..575675145fe2 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -39,13 +39,37 @@ enum aarch64_insn_encoding_class { * system instructions */ }; -enum aarch64_insn_hint_op { +enum aarch64_insn_hint_cr_op { AARCH64_INSN_HINT_NOP = 0x0 << 5, AARCH64_INSN_HINT_YIELD = 0x1 << 5, AARCH64_INSN_HINT_WFE = 0x2 << 5, AARCH64_INSN_HINT_WFI = 0x3 << 5, AARCH64_INSN_HINT_SEV = 0x4 << 5, AARCH64_INSN_HINT_SEVL = 0x5 << 5, + + AARCH64_INSN_HINT_XPACLRI = 0x07 << 5, + AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5, + AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5, + AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5, + AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5, + AARCH64_INSN_HINT_PACIAZ = 0x18 << 5, + AARCH64_INSN_HINT_PACIASP = 0x19 << 5, + AARCH64_INSN_HINT_PACIBZ = 0x1A << 5, + AARCH64_INSN_HINT_PACIBSP = 0x1B << 5, + AARCH64_INSN_HINT_AUTIAZ = 0x1C << 5, + AARCH64_INSN_HINT_AUTIASP = 0x1D << 5, + AARCH64_INSN_HINT_AUTIBZ = 0x1E << 5, + AARCH64_INSN_HINT_AUTIBSP = 0x1F << 5, + + AARCH64_INSN_HINT_ESB = 0x10 << 5, + AARCH64_INSN_HINT_PSB = 0x11 << 5, + AARCH64_INSN_HINT_TSB = 0x12 << 5, + AARCH64_INSN_HINT_CSDB = 0x14 << 5, + + AARCH64_INSN_HINT_BTI = 0x20 << 5, + AARCH64_INSN_HINT_BTIC = 0x22 << 5, + AARCH64_INSN_HINT_BTIJ = 0x24 << 5, + AARCH64_INSN_HINT_BTIJC = 0x26 << 5, }; enum aarch64_insn_imm_type { @@ -370,7 +394,7 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr, enum aarch64_insn_branch_type type); u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr, enum aarch64_insn_condition cond); -u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_op op); +u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op); u32 aarch64_insn_gen_nop(void); u32 aarch64_insn_gen_branch_reg(enum aarch64_insn_register reg, enum aarch64_insn_branch_type type); diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 595c0fddeeb2..d458604a96a3 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -570,7 +570,7 @@ u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr, offset >> 2); } -u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_op op) +u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_cr_op op) { return aarch64_insn_get_hint_value() | op; } -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs 2020-05-01 12:37 [PATCH v3 0/3] arm64: Make NOP handling a whitelist Mark Brown 2020-05-01 12:37 ` [PATCH v3 1/3] arm64: insn: Don't assume unrecognized HINTs are NOPs Mark Brown 2020-05-01 12:37 ` [PATCH v3 2/3] arm64: insn: Add constants for new HINT instruction decode Mark Brown @ 2020-05-01 12:37 ` Mark Brown 2020-05-01 12:57 ` Mark Rutland 2 siblings, 1 reply; 7+ messages in thread From: Mark Brown @ 2020-05-01 12:37 UTC (permalink / raw) To: Catalin Marinas, Will Deacon; +Cc: Mark Brown, linux-arm-kernel 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 | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index d458604a96a3..aa525975bf94 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -57,7 +57,30 @@ 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 and they + * are commonly placed in locations where it is useful to be + * able to probe. + */ switch (insn & 0xFE0) { + case AARCH64_INSN_HINT_XPACLRI: + 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: -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs 2020-05-01 12:37 ` [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs Mark Brown @ 2020-05-01 12:57 ` Mark Rutland 2020-05-01 16:12 ` Mark Brown 0 siblings, 1 reply; 7+ messages in thread From: Mark Rutland @ 2020-05-01 12:57 UTC (permalink / raw) To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel On Fri, May 01, 2020 at 01:37:09PM +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 | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c > index d458604a96a3..aa525975bf94 100644 > --- a/arch/arm64/kernel/insn.c > +++ b/arch/arm64/kernel/insn.c > @@ -57,7 +57,30 @@ 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 and they > + * are commonly placed in locations where it is useful to be > + * able to probe. > + */ > switch (insn & 0xFE0) { > + case AARCH64_INSN_HINT_XPACLRI: > + 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: I appreciate the desire to not change this code too much, but could we please rename this to aarch64_insn_is_steppable_hint() to avoid the misleading name? This is only ever used in the stepping code today, so the impact is just the prototype/declaration/callside, but it avoids confusion in future and renders the comment redundant. If we really need to identify true nops in future we can add a new aarch64_insn_is_nop() implementation just for that. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs 2020-05-01 12:57 ` Mark Rutland @ 2020-05-01 16:12 ` Mark Brown 2020-05-04 12:14 ` Will Deacon 0 siblings, 1 reply; 7+ messages in thread From: Mark Brown @ 2020-05-01 16:12 UTC (permalink / raw) To: Mark Rutland; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 720 bytes --] On Fri, May 01, 2020 at 01:57:34PM +0100, Mark Rutland wrote: > I appreciate the desire to not change this code too much, but could we > please rename this to aarch64_insn_is_steppable_hint() to avoid the > misleading name? That's definitely clearer. I've got this change locally but looking at the new function name I can see that having done the rename people might want to go through and also make sure that the list of HINTs we mark as steppable is up to date which is probably a good idea but would hold things up further, some of the barriers look like they might need a bit of thought. Will, what would you prefer - should I send out a version with the just rename, do that incrementally or some other thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs 2020-05-01 16:12 ` Mark Brown @ 2020-05-04 12:14 ` Will Deacon 0 siblings, 0 replies; 7+ messages in thread From: Will Deacon @ 2020-05-04 12:14 UTC (permalink / raw) To: Mark Brown; +Cc: Mark Rutland, Catalin Marinas, linux-arm-kernel On Fri, May 01, 2020 at 05:12:10PM +0100, Mark Brown wrote: > On Fri, May 01, 2020 at 01:57:34PM +0100, Mark Rutland wrote: > > > I appreciate the desire to not change this code too much, but could we > > please rename this to aarch64_insn_is_steppable_hint() to avoid the > > misleading name? > > That's definitely clearer. I've got this change locally but looking at > the new function name I can see that having done the rename people might > want to go through and also make sure that the list of HINTs we mark as > steppable is up to date which is probably a good idea but would hold > things up further, some of the barriers look like they might need a bit > of thought. > > Will, what would you prefer - should I send out a version with the > just rename, do that incrementally or some other thing? Please send a version with the rename and then people can add extra instructions to the whitelist as and when they are needed. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-04 12:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-01 12:37 [PATCH v3 0/3] arm64: Make NOP handling a whitelist Mark Brown 2020-05-01 12:37 ` [PATCH v3 1/3] arm64: insn: Don't assume unrecognized HINTs are NOPs Mark Brown 2020-05-01 12:37 ` [PATCH v3 2/3] arm64: insn: Add constants for new HINT instruction decode Mark Brown 2020-05-01 12:37 ` [PATCH v3 3/3] arm64: insn: Report PAC and BTI instructions as NOPs Mark Brown 2020-05-01 12:57 ` Mark Rutland 2020-05-01 16:12 ` Mark Brown 2020-05-04 12:14 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).