public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@kernel.org>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Jason Baron <jbaron@akamai.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>
Subject: [RFC][PATCH 0/5] Improve static call NULL handling
Date: Fri, 10 Mar 2023 12:31:12 -0800	[thread overview]
Message-ID: <cover.1678474914.git.jpoimboe@kernel.org> (raw)

Static calling a NULL pointer is a NOP, unless you're one of those poor
souls running on an arch (or backported x86 monstrosity) with
CONFIG_HAVE_STATIC_CALL=n, then it's a panic.

The "fix" for this undefined behavior is to tell the user to just use
static_call_cond() instead, if they want consistent NOP behavior.  But
forgetting to do that is likely to cause subtle bugs.  It actually
already did (during RHEL development).

There are two ways to make it consistent:

  a) Make static_call(NULL) a NOP for all configs; or

  b) Make static_call(NULL) a panic for all configs.

Do (a) because it's consistent with the existing HAVE_STATIC_CALL
behavior.  Also it seems simpler to implement and use, and based on
looking at the existing use cases, it's common to want the "do nothing
and return 0" behavior by default.

Then take it a step further and get rid of the distinction between
STATIC_CALL_NULL and STATIC_CALL_RET0.

The end result is less confusing semantics and simpler code all around.


EPILOGUE
--------

If any users wanted panic-on-NULL by default instead of NOP-on-NULL,
that could be added on top of this.  They could just initialize the
static call with a __static_call_bug() helper.

  void __static_call_bug(void)
  {
  	BUG();
  }
  ..
  DEFINE_STATIC_CALL(foo, (func_type)__static_call_bug);

We could take that even further:

  DEFINE_STATIC_CALL_NOP(foo, func_type);
  DEFINE_STATIC_CALL_BUG(bar, func_type);
  ...
  #define STATIC_CALL_NOP (func_type)__static_call_nop
  #define STATIC_CALL_BUG (func_type)__static_call_bug
  ...
  static_call_update(foo, STATIC_CALL_NOP); // do nothing and return 0
  static_call_update(foo, STATIC_CALL_BUG); // panic
  static_call_update(foo, NULL);	    // ???

The default behavior for NULL could be a key-specific policy, stored as
a flag in the static_call_key struct.

The key-specific policy would be easier to deal with than the
call-site-specific policy we have today with static_call_cond().



Josh Poimboeuf (5):
  static_call: Make NULL static calls consistent
  static_call: Make NULL static calls return 0
  static_call: Remove static_call_cond() and its usages
  static_call: Remove DEFINE_STATIC_CALL_RET0() and its uses
  x86/kvm: Simplify static call handling

 arch/powerpc/include/asm/static_call.h    |   1 -
 arch/powerpc/kernel/irq.c                 |   2 +-
 arch/x86/events/amd/core.c                |   2 +-
 arch/x86/events/core.c                    |  26 ++---
 arch/x86/include/asm/kvm-x86-ops.h        |  86 +++++++-------
 arch/x86/include/asm/kvm-x86-pmu-ops.h    |  17 +--
 arch/x86/include/asm/kvm_host.h           |   6 +-
 arch/x86/include/asm/static_call.h        |   8 --
 arch/x86/kvm/irq.c                        |   2 +-
 arch/x86/kvm/lapic.c                      |  22 ++--
 arch/x86/kvm/pmu.c                        |  11 +-
 arch/x86/kvm/x86.c                        |  36 +++---
 include/linux/static_call.h               | 131 +++++-----------------
 kernel/events/core.c                      |   8 +-
 kernel/sched/core.c                       |  10 +-
 security/keys/trusted-keys/trusted_core.c |   2 +-
 16 files changed, 126 insertions(+), 244 deletions(-)

-- 
2.39.2


             reply	other threads:[~2023-03-10 20:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 20:31 Josh Poimboeuf [this message]
2023-03-10 20:31 ` [RFC][PATCH 1/5] static_call: Make NULL static calls consistent Josh Poimboeuf
2023-03-10 20:59   ` Peter Zijlstra
2023-03-11  1:20     ` Josh Poimboeuf
2023-03-12 15:17       ` Peter Zijlstra
2023-03-13 15:08         ` Sean Christopherson
2023-03-13 17:48         ` Sami Tolvanen
2023-03-14  1:58           ` Josh Poimboeuf
2023-03-14 10:06             ` Peter Zijlstra
2023-03-10 20:31 ` [RFC][PATCH 2/5] static_call: Make NULL static calls return 0 Josh Poimboeuf
2023-03-10 20:31 ` [RFC][PATCH 3/5] static_call: Remove static_call_cond() and its usages Josh Poimboeuf
2023-03-10 20:31 ` [RFC][PATCH 4/5] static_call: Remove DEFINE_STATIC_CALL_RET0() and its uses Josh Poimboeuf
2023-03-10 20:31 ` [RFC][PATCH 5/5] x86/kvm: Simplify static call handling Josh Poimboeuf
2023-03-10 21:07   ` Sean Christopherson
2023-03-10 21:13     ` Steven Rostedt
2023-03-10 21:29       ` Sean Christopherson
2023-03-10 22:23         ` Josh Poimboeuf
2023-03-10 21:09 ` [RFC][PATCH 0/5] Improve static call NULL handling Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1678474914.git.jpoimboe@kernel.org \
    --to=jpoimboe@kernel.org \
    --cc=ardb@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=jbaron@akamai.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=seanjc@google.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox