On Tue, Jan 12, 2021 at 02:39:49PM +0000, Mark Rutland wrote:

On Tue, Jan 12, 2021 at 02:25:27PM +0000, Will Deacon wrote:

quoted

On Thu, Jan 07, 2021 at 02:53:10PM +0000, Mark Rutland wrote:

quoted

All EL0 returns go via ret_to_user(), which masks IRQs and notifies
lockdep and tracing before calling into do_notify_resume(). Therefore,
there's no need for do_notify_resume() to call trace_hardirqs_off(), and
the comment is stale. The call is simply redundant.

In ret_to_user() we call exit_to_user_mode(), which notifies lockdep and
tracing the IRQs will be enabled in userspace, so there's no need for
el0_svc_common() to call trace_hardirqs_on() before returning. Further,
at the start of ret_to_user() we call trace_hardirqs_off(), so not only
is this redundant, but it is immediately undone.

In addition to being redundant, the trace_hardirqs_on() in
trace_hardirqs_on() isn't consistent with the HW state, and is liable to
cause issues for any C code or instrumentation invoked before this is
undone in ret_to_user().

I can't parse this final paragraph, but it seems to be the part which
justifies this as a fix. Please can you reword?

Oh whoops, I messed that up. Does the following make more sense to you?

| In addition to being redundant, the trace_hardirqs_on() in
| el0_svc_common() leaves lockdep inconsistent with the hardware state,
| and is liable to cause issues for any C code or instrumentation
| between this and the call to trace_hardirqs_off() which undoes it in
| ret_to_user().

... if so, I can resend with that folded in, unless you'd prefer to fix
it up locally.

With that change:

Acked-by: Will Deacon <will@kernel.org>

Catalin's looking after fixes, so I'll leave it up to him.

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel