Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header

[Sean Christopherson <seanjc@google.com>]

On Wed, Nov 02, 2022, David Matlack wrote:
> On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Oct 31, 2022, David Matlack wrote:
> > > +
> > > +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)
> >
> > I think it makes sense to keeping the bundling of the assert+skip.  As written,
> > the last test doesn't need to skip, but that may not always hold true, e.g. if
> > the test adds more stages to verify KVM handles page splits correctly, and even
> > when a skip is required, it does no harm.  I can't think of a scenario where a
> > test would want an FLDS emulation error but wouldn't want to skip the instruction,
> > e.g. injecting a fault from userspace is largely an orthogonal test.
> >
> > Maybe this as a helper name?  I don't think it's necessary to include "assert"
> > anywhere in the name, the idea being that "emulated" provides a hint that it's a
> > non-trivial helper.
> >
> >   static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)
> >
> > or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
> > "flds" is an instruction mnemonic.
> 
> I kept them separate for readability,

Ha, and of course I found assert_exit_for_flds_emulation_failure() hard to read :-)

> but otherwise I have no argument against bundling. I find skip_emulated*()
> somewhat misleading since flds is not actually emulated (successfully). I'm
> trending toward something like handle_flds_emulation_failure_exit() for v4.

How about "skip_flds_on_emulation_failure_exit()"?  "handle" is quite generic and
doesn't provide any hints as to what the function actually does under the hood.