| Message ID | 20200108185634.1163-2-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: entry deasmification and cleanup | expand |
On 01/09/2020 12:26 AM, Mark Rutland wrote: > Almost all functions in entry-common.c are marked notrace, with > el1_undef and el1_inv being the only exceptions. We appear to have done > this on the assumption that there were no exception registers that we > needed to snapshot, and thus it was safe to run trace code that might > result in further exceptions and clobber those registers. > > However, until we inherit the DAIF flags, our irq flag tracing is stale, > and this discrepancy could set off warnings in some configurations. Could you give some example scenarios when this might happen ?
On Thu, Jan 09, 2020 at 10:51:10AM +0530, Anshuman Khandual wrote: > > > On 01/09/2020 12:26 AM, Mark Rutland wrote: > > Almost all functions in entry-common.c are marked notrace, with > > el1_undef and el1_inv being the only exceptions. We appear to have done > > this on the assumption that there were no exception registers that we > > needed to snapshot, and thus it was safe to run trace code that might > > result in further exceptions and clobber those registers. > > > > However, until we inherit the DAIF flags, our irq flag tracing is stale, > > and this discrepancy could set off warnings in some configurations. > > Could you give some example scenarios when this might happen ? With CONFIG_DEBUG_LOCKDEP, any locked-instrumented function which calls check_flags() would trigger this. So if your trace function does any sort of lock manipulation it's liable to set this off. I'll amend the above: | However, until we inherit the DAIF flags, our irq flag tracing is | stale, and this discrepancy could set off warnings in some | configurations (e.g. with CONFIG_DEBUG_LOCKDEP). I also hope that flag checking is performed more generally in future, since at the moment it's only strictly performed for locking operations. Thanks, Mark.
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index 5dce5e56995a..67198142a0fc 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -36,14 +36,14 @@ static void notrace el1_pc(struct pt_regs *regs, unsigned long esr) } NOKPROBE_SYMBOL(el1_pc); -static void el1_undef(struct pt_regs *regs) +static void notrace el1_undef(struct pt_regs *regs) { local_daif_inherit(regs); do_undefinstr(regs); } NOKPROBE_SYMBOL(el1_undef); -static void el1_inv(struct pt_regs *regs, unsigned long esr) +static void notrace el1_inv(struct pt_regs *regs, unsigned long esr) { local_daif_inherit(regs); bad_mode(regs, 0, esr);
Almost all functions in entry-common.c are marked notrace, with el1_undef and el1_inv being the only exceptions. We appear to have done this on the assumption that there were no exception registers that we needed to snapshot, and thus it was safe to run trace code that might result in further exceptions and clobber those registers. However, until we inherit the DAIF flags, our irq flag tracing is stale, and this discrepancy could set off warnings in some configurations. Given we don't expect to trigger el1_undef or el1_inv unless something is already wrong, any irqflag warnigns are liable to mask the information we'd actually care about. Let's keep things simple and mark el1_undef and el1_inv as notrace. Developers can trace do_undefinstr and bad_mode if they really want to monitor these cases. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: James Morse <james.morse@arm.com> Cc: Will Deacon <will@kernel.org> --- arch/arm64/kernel/entry-common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)