Re: [RFC][PATCH 5/5] x86/kvm: Simplify static call handling

[Josh Poimboeuf]

On Fri, Mar 10, 2023 at 09:29:36PM +0000, Sean Christopherson wrote:
> > > I would much prefer to keep KVM mostly as-is, specifically so that we don't lose
> > > this WARN_ON() that guards against a vendor module neglecting to implement a
> > > mandatory callback.  This effectively gives KVM "full" protection against consuming
> > > an unexpectedly-NULL function pointer.

Ok, sure.

> > As in my reply to patch 0/5, I suggested that static_call_update(NULL)
> > would trigger a WARN_ON() always. Then this could be cleaned up and still
> > get that warning.
> 
> I don't think that provides the functionality KVM wants/needs.  KVM only disallows
> NULL updates for select mandatory hooks.  For optional hooks, KVM needs to support
> NULL updates in some capacity to handle the scenario where a vendor module is
> reloaded with different settings, e.g. loading kvm_intel with enable_apicv=0 after
> running with enable_apicv=1.
> 
> WARN_ON() a static_call_update(..., NULL) should be ok, but I believe KVM would
> still need/want macro shenanigans, e.g.
> 
> #define __KVM_X86_OP(func) \
> 	static_call_update(kvm_x86_##func,
> 			   kvm_x86_ops.func ? kvm_x86_ops.func : STATIC_CALL_NOP);
> #define KVM_X86_OP(func) \
> 	WARN_ON(!kvm_x86_ops.func); __KVM_X86_OP(func)
> #define KVM_X86_OP_OPTIONAL __KVM_X86_OP
> #define KVM_X86_OP_OPTIONAL_RET0(func) __KVM_X86_OP

Yeah, something like that might be ok, if we just refuse NULL as an
option.

If only Peter hadn't ruined my Friday with the CFI talk.

-- 
Josh