RE: [RFC PATCH v2 4/5] KVM: x86: Remove KVM_X86_OP_OPTIONAL

[Wang, Wei W]

On Friday, April 19, 2024 9:42 PM, Sean Christopherson wrote:
> On Fri, Apr 19, 2024, Wei Wang wrote:
> > KVM_X86_OP and KVM_X86_OP_OPTIONAL were utilized to define and
> execute
> > static_call_update() calls on mandatory and optional hooks, respectively.
> > Mandatory hooks were invoked via static_call() and necessitated
> > definition due to the presumption that an undefined hook (i.e., NULL)
> > would cause
> > static_call() to fail. This assumption no longer holds true as
> > static_call() has been updated to treat a "NULL" hook as a NOP on x86.
> > Consequently, the so-called mandatory hooks are no longer required to
> > be defined, rendering them non-mandatory.
> 
> This is wrong.  They absolutely are mandatory.  The fact that static_call()
> doesn't blow up doesn't make them optional.  If a vendor neglects to
> implement a mandatory hook, KVM *will* break, just not immediately on the
> static_call().
> 
> The static_call() behavior is actually unfortunate, as KVM at least would prefer
> that it does explode on a NULL point.  I.e. better to crash the kernel (hopefully
> before getting to production) then to have a lurking bug just waiting to cause
> problems.
> 
> > This eliminates the need to differentiate between mandatory and
> > optional hooks, allowing a single KVM_X86_OP to suffice.
> >
> > So KVM_X86_OP_OPTIONAL and the WARN_ON() associated with
> KVM_X86_OP
> > are removed to simplify usage,
> 
> Just in case it isn't clear, I am very strongly opposed to removing
> KVM_X86_OP_OPTIONAL() and the WARN_ON() protection to ensure
> mandatory ops are implemented.

OK, we can drop patch 4 and 5.

Btw, may I know what is the boundary between mandatory and optional hooks?
For example, when adding a new hook, what criteria should we use to determine
whether it's mandatory, thereby requiring both SVM and VMX to implement it (and
seems need to be merged them together?)
(I searched a bit, but didn't find it)